storage/rangefeed: push rangefeed.Filter into Raft processing goroutine#41788
Merged
craig[bot] merged 1 commit intocockroachdb:masterfrom Nov 21, 2019
Merged
Conversation
Member
nvb
added a commit
to nvb/cockroach
that referenced
this pull request
Oct 22, 2019
Fixes cockroachdb#28666. First commit from cockroachdb#41788. This commit adds a `WITH diff` option to CREATE CHANGEFEED. When the option is provided, changefeeds publications will include a `before` field, which includes the value of the row before the CDC update was applied. We are able to implement this efficiently by pushing the option into Rangefeed and performing the scan of the previous value immediately before applying the update in the Raft processing goroutine. cockroachdb#41788 allows us to avoid performing this lookup when `WITH diff` isn't specified for Rangefeeds, so the small (unmeasured) perf hit is strictly opt-in. DNM: this still needs testing and a bit of polish. It's also not clear how we want to handle catch-up scans when the option is provided.
nvb
added a commit
to nvb/cockroach
that referenced
this pull request
Nov 16, 2019
Fixes cockroachdb#28666. First commit from cockroachdb#41788. This commit adds a `WITH diff` option to CREATE CHANGEFEED. When the option is provided, changefeeds publications will include a `before` field, which includes the value of the row before the CDC update was applied. We are able to implement this efficiently by pushing the option into Rangefeed and performing the scan of the previous value immediately before applying the update in the Raft processing goroutine. cockroachdb#41788 allows us to avoid performing this lookup when `WITH diff` isn't specified for Rangefeeds, so the small (unmeasured) perf hit is strictly opt-in. Release note (sql change): CHANGEFEED now supports a `WITH diff` option, which instructs it to include a `before` field in each publication.
danhhz
reviewed
Nov 20, 2019
Contributor
danhhz
left a comment
There was a problem hiding this comment.
(assuming this is still the same as it was in #41793)
Reviewable status:
complete! 1 of 0 LGTMs obtained (waiting on @danhhz)
This commit creates a rangefeed.Filter object, which informs the producer of logical operations of the information that a rangefeed Processor is interested in, given its current set of registrations. It can be used to avoid performing extra work to provide the Processor with information which will be ignored. This is an important optimization for single-key or small key span rangefeeds, as it avoids some extraneous work for the key spans not being watched. For now, this extra work is just fetching the post-operation value for MVCCWriteValueOp and MVCCCommitIntentOp operations, but this can be extended in the future to include the pre-operation value as well. This will be important to avoid any perf regression when addressing cockroachdb#28666.
59a4c2b to
acf26c0
Compare
Contributor
Author
Yes, exactly the same. TFTR! bors r=danhhz |
Contributor
Build failed (retrying...) |
craig bot
pushed a commit
that referenced
this pull request
Nov 21, 2019
41788: storage/rangefeed: push rangefeed.Filter into Raft processing goroutine r=danhhz a=nvanbenschoten This commit creates a `rangefeed.Filter` object, which informs the producer of logical operations of the information that a `rangefeed.Processor` is interested in, given its current set of registrations. It can be used to avoid performing extra work to provide the Processor with information which will be ignored. This is an important optimization for single-key or small key span rangefeeds, as it avoids some extraneous work for the key spans not being watched. For now, this extra work is just fetching the post-operation value for `MVCCWriteValueOp` and `MVCCCommitIntentOp` operations, but this can be extended in the future to include the pre-operation value as well. This will be important to avoid any perf regression when addressing #28666. 42580: sql: add precision support for TIMESTAMP/TIMESTAMPTZ (v2) r=otan a=otan Refs #32098 Alternative to #42552 ---- Add precision support for TIMESTAMP/TIMESTAMPTZ, supporting precisions from 0 to 6 inclusive. When storing this in the type proto, we introduce a `TimePrecisionIsSet` variable, which is used to differentiate 0 (unset) or 0 (explicitly set) for time related protos. This is abstracted away in the `Precision()` function which returns the correct precision (unset => 6) for time. This allows forwards and backwards compatibility. Note Time will come later, as it involves a bit more plumbing to the base library. Release note (sql change): Introduces precision support for TIMESTAMP and TIMESTAMPTZ, supporting precisions from 0 to 6 inclusive. Previous versions of TIMESTAMP and TIMESTAMPTZ will default to 6 units of precision. Note that if anyone downgrades whilst having precision set, they will have full precision (6) again, but if they re-upgrade they will find their precisions truncated again. Co-authored-by: Nathan VanBenschoten <nvanbenschoten@gmail.com> Co-authored-by: Oliver Tan <otan@cockroachlabs.com>
Contributor
Build succeeded |
nvb
added a commit
to nvb/cockroach
that referenced
this pull request
Nov 21, 2019
Fixes cockroachdb#28666. First commit from cockroachdb#41788. This commit adds a `WITH diff` option to CREATE CHANGEFEED. When the option is provided, changefeeds publications will include a `before` field, which includes the value of the row before the CDC update was applied. We are able to implement this efficiently by pushing the option into Rangefeed and performing the scan of the previous value immediately before applying the update in the Raft processing goroutine. cockroachdb#41788 allows us to avoid performing this lookup when `WITH diff` isn't specified for Rangefeeds, so the small (unmeasured) perf hit is strictly opt-in. Release note (sql change): CHANGEFEED now supports a `WITH diff` option, which instructs it to include a `before` field in each publication.
craig bot
pushed a commit
that referenced
this pull request
Nov 21, 2019
41793: ccl/changefeedccl: implement `WITH diff` option r=nvanbenschoten a=nvanbenschoten Fixes #28666. This commit adds a `WITH diff` option to CREATE CHANGEFEED. When the option is provided, changefeeds publications will include a `before` field, which includes the value of the row before the CDC update was applied. We are able to implement this efficiently by pushing the option into Rangefeed and performing the scan of the previous value immediately before applying the update in the Raft processing goroutine. #41788 allows us to avoid performing this lookup when `WITH diff` isn't specified for Rangefeeds, so the small (unmeasured) perf hit is strictly opt-in. Co-authored-by: Nathan VanBenschoten <nvanbenschoten@gmail.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
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 commit creates a
rangefeed.Filterobject, which informs the producer of logical operations of the information that arangefeed.Processoris interested in, given its current set of registrations. It can be used to avoid performing extra work to provide the Processor with information which will be ignored.This is an important optimization for single-key or small key span rangefeeds, as it avoids some extraneous work for the key spans not being watched.
For now, this extra work is just fetching the post-operation value for
MVCCWriteValueOpandMVCCCommitIntentOpoperations, but this can be extended in the future to include the pre-operation value as well. This will be important to avoid any perf regression when addressing #28666.