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

sql: add replication start time to SHOW TENANT WITH REPLICATION STATUS #93551

Merged
merged 1 commit into from
Dec 15, 2022

Conversation

adityamaru
Copy link
Contributor

@adityamaru adityamaru commented Dec 13, 2022

The start time is a useful piece of information to expose to the end user. It is the lower bound for the data we have replicated.

Informs: #93447

Release note: None

@adityamaru adityamaru requested review from lidorcarmel and a team December 13, 2022 21:43
@cockroach-teamcity
Copy link
Member

This change is Reviewable

Copy link
Contributor

@lidorcarmel lidorcarmel left a comment

Choose a reason for hiding this comment

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

lgtm!

// TODO(lidor): replace this start time with the actual replication start time when we have it.
require.GreaterOrEqual(t, protectedTime, testStartTime)
require.GreaterOrEqual(t, protectedTime, replicationDetails.ReplicationStartTime.GoTime())
require.Equal(t, replicationStartTime.UnixNano(), replicationDetails.ReplicationStartTime.GoTime().UnixNano())
Copy link
Contributor

Choose a reason for hiding this comment

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

can you drop the UnixNano()?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The test flake actually revealed that the replicationStartTime is only accurate up to the microsecond after we start running into rounding mismatches. I had to change this to UnixMicro to pass under stress on a gceworker.

The start time is an important and useful piece of information
to expose to the end user. It is the lower bound for the data we have
replicated.

Release note: None
@adityamaru
Copy link
Contributor Author

adityamaru commented Dec 15, 2022

Tracking the unrelated flake here #93731

bors r=lidorcarmel

@craig
Copy link
Contributor

craig bot commented Dec 15, 2022

Build succeeded:

@craig craig bot merged commit 2c2822c into cockroachdb:master Dec 15, 2022
@adityamaru adityamaru deleted the start-time-show branch December 16, 2022 00:58
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.

3 participants