Skip to content

kvclient/rangefeed: make frontier advances observable using an option#71256

Merged
craig[bot] merged 1 commit intocockroachdb:masterfrom
arulajmani:rangefeed-on-frontier-advance
Oct 14, 2021
Merged

kvclient/rangefeed: make frontier advances observable using an option#71256
craig[bot] merged 1 commit intocockroachdb:masterfrom
arulajmani:rangefeed-on-frontier-advance

Conversation

@arulajmani
Copy link
Copy Markdown
Collaborator

This patch adds a OnFrontierAdvance option to the rangefeed libary.
This option allows users of this library to install a callback which
is called whenever the frontier is advanced.

The frontier is used to track the minimum resolvedTS accross all ranges
that the rangefeed span breaks down to. The intention is to allow users
of this library to be able to use this option to checkpoint the
rangefeed progress. The plan is to use it as such for #69661 and
#69614.

Release note: None

@arulajmani arulajmani requested a review from a team as a code owner October 7, 2021 00:52
@cockroach-teamcity
Copy link
Copy Markdown
Member

This change is Reviewable

@arulajmani arulajmani force-pushed the rangefeed-on-frontier-advance branch 2 times, most recently from 19b5d2c to 44752c4 Compare October 7, 2021 14:57
@arulajmani
Copy link
Copy Markdown
Collaborator Author

@ajwerner or @nvanbenschoten does one of you want to take a second look at this as well or is this good to go once Irfan's comments are addressed?

@arulajmani arulajmani force-pushed the rangefeed-on-frontier-advance branch from 44752c4 to 4eab4e6 Compare October 13, 2021 14:20
Copy link
Copy Markdown
Contributor

@ajwerner ajwerner left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

:lgtm:

Reviewed 2 of 3 files at r1.
Reviewable status: :shipit: complete! 1 of 0 LGTMs obtained (waiting on @arulajmani and @nvanbenschoten)


pkg/kv/kvclient/rangefeed/rangefeed_external_test.go, line 198 at r2 (raw file):

				minTS.Backward(ts)
			}
			require.True(

assert.True because it's a separate goroutine?


pkg/kv/kvclient/rangefeed/rangefeed_external_test.go, line 198 at r2 (raw file):

				minTS.Backward(ts)
			}
			require.True(

Truef?

This patch adds a `OnFrontierAdvance` option to the rangefeed libary.
This option allows users of this library to install a callback which
is called whenever the frontier is advanced (with the new frontier
timestamp).

The frontier is used to track the minimum resolvedTS accross all ranges
that the rangefeed span breaks down to. The intention is to allow users
of this library to be able to use this option to checkpoint the
rangefeed progress. The plan is to use it as such for cockroachdb#69661 and
 cockroachdb#69614.

Release note: None
@arulajmani arulajmani force-pushed the rangefeed-on-frontier-advance branch from 4eab4e6 to fc803ee Compare October 14, 2021 17:51
Copy link
Copy Markdown
Collaborator Author

@arulajmani arulajmani left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

TFTRs!

bors r+

Reviewable status: :shipit: complete! 0 of 0 LGTMs obtained (and 1 stale) (waiting on @ajwerner, @arulajmani, and @nvanbenschoten)


pkg/kv/kvclient/rangefeed/rangefeed_external_test.go, line 198 at r2 (raw file):

Previously, ajwerner wrote…

Truef?

Done.

@craig
Copy link
Copy Markdown
Contributor

craig bot commented Oct 14, 2021

Build succeeded:

@craig craig bot merged commit bfe8164 into cockroachdb:master Oct 14, 2021
craig bot pushed a commit that referenced this pull request Oct 18, 2021
71225: rangefeedbuffer: introduce a rangefeed buffer r=irfansharif a=irfansharif

Buffer provides a thin memory-bounded buffer to sit on top of a rangefeed. It
accumulates raw rangefeed events[^1], which can be flushed out in timestamp
sorted order en-masse whenever the rangefeed frontier is bumped. If we
accumulate more events than the limit allows for, we error out to the caller.

We need such a thing in both #69614 and #69661.

[^1]: Rangefeed error events are propagated to the caller, checkpoint events
     are discarded.

Release note: None

First commit is from #71256. Co-authored-by: Arul Ajmani <arula@cockroachlabs.com>.

71534: ui/sql: show summarized statements in the statements table r=lindseyjin a=lindseyjin

Resolves #27021

Previously, statements on the statements page hid too much information.
There were complaints that it was difficult to disambiguate between
statements without having to view the full query on the tooltips.

The first commit in this patch implemented back-end changes to add a new
metadata field for summarized queries, as well as formatting functions.
This second commit implements additional logic to pass that new metadata
to the front-end and display it in the Statements Table.

Currently, we only create summaries of SELECT, INSERT/UPSERT, and UPDATE
statements in the back-end. For all other statement types, we will
continue to use the existing summary system.

![image](https://user-images.githubusercontent.com/29153209/137195266-8582bfe8-23bb-4d64-9129-d876087c9abc.png)

Release note (ui change): Show new statement summaries on the Statements
page. This applies for SELECT, INSERT/UPSERT, and UPDATE statements, and
will enable them to be more detailed and less ambiguous than our
previous formats.

71625: clusterversion: add a (disabled) assertion that binary version is latest r=dt a=dt

This is intended to be flipped on and the release version updated when the cluster version mint
commit is backported to a release branch. Doing so would then prevent accidentally backporting
any future cluster versions without causing this to panic in all tests.

Release note: none.

Co-authored-by: irfan sharif <irfanmahmoudsharif@gmail.com>
Co-authored-by: Lindsey Jin <lindsey.jin@cockroachlabs.com>
Co-authored-by: David Taylor <tinystatemachine@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants