Skip to content

Fix: LockManagerTest.updateValue is flaky - #13850

Closed
shibd wants to merge 2 commits into
apache:masterfrom
shibd:fix-1366
Closed

Fix: LockManagerTest.updateValue is flaky#13850
shibd wants to merge 2 commits into
apache:masterfrom
shibd:fix-1366

Conversation

@shibd

@shibd shibd commented Jan 20, 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 MetadataStore and the concurrent access of the notification thread to the MetadataCache#refresh.

We should queue them in the metada-store thread pool(SingleThread)

Modifications

  1. In AbstractMetadataStore method, all future callback opt are executed through metada-store thread.

Documentation

  • [ x] no-need-doc

@shibd shibd changed the title fix: LockManagerTest.updateValue is flaky Fix: LockManagerTest.updateValue is flaky Jan 20, 2022
@github-actions github-actions Bot added the doc-not-needed Your PR changes do not impact docs label Jan 20, 2022
@shibd

shibd commented Jan 20, 2022

Copy link
Copy Markdown
Member Author

@Jason918 @Technoboy- @codelipenghui Hi, Can you help me review it again? Thanks.

@nodece

nodece commented Jan 20, 2022

Copy link
Copy Markdown
Member

This PR looks the same as #13809, we can find this solution together.

@shibd

shibd commented Jan 20, 2022

Copy link
Copy Markdown
Member Author

This PR looks the same as #13809, we can find this solution together.

Thank you reply, I saw you fix the callback problem of ZKMetadata. I'll cancel this commit.

However, the essential reason why this unit test will report an error is LocalMemoryMetadataStore that does not callback through metada-store thread pool.

I think it can be handled like this:

In AbstractMetadataStore method, all future callback opt are executed through metada-store thread.

@shibd

shibd commented Jan 23, 2022

Copy link
Copy Markdown
Member Author

/pulsarbot run-failure-checks

1 similar comment
@shibd

shibd commented Jan 23, 2022

Copy link
Copy Markdown
Member Author

/pulsarbot run-failure-checks

@shibd

shibd commented Jan 23, 2022

Copy link
Copy Markdown
Member Author

@Jason918 @codelipenghui @nodece Sorry,The current changes have caused other problems. I will resubmit the PR in another way

@shibd shibd closed this Jan 23, 2022
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.

4 participants