Skip to content

Fix: LockManagerTest.updateValue is flaky - #13911

Merged
codelipenghui merged 2 commits into
apache:masterfrom
shibd:fix-1366
Jan 25, 2022
Merged

Fix: LockManagerTest.updateValue is flaky#13911
codelipenghui merged 2 commits into
apache:masterfrom
shibd:fix-1366

Conversation

@shibd

@shibd shibd commented Jan 23, 2022

Copy link
Copy Markdown
Member

Motivation

#13663 Flaky-test: org.apache.pulsar.metadata.LockManagerTest.updateValue

The reasons are discussed in detail in this PR(#13725)

This is mainly caused by the callback method of LocalMemoryMetadataStore and the concurrent access of the notification thread to the MetadataCache#refresh.

This will only reproduce when MetadataStore implemented as LocalMemoryMetadataStore.

Modifications

  1. Let the LocalMemoryMetadataStore return for future execution in the metadata-store thread pool

Documentation

  • [ x] no-need-doc

@github-actions github-actions Bot added the doc-not-needed Your PR changes do not impact docs label Jan 23, 2022
@shibd

shibd commented Jan 23, 2022

Copy link
Copy Markdown
Member Author

/pulsarbot run-failure-checks

2 similar comments
@shibd

shibd commented Jan 23, 2022

Copy link
Copy Markdown
Member Author

/pulsarbot run-failure-checks

@shibd

shibd commented Jan 24, 2022

Copy link
Copy Markdown
Member Author

/pulsarbot run-failure-checks

@Jason918 Jason918 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

LGTM

@codelipenghui codelipenghui added this to the 2.10.0 milestone Jan 24, 2022

@nodece nodece left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

LGTM

@shibd

shibd commented Jan 24, 2022

Copy link
Copy Markdown
Member Author

/pulsarbot run-failure-checks

@shibd

shibd commented Jan 24, 2022

Copy link
Copy Markdown
Member Author

The reason why the unit test fails is that the #13908 is every time reproduce, so it needs to wait for #13908 merge.

@codelipenghui

Copy link
Copy Markdown
Contributor

@shibd #13908 gets merged. Thanks for the contribution

@dlg99

dlg99 commented Apr 8, 2022

Copy link
Copy Markdown
Contributor

@shibd please take a look #15091
With this change in, updateValueWhenKeyDisappears test fails/fakes.
With this change reverted all LockManagerTest tests pass locally.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

doc-not-needed Your PR changes do not impact docs

Projects

None yet

Development

Successfully merging this pull request may close these issues.

7 participants