-
Notifications
You must be signed in to change notification settings - Fork 3.8k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
kvserver/rangefeed: remove future package for server rangefeed #125782
Draft
wenyihu6
wants to merge
4
commits into
cockroachdb:master
Choose a base branch
from
wenyihu6:addmuxer
base: master
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
+164
−64
Conversation
This file contains 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
Your pull request contains more than 1000 changes. It is strongly encouraged to split big PRs into smaller chunks. 🦉 Hoot! I am a Blathers, a bot for CockroachDB. My owner is dev-inf. |
wenyihu6
force-pushed
the
addmuxer
branch
3 times, most recently
from
June 18, 2024 18:32
e961b8a
to
3c02358
Compare
It looks like your PR touches production code but doesn't add or edit any test code. Did you consider adding tests to your PR? 🦉 Hoot! I am a Blathers, a bot for CockroachDB. My owner is dev-inf. |
wenyihu6
force-pushed
the
addmuxer
branch
24 times, most recently
from
June 23, 2024 21:48
7d49f2a
to
58e61d6
Compare
wenyihu6
force-pushed
the
addmuxer
branch
14 times, most recently
from
July 1, 2024 12:38
b621e39
to
e5ea3f3
Compare
wenyihu6
force-pushed
the
addmuxer
branch
10 times, most recently
from
July 29, 2024 02:34
bef5e37
to
7f9b83c
Compare
After the output loop completes, 2 cleanup steps are performed: 1) The registration is removed from the processors registry 2) The process is potentially stopped and removed from the replica. Before this change, both of these happened syncronously after the output loop finished. With this change, step (1) happens asyncronously. To facilitate this, an overflow mechanism is provided. This overflow mechanism potentially allocates. Note that we expect that the number of requests is relatively small and should be O(rangefeeds_on_range) so hopefully this mechanism won't be used often. Step (2) is now handled by the processor itself. After processing an unregister request, if the set of registrations falls to zero, we enqueue a Stop event for ourselves. Then, when processing the Stop, we unregister ourselves from the replica. Note that this may look like a small semantics change since the previous unregister callback called Stop() which processes all events. However, note that a Stopped event is processed after all other events, so any events in the queue at the point of processing the unregistration that enqueued the stop will be processed. The motivation for this change is to eventually allow cleanup step (1) to be run as part of registration.disconnect(), which needs to be non-blocking. This is desired for a future change in which there is not a dedicated goroutine to perform this cleanup. Epic: none Release note: None
Rather than a dynamic overflow queue, we now set a flag indicating that an overflow happened and then scan all registrations in the case of overflow. Release note: None Epic: none
For the ScheduledProcess, we now enqueue the unregister as part of the first disconnect call. Epic: none Release note: None
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
Epic: None