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

kvserver: fix and re-enable slow lease application logging #105519

Merged
merged 1 commit into from
Jun 26, 2023

Conversation

erikgrinaker
Copy link
Contributor

Resolves #97209.

Epic: none
Release note: None

@erikgrinaker erikgrinaker added the backport-23.1.x Flags PRs that need to be backported to 23.1 label Jun 25, 2023
@erikgrinaker erikgrinaker requested review from tbg and a team June 25, 2023 11:51
@erikgrinaker erikgrinaker self-assigned this Jun 25, 2023
@erikgrinaker erikgrinaker requested a review from a team June 25, 2023 11:51
@blathers-crl
Copy link

blathers-crl bot commented Jun 25, 2023

It looks like your PR touches production code but doesn't add or edit any test code. Did you consider adding tests to your PR?

🦉 Hoot! I am a Blathers, a bot for CockroachDB. My owner is dev-inf.

@cockroach-teamcity
Copy link
Member

This change is Reviewable

if leaseChangingHands && iAmTheLeaseHolder {
// Log acquisition of meta and liveness range leases. These are critical to
// cluster health, so it's useful to know their location over time.
if r.descRLocked().StartKey.Less(roachpb.RKey(keys.NodeLivenessKeyMax)) {
Copy link
Member

Choose a reason for hiding this comment

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

From the looks of it this could be moved into maybeLogSlowLeaseApplyWarning with system bool flag, but it's fine as is too.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This isn't slow application though, we log every time the lease is acquired regardless of how slowly it's applied. But I can restructure the entire method to be about lease application logging.

@erikgrinaker erikgrinaker force-pushed the fix-slow-lease-logging branch from 8d6172b to 14b992c Compare June 26, 2023 09:56
@erikgrinaker
Copy link
Contributor Author

bors r+

@craig
Copy link
Contributor

craig bot commented Jun 26, 2023

Build succeeded:

@craig craig bot merged commit 987249b into cockroachdb:master Jun 26, 2023
@erikgrinaker erikgrinaker deleted the fix-slow-lease-logging branch November 14, 2023 10:39
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backport-23.1.x Flags PRs that need to be backported to 23.1
Projects
None yet
Development

Successfully merging this pull request may close these issues.

kvserver: lease expiration warnings are either wrong or overly eager
3 participants