Make DiscoveryServer stream ID global#157
Conversation
smtilden
force-pushed
the
stilden/global-stream-counter
branch
from
2dddc8f to
e8ea8ba
Compare
|
When surfacing V2 and V3 streams alongside each other in envoy-control, DiscoveryServerCallbacks are unable to differentiate between V2 & V3 ADS upon onStreamClose(), onStreamCloseWithError(). This means that any DiscoveryServerCallback that keeps state cannot pivot on stream IDs, since there will be duplicate between V2 & V3. This change creates a global StreamCounter, which ensures stream IDs will be unique across V2 & V3 streams. Signed-off-by: Stephanie Tilden <stephanietilden@squareup.com>
smtilden
force-pushed
the
stilden/global-stream-counter
branch
from
e8ea8ba to
91da9e9
Compare
Codecov Report
@@ Coverage Diff @@
## master #157 +/- ##
============================================
- Coverage 88.22% 88.15% -0.08%
- Complexity 298 300 +2
============================================
Files 31 32 +1
Lines 977 979 +2
Branches 78 78
============================================
+ Hits 862 863 +1
- Misses 85 86 +1
Partials 30 30
Continue to review full report at Codecov.
|
| final ConfigWatcher configWatcher; | ||
| final ProtoResourcesSerializer protoResourcesSerializer; | ||
| private final ExecutorGroup executorGroup; | ||
| private final AtomicLong streamCount = new AtomicLong(); |
There was a problem hiding this comment.
Can you instead just make this field static?
baojr
left a comment
baojr
left a comment
There was a problem hiding this comment.
We've been running this in production for a while without issue.
jakubdyszkiewicz
left a comment
jakubdyszkiewicz
left a comment
There was a problem hiding this comment.
@slonka LGTM, can we merge it?
|
I will look at it tomorrow, if you're in a hurry then go for it. |
|
@jakubdyszkiewicz do you want to release it or should I? |
|
@slonka looks like we should probably merge this? is your approval still good? |
|
@mpuncel - yeah 👍 - I just did not release it because the release process was broken at the time. If it's fixed then go ahead. |
6 participants
No description provided.