Skip to content
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

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

Merged
merged 1 commit into from
Oct 14, 2021

Conversation

arulajmani
Copy link
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
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
pkg/kv/kvclient/rangefeed/rangefeed_external_test.go Outdated Show resolved Hide resolved
pkg/kv/kvclient/rangefeed/rangefeed_external_test.go Outdated Show resolved Hide resolved
pkg/kv/kvclient/rangefeed/rangefeed_external_test.go Outdated Show resolved Hide resolved
pkg/kv/kvclient/rangefeed/rangefeed_external_test.go Outdated Show resolved Hide resolved
@arulajmani
Copy link
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
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
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
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 <[email protected]>.

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 <[email protected]>
Co-authored-by: Lindsey Jin <[email protected]>
Co-authored-by: David Taylor <[email protected]>
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