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: decouple lease requests from client context #98453

Closed
erikgrinaker opened this issue Mar 12, 2023 · 0 comments · Fixed by #98460
Closed

kvserver: decouple lease requests from client context #98453

erikgrinaker opened this issue Mar 12, 2023 · 0 comments · Fixed by #98460
Assignees
Labels
A-kv Anything in KV that doesn't belong in a more specific category. C-bug Code not up to spec/doc, specs & docs deemed correct. Solution expected to change code/behavior. T-kv KV Team

Comments

@erikgrinaker
Copy link
Contributor

erikgrinaker commented Mar 12, 2023

Currently, lease requests (acquisitions and extensions) will be cancelled once all waiters go away:

https://github.com/cockroachdb/cockroach/blob/84a19f6e8f0c33b637deaa047c524ea6ab58f9a1/pkg/kv/kvserver/replica_range_lease.go#L105-L121

This is unfortunate in itself, because it might cause ranges to remain without leases indefinitely if lease acquisition takes a long time (e.g. because the previous leaseholder is unresponsive) and the client has a timeout that fires before the lease acquisition completes.

However, it is particularly problematic for expiration based leases, where we attempt to extend the lease asynchronously when a request comes in during the last half of the lease interval. This lease extension uses the client's context, but the client request is likely to complete before the lease extension does, which will cancel the lease extension. We already have a TODO saying as much:

https://github.com/cockroachdb/cockroach/blob/84a19f6e8f0c33b637deaa047c524ea6ab58f9a1/pkg/kv/kvserver/replica_range_lease.go#L1461-L1467

This problem is further exacerbated by the fact that concurrent requests won't join that pending lease request (which might otherwise have tided it over):

https://github.com/cockroachdb/cockroach/blob/84a19f6e8f0c33b637deaa047c524ea6ab58f9a1/pkg/kv/kvserver/replica_range_lease.go#L1434-L1436

Jira issue: CRDB-25277

Epic CRDB-25206

@erikgrinaker erikgrinaker added C-bug Code not up to spec/doc, specs & docs deemed correct. Solution expected to change code/behavior. A-kv Anything in KV that doesn't belong in a more specific category. T-kv KV Team labels Mar 12, 2023
@erikgrinaker erikgrinaker self-assigned this Mar 12, 2023
@craig craig bot closed this as completed in 0c6ccaf Mar 14, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-kv Anything in KV that doesn't belong in a more specific category. C-bug Code not up to spec/doc, specs & docs deemed correct. Solution expected to change code/behavior. T-kv KV Team
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant