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

kv: Add created column to active_range_feeds table. #77597

Merged
merged 1 commit into from
Mar 10, 2022

Conversation

miretskiy
Copy link
Contributor

@miretskiy miretskiy commented Mar 10, 2022

Add created column to active_range_feeds table.
This column is initialized to the time when the partial range feed
was created. This allows us to determine, among other things,
whether or not the rangefeed is currently performing a catchup scan
(i.e. it's resolved column is 0), and how long the scan has been running
for.

Release Notes (enterprise): Add created time column
to crdb_internal.active_range_feeds virtual table to improve observability
and debugability of rangefeed system.

Fixes #77581

Release Justification: Low impact observability/debugability improvement.

@miretskiy miretskiy requested a review from a team as a code owner March 10, 2022 14:30
@miretskiy miretskiy requested a review from a team March 10, 2022 14:30
@cockroach-teamcity
Copy link
Member

This change is Reviewable

StartTS: startFrom,
Span: span,
StartTS: startFrom,
EstablishedTime: timeutil.Now(),
Copy link
Contributor

@erikgrinaker erikgrinaker Mar 10, 2022

Choose a reason for hiding this comment

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

I'm not sure what exactly we want the semantics of this field to be, but maybe we should set it whenever the rangefeed is actually established, i.e. when we get a nil error back from client.RangeFeed()? At least to me, "established" implies that the rangefeed connection was successfully set up. I guess that also implies that it would get updated if we ever disconnect or reconnect too.

Alternatively, we could consider calling this Created and use Established as a separate field.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ack... I was going back and forth w/ established v created. Changing to created.

Copy link
Contributor

@erikgrinaker erikgrinaker left a comment

Choose a reason for hiding this comment

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

LGTM, but remember to update commit/PR description.

@miretskiy miretskiy changed the title kv: Add established column to active_range_feeds table. kv: Add created column to active_range_feeds table. Mar 10, 2022
Add `created` column to `active_range_feeds` table.
This column is initialized to the time when the partial range feed
was created.  This allows us to determine, among other things,
whether or not the rangefeed is currently performing a catchup scan
(i.e. it's resolved column is 0), and how long the scan has been running
for.

Release Notes (enterprise): Add created time column
to `crdb_internal.active_range_feeds` virtual table to improve observability
and debugability of rangefeed system.

Release Justification: Low impact observability/debugability improvement.
@miretskiy
Copy link
Contributor Author

bors r+

@craig
Copy link
Contributor

craig bot commented Mar 10, 2022

Build succeeded:

@craig craig bot merged commit 8d046fc into cockroachdb:master Mar 10, 2022
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.

changefeedccl, kv: Expand active_range_feeds
3 participants