refactor and optimize downsampling interceptor #2097
Conversation
wyfo
force-pushed
the
downsampling-refactor
branch
from
3429670 to
0efb3f3
Compare
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #2097 +/- ##
==========================================
+ Coverage 70.89% 70.93% +0.04%
==========================================
Files 373 375 +2
Lines 63127 63243 +116
==========================================
+ Hits 44751 44861 +110
- Misses 18376 18382 +6 ☔ View full report in Codecov by Sentry. |
oteffahi
left a comment
oteffahi
left a comment
There was a problem hiding this comment.
LGTM.
Left a few nitpick comments.
| flow: InterceptorFlow, | ||
| #[cfg(feature = "stats")] stats: Arc<TransportStats>, | ||
| ) -> Self { | ||
| let mut ke_id = KeBoxTree::default(); |
There was a problem hiding this comment.
For future reference: we should move the computation of the key-expr tree outside of the interceptor, and wrap it in an Arc in order to reuse the same tree for both Ingree/Egress interceptors constructed from the same Interceptor Factory.
There was a problem hiding this comment.
For future reference
Yes, because I don't want to do it now
| match msg.body { | ||
| NetworkBodyMut::Push(Push { payload, .. }) => match payload { | ||
| PushBody::Put(_) => self.put, | ||
| PushBody::Del(_) => self.delete, | ||
| }, | ||
| NetworkBodyMut::Request(_) => self.query, | ||
| NetworkBodyMut::Response(_) => self.reply, | ||
| _ => false, | ||
| } |
There was a problem hiding this comment.
The match with all enum variants was intentional to cause a compile-time error if the enum is ever changed. IMO we should keep that.
2 participants
No description provided.