-
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: wip #132882
Draft
wenyihu6
wants to merge
15
commits into
cockroachdb:master
Choose a base branch
from
wenyihu6:newintegration
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.
Draft
kvserver/rangefeed: wip #132882
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
Previously, `node.MuxRangefeed` created a child context for each rangefeed request, storing it in the stream interface to allow the node level to be able to shut down registration goroutines. This patch simplifies the approach by passing the stream context directly to `p.Register`, eliminating the need to store context in `streamSink` or return context via the interface. So this patch also removes context from `kvpb.RangeFeedEventSink`. Epic: none Release note: none
This patch moves helper functions to registry_helpers_test. Part of: cockroachdb#129814 Release note: none
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. |
…re tests This patch refactors helper functions in registry_helpers_test. Part of: cockroachdb#129814 Release note: none # Conflicts: # pkg/kv/kvserver/rangefeed/registry_test.go # Conflicts: # pkg/kv/kvserver/rangefeed/registry_test_helper.go
wenyihu6
force-pushed
the
newintegration
branch
from
October 17, 2024 23:21
ab28b53
to
20836cc
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
wenyihu6
force-pushed
the
newintegration
branch
from
October 17, 2024 23:28
f79250d
to
3de8f21
Compare
This patch adds unbufferedRegistration. UnbufferedRegistration is like BufferedRangefeed but uses BufferedStream to buffer live raft updates instead of a using buf channel and having a dedicated per-range per-registration goroutine to volley events to underlying grpc stream. Instead, there is only one BufferedStream for each incoming node.MuxRangefeed rpc call. BufferedStream is responsible for buffering and sending its updates to the underlying grpc stream in a dedicated goroutine O(node). Note that BufferedStreamSender is still left unimplemented. This commit doesn't add any tests for it yet. More tests will be added in future commits of this PR. Part of: cockroachdb#129814 Release note: none # Conflicts: # pkg/kv/kvserver/rangefeed/scheduled_processor.go # Conflicts: # pkg/kv/kvserver/rangefeed/scheduled_processor.go
wenyihu6
force-pushed
the
newintegration
branch
from
October 17, 2024 23:30
44b1137
to
216c173
Compare
This patch adds a new data structure eventQueue which is like a queue but uses a fixed size for the chunked linked list. Each chunk has a fixed size of 4096 elements. This implementation uses sync.Pool to reduce the number of allocations. pushBack, popFront, len run in constant time. removeAll runs in linear time with respect to the number of elements in the queue. This structure is not safe for concurrent use. This is for future commits to include the queue in the BufferedSender to buffer events at the node level. Part of: cockroachdb#129813 Release note: none
This patch is the last step for reducing long-running O(ranges) goroutines in kvserver/rangefeed. It changes the BufferedSender to use a queue to buffer events before forwarding them to underlying grpc stream. Closed: cockroachdb#129813 Release note: A new cluster setting `kv.rangefeed.buffered_stream_sender.enabled` can now be used to allow rangefeed to use buffered sender for all rangefeed feeds instead of buffering events separately per client per range. # Conflicts: # pkg/kv/kvserver/rangefeed/buffered_sender.go # Conflicts: # pkg/kv/kvserver/rangefeed/buffered_sender.go # Conflicts: # pkg/kv/kvserver/rangefeed/buffered_sender.go
wenyihu6
force-pushed
the
newintegration
branch
from
October 18, 2024 01:15
5bf9eb9
to
cd21718
Compare
This patch refactors the error for kvpb.RangeFeedRetryError_REASON_SLOW_CONSUMER into newRetryErrBufferCapacityExceeded. Part of: cockroachdb#126560 Release note: none
wenyihu6
force-pushed
the
newintegration
branch
from
October 18, 2024 01:16
cd21718
to
c3782db
Compare
This patch adds capacity to node level buffered sender which will shut down all registrations if the node level buffer had overflowed. Part of: cockroachdb#129813 Release note: none TODO: add a larger test at the kvclient side to make sure error returned here is treated as a restart signal # Conflicts: # pkg/kv/kvserver/rangefeed/buffered_sender.go # pkg/kv/kvserver/rangefeed/buffered_sender_test.go # Conflicts: # pkg/kv/kvserver/rangefeed/buffered_sender_test.go
wenyihu6
force-pushed
the
newintegration
branch
from
October 18, 2024 01:19
c3782db
to
ee1adf8
Compare
# Conflicts: # pkg/kv/kvserver/rangefeed/registry_test_helper.go
# Conflicts: # pkg/kv/kvserver/rangefeed/processor_test.go # pkg/kv/kvserver/rangefeed/registry_helpers_test.go # Conflicts: # pkg/kv/kvserver/rangefeed/processor_helpers_test.go # Conflicts: # pkg/kv/kvserver/rangefeed/processor_test.go
This patch adds more test cases for unbufferedRegistration. Closed: cockroachdb#126560 Release note: none # Conflicts: # pkg/kv/kvserver/rangefeed/registry_test.go # Conflicts: # pkg/kv/kvserver/rangefeed/registry_test.go
# Conflicts: # pkg/kv/kvserver/rangefeed/buffered_sender_test.go # Conflicts: # pkg/kv/kvserver/rangefeed/buffered_sender_test.go # pkg/kv/kvserver/rangefeed/unbuffered_registration_test.go # Conflicts: # pkg/kv/kvserver/rangefeed/buffered_sender_test.go
wenyihu6
force-pushed
the
newintegration
branch
from
October 18, 2024 02:06
7cb9a82
to
7fc4d84
Compare
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.
No description provided.