extproc : implement flow control for A93 - #9343
Conversation
eshitachandwani
added
Type: Feature
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #9343 +/- ##
========================================
Coverage 87.50% 87.50%
========================================
Files 425 429 +4
Lines 30303 30622 +319
========================================
+ Hits 26516 26797 +281
- Misses 3786 3825 +39
+ Partials 1 0 -1
🚀 New features to boost your workflow:
|
easwars
left a comment
easwars
left a comment
There was a problem hiding this comment.
The changes look good in general. The only major comment is about possibly adding those extra test scenarios.
| downstreamToSideStreamWindow atomic.Int64 // downstream to sidestream window size remaining | ||
| downstreamToSideStreamPositiveUpdateCh chan struct{} // signals the downstream to sidestream window becoming positive | ||
|
|
||
| upstreamToSideStreamWindow atomic.Int64 // upstream to sidestream window size remaining | ||
| upstreamToSideStreamPositiveUpdateCh chan struct{} // signals the upstream to sidestream window becoming positive |
There was a problem hiding this comment.
Nit/Optional: What do you think about names like: d2sWindow, d2sPositiveUpdateCh, u2sWindow and u2sPositiveUpdateCh?
There was a problem hiding this comment.
I think they will be a little difficult to follow through while reading the code. Nonetheless I have changed the names, let me know if you want me to change it back.
There was a problem hiding this comment.
Hmm .... I wasn't too particular about the names. I do agree that they aren't the most readable. Ok, let's revert them back. Sorry for the churn.
| downstreamToSideStreamWindow atomic.Int64 // downstream to sidestream window size remaining | ||
| downstreamToSideStreamPositiveUpdateCh chan struct{} // signals the downstream to sidestream window becoming positive | ||
|
|
||
| upstreamToSideStreamWindow atomic.Int64 // upstream to sidestream window size remaining | ||
| upstreamToSideStreamPositiveUpdateCh chan struct{} // signals the upstream to sidestream window becoming positive |
There was a problem hiding this comment.
Hmm .... I wasn't too particular about the names. I do agree that they aren't the most readable. Ok, let's revert them back. Sorry for the churn.
| if err != nil { | ||
| t.Fatalf("proto.Marshal failed: %v", err) | ||
| } | ||
| expectedInc := int64(len(reqBytes)) |
| if err != nil { | ||
| t.Fatalf("proto.Marshal failed: %v", err) | ||
| } | ||
| expectedRespInc := int64(len(mutatedRespBytes)) |
2 participants
This PR implements flow control for ext proc filter. THis PR also has version change for
envoyproxy/go-control-plane/envoysince the new flow control protocol changes are in master and not in ay release.#ext-proc-a93
RELEASE NOTES: None