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

backport-19.1: sql: ensure lease expiration is monotonically increasing #36631

Merged

Conversation

vivekmenezes
Copy link
Contributor

Backport 1/1 commits from #36531.

/cc @cockroachdb/release


This is a fix for both a correctness issue and a performance problem.

  1. Correctness issue during schema changes: A node could hand an
    expiration time from a lease to a transaction T, and if the
    node reacquired a lease for the table at an expiration less than
    the existing lease's expiration it could release the existing
    lease (with a greater expiration) before the transaction T committed,
    thereby invalidating T's deadline.
  2. Performance issue: After a lease acquisition if the expiration
    time of the new lease was less than the existing lease, it would replace
    the table in the cache with the new lease and release the older
    lease. But unfortunately it would also not update the table in the
    table name cache (the table name cache always uses the lease with
    the highest expiration). The table in the table name cache would have
    it's lease released rendering the name entry invalid. Therefore until
    the node acquired another lease for the same table, the node would be
    in a state where there was a table cached but it's name -> table cache
    entry invalid, resulting in it making name resolution requests for
    every query (but not lease renewals).

It is likely this bug started happening with the addition of more
aggressive lease acquisitions through #28725. I've reduced the value
of DefaultTableDescriptorLeaseJitterFraction to make the logic
ensuring the monotonicity of the lease expiration fire infrequently.

fixes #36044

Release note: None

This is a fix for both a correctness issue and a performance problem.
1. Correctness issue during schema changes: A node could hand an
expiration time from a lease to a transaction T, and if the
node reacquired a lease for the table at an expiration less than
the existing lease's expiration it could release the existing
lease (with a greater expiration) before the transaction T committed,
thereby invalidating T's deadline.
2. Performance issue: After a lease acquisition if the expiration
time of the new lease was less than the existing lease, it would replace
the table in the cache with the new lease and release the older
lease. But unfortunately it would also not update the table in the
table name cache (the table name cache always uses the lease with
the highest expiration). The table in the table name cache would have
it's lease released rendering the name entry invalid. Therefore until
the node acquired another lease for the same table, the node would be
in a state where there was a table cached but it's name -> table cache
entry invalid, resulting in it making name resolution requests for
every query (but not lease renewals).

It is likely this bug started happening with the addition of more
aggressive lease acquisitions through cockroachdb#28725. I've reduced the value
of DefaultTableDescriptorLeaseJitterFraction to make the logic
ensuring the monotonicity of the lease expiration fire infrequently.

Release note: None
@vivekmenezes vivekmenezes requested review from andreimatei and a team April 8, 2019 20:09
@cockroach-teamcity
Copy link
Member

This change is Reviewable

@vivekmenezes vivekmenezes merged commit c03c307 into cockroachdb:release-19.1 Apr 9, 2019
@vivekmenezes vivekmenezes deleted the backport19.1-36531 branch April 9, 2019 01:06
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