Skip to content

Conversation

@shibd
Copy link
Member

@shibd shibd commented Apr 20, 2022

Motivation

#15091

The essential reason for #13911 is that two threads execute the refresh method concurrently. In PR #13911 change, it doesn't completely solve the problem, just avoid concurrent access by threads.

In #14283, The problem was radically resolved by changing below.

objCache.asMap().computeIfPresent(path, (oldKey, oldValue) -> readValueFromStore(path));

In #13911, although multi-threaded access is avoided, it will cause the test future.get() to not get the expected result.
This will cause the updateValueWhenKeyDisappears test to fail. So, revert this commit.

Modifications

Documentation

  • no-need-doc

@github-actions github-actions bot added the doc-not-needed Your PR changes do not impact docs label Apr 20, 2022
@codelipenghui
Copy link
Contributor

@shibd So we need another solution for the flaky test? this PR just revert the previous solution?

@codelipenghui codelipenghui added this to the 2.11.0 milestone Apr 21, 2022
@shibd
Copy link
Member Author

shibd commented Apr 21, 2022

@shibd So we need another solution for the flaky test? this PR just revert the previous solution?

No need, Because #14283 also solved these flaky tests.

Copy link
Contributor

@Jason918 Jason918 left a comment

Choose a reason for hiding this comment

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

LGTM

Copy link
Contributor

@dlg99 dlg99 left a comment

Choose a reason for hiding this comment

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

LGTM

@mattisonchao mattisonchao merged commit 306a0a1 into apache:master May 7, 2022
codelipenghui pushed a commit that referenced this pull request May 20, 2022
nicoloboschi pushed a commit to datastax/pulsar that referenced this pull request May 23, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants