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

Resource locks should automatically revalidate after a metadata session is re-established #10351

Merged
merged 1 commit into from
Apr 26, 2021

Conversation

merlimat
Copy link
Contributor

Motivation

This change is to enhance the ResourceLock to automatically revalidate the lock with the metadata store once a session is lost and then re-established.
There are multiple state in which a lock can be left after the session is recovered, with either stale value that is going to get cleaned up or some other instance took over the lock.

This single implementation will be then used in all the places where we're taking ownerships in the code-base.

@merlimat merlimat added the type/enhancement The enhancements for the existing features or docs. e.g. reduce memory usage of the delayed messages label Apr 24, 2021
@merlimat merlimat added this to the 2.8.0 milestone Apr 24, 2021
@merlimat merlimat self-assigned this Apr 24, 2021
Copy link
Contributor

@eolivelli eolivelli left a comment

Choose a reason for hiding this comment

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

I did a first pass and found nothing bad.
Great work.
I would like to take another look without hurry on Monday

@lhotari
Copy link
Member

lhotari commented Apr 25, 2021

"CI - Unit - Brokers - Other / unit-tests" build job hangs because of separate issue #10356 .

@lhotari
Copy link
Member

lhotari commented Apr 25, 2021

/pulsarbot run-failure-checks

@merlimat merlimat requested a review from eolivelli April 26, 2021 16:52
Copy link
Contributor

@eolivelli eolivelli left a comment

Choose a reason for hiding this comment

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

LGTM

@eolivelli eolivelli merged commit 903494b into apache:master Apr 26, 2021
@merlimat merlimat deleted the reacquiring-locks branch April 26, 2021 20:22
@lhotari
Copy link
Member

lhotari commented Aug 19, 2021

The test LockManagerTest.revalidateLockOnDifferentSession added by this PR is flaky. @merlimat would you mind taking a look at the stacktraces provided in the issue about the flaky test, #11690 ? Thanks

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type/enhancement The enhancements for the existing features or docs. e.g. reduce memory usage of the delayed messages
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants