Ssd/stream status early remove #156467
Draft
Ssd/stream status early remove #156467
+231
−86
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This PR fixes some bugs discovered when trying to increase the test coverage of the buffered sender unit tests: - A stream would actually never move into the "overflowing" state. Rather it would stay in the "active" state and keep returning a buffer capacity error until the registration sent the error back to us, in which case it would go straight to overflowed. - This was papering over another bug in which the overflowing state could produce an invalid stream of events. Here, I add test coverage for the expected state transitions and fix these bugs by: - Dropping buffered messages for streams not in the stream map. This works because the stream will be in the stream map from the point of the RegisteringStream call and is only removed when the stream finally sends the error. Thus any message for a stream not in map represents either (1) a message sent to a StreamID before RegisteringStream has been called or (2) a message sent to a StreamID after an Error. (1) would be a programming error and (2) would be messages that are dropped server-side anyway. - Re-organizing the code such that it is harder to miss saving the stream status back to the status map. Note any potential bug here only existed on master and thus no release note is needed. Epic: CRDB-55217 Release note: None
|
This change is |
This changes the flow control in the buffered sender to allow us to immediately start dropping buffered sends after the first error is enqueued. This simplifies the code, makes it easier to write assertions about message ordering, and avoids cases where we have unaccounted for memory used by messages that were in the queue after an error. Epic: CRDB-55217 Release note: None
stevendanna
force-pushed
the
ssd/stream-status-early-remove
branch
from
6339429 to
239a077
Compare
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
No description provided.