[WIP, DNM] spanconfig: change system span configurations rpcs, improve types#76138
Closed
arulajmani wants to merge 4 commits intocockroachdb:masterfrom
Closed
[WIP, DNM] spanconfig: change system span configurations rpcs, improve types#76138arulajmani wants to merge 4 commits intocockroachdb:masterfrom
arulajmani wants to merge 4 commits intocockroachdb:masterfrom
Conversation
This patch introduces a new `systemspanconfig.Target` type which is intended as in interemediate representation for system span config targets throughout the `spanconfig` package. It ties together a targeter tenant ID with a targetee tenant ID (along with validation to ensure secondary tenants don't target other tenants). We also reserve a `SystemSpanConfigSpan` at the end of the System range. This allows us to assign special meaning to key-prefixes carved out of this range (when stored in `system.span_configurations`). By ensuring this span is carved at the end of the system range, we can ensure that the host tenant does not install span configurations to this span directly. We define special key prefixes for system span configurations in this range and establish a mapping between a Target <-> span using `Encode` and `Decode` methods. This is useful when we store sysytem span configurations in `system.span_configurations` given its schema. Ditto for reading from the table. We'll make use of the `Target` in upcoming PRs, specifically in the `KVAccessor`, `KVSubscriber`, and (the upcoming) `SystemSpanConfigStore`. The `KVAccessor` will construct `Targets` using tenant information from the context as the targeter tenant. It'll use the `roachpb.SystemSpanConfigTargets` supplied to it by RPCs (See cockroachdb#75615). The encoding/decoding methods will be used when writing to/reading from `system.span_configurations`. While the existing `roachpb.SystemSpanConfigTarget` is sufficient for our use in the `KVAccessor` (which runs on SQL pods), the richer structure is motivated by the desire to also use this type in the `KVSubscriber` (when receiving system span config events) and `SystemSpanConfigStore` (which is an in-memory representation of the system span config state). Down in KV we want to distinguish between a tenant installed system span config on its entire keyspace vs. a host tenant installed system span config over a tenants keyspace. Release note: None
This patch implements `GetSystemSpanConfigEntries` and `UpdateSystemSpanConfigEntries` RPCs. Release note: None
This reverts commit 5177417. Release note (<category, see below>): <what> <show> <why>
TODO(arul) Introduce spanconfig.Record to replace roachpb.SpanConfigEntry in the spanconfig package. Teach the existing RPCs on KVAccessor to work for system span configs. Release note: None
Member
|
This change is |
Collaborator
Author
|
@irfansharif and I looked over this together. We're mostly on-board with the place this is headed, bar a few suggestions that Irfan had. I'll capture them here so that when I carve out polished pieces people can refer to them.
|
craig bot
pushed a commit
that referenced
this pull request
Feb 9, 2022
75519: cli: add tracing to `debug send-kv-batch` r=andreimatei,erikgrinaker a=knz This extends the `debug send-kv-batch` CLI utility to optionally collect and print a trace of the remote processing. (big thanks to @andreimatei for the help) 76213: spanconfig: replace roachpb.SpanConfigEntry in package spanconfig r=arulajmani a=arulajmani See individual commits for details. Polished up and carved out from #76138. In the next patch, we'll modify the existing span configuration RPC arguments to something similar in the draft PR above. Then, we'll make `spanconfig.Target` a union over system targets and spans (instead of the alias for roachpb.Spans as in this PR). We'll also add all the encoding and decoding methods for system targets that we've seen in prior draft patches. 76254: backupccl: flip `bulkio.backup.split_keys_on_timestamps` to true r=dt a=adityamaru Release note (sql change): Release note (sql change): BACKUPs of ranges containing extremely large numbers of revisions to a single row no longer fail with errors related to exceeding size limit. 76265: changefeedccl: Increase message size limits for kafka sink. r=miretskiy a=miretskiy Sarama library, used by kafka sink, limits the maximum message sizes locally. When those limits are exceeded, sarama library returns confusing error message which seems to imply that the remote kafka server rejected the message, even though this rejection happened locally: `kafka server: Message was too large, server rejected it to avoid allocation error.` This PR addresses the problem by increasing sarama limits to 2GB (max int32). An alternative approach was to extend `kafka_sink_config` to specify maximum message size. However, this alternative is less desirable. For one, the user supplied configuration can run afoul other limits imposed by sarama library (e.g. `MaxRequestSize`), so more configuration option must be added. In addition, this really exposes very low level implementation details in the sarama library -- something that we probably should not do. Fixes #76258 Release Notes (enterprise change): Kafka sink supports larger messages, up to 2GB in size. Co-authored-by: Raphael 'kena' Poss <knz@thaumogen.net> Co-authored-by: arulajmani <arulajmani@gmail.com> Co-authored-by: Aditya Maru <adityamaru@gmail.com> Co-authored-by: Yevgeniy Miretskiy <yevgeniy@cockroachlabs.com>
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
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.
This is very much a WIP, in that, the commit history needs to be cleaned up, a fair few comments need polishing, and the system span config tests from #75769 need to be adapted to work here. However, the types in the package
spanconfigand the RPCs to set system span configurations, are a better reflection of the various discussions we've had in the last week or so. In particular, this PR:roachpb.SpanConfigfor system span configurations (with some validation).spanconfig.Targetto achieve the above (and a similar looking proto type) to do achieve the above. ATargetcan be aspan, like before, or a system target. System targets can apply to a particular tenant's ranges or the entire cluster.spanconfig.Recordto tie together aspanconfig.Targetandroachpb.SpanConfig. This replaces usage ofSpanConfigEntrys from before; they're now only used in RPCs.I'm going to graft and polish parts of this into separate patches now that we have some rough end state laid out. Let me know how these types look/if you have high level comments about this.