apache / pulsar

Apache Pulsar - distributed pub-sub messaging system
https://pulsar.apache.org/
Apache License 2.0
14.14k stars 3.57k forks source link

Flaky-test: org.apache.pulsar.metadata.LockManagerTest updateValueWhenKeyDisappears flakes #15091

Open dlg99 opened 2 years ago

dlg99 commented 2 years ago

org.apache.pulsar.metadata.LockManagerTest updateValueWhenKeyDisappears is flaky. It passes occasionally but more often it fails.

It passes if I revert https://github.com/apache/pulsar/pull/13911

 git revert 78827bebb5ed92885a04a138cf8f5aaa7370d2bc

All tests in org.apache.pulsar.metadata.LockManagerTest pass if I revert the PR. @shibd It is easy to submit a PR to revert your change but can I ask you to repro this? I tested on latest master (a595e03aff4a6c6174cde09acef7b89d0c36ac96)

I ran it locally (on mac):

[INFO] Running org.apache.pulsar.metadata.LockManagerTest
[ERROR] Tests run: 11, Failures: 1, Errors: 0, Skipped: 9, Time elapsed: 1.721 s <<< FAILURE! - in org.apache.pulsar.metadata.LockManagerTest
[ERROR] updateValueWhenKeyDisappears(org.apache.pulsar.metadata.LockManagerTest)  Time elapsed: 0.011 s  <<< FAILURE!
java.util.NoSuchElementException: No value present
    at java.base/java.util.Optional.get(Optional.java:148)
    at org.apache.pulsar.metadata.LockManagerTest.updateValueWhenKeyDisappears(LockManagerTest.java:206)
    at java.base/jdk.internal.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
    at java.base/jdk.internal.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:62)
    at java.base/jdk.internal.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43)
    at java.base/java.lang.reflect.Method.invoke(Method.java:566)
    at org.testng.internal.MethodInvocationHelper.invokeMethod(MethodInvocationHelper.java:132)
    at org.testng.internal.InvokeMethodRunnable.runOne(InvokeMethodRunnable.java:45)
    at org.testng.internal.InvokeMethodRunnable.call(InvokeMethodRunnable.java:73)
    at org.testng.internal.InvokeMethodRunnable.call(InvokeMethodRunnable.java:11)
    at java.base/java.util.concurrent.FutureTask.run(FutureTask.java:264)
    at java.base/java.util.concurrent.ThreadPoolExecutor.runWorker(ThreadPoolExecutor.java:1128)
    at java.base/java.util.concurrent.ThreadPoolExecutor$Worker.run(ThreadPoolExecutor.java:628)
    at java.base/java.lang.Thread.run(Thread.java:829)
shibd commented 2 years ago

Sorry for the late response, I checked the code, and there is a problem.

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.

I looked at the history commits, #14283 concurrent access is solved by direct locking.

So, we can revert this commit.

github-actions[bot] commented 2 years ago

The issue had no activity for 30 days, mark with Stale label.

github-actions[bot] commented 2 years ago

The issue had no activity for 30 days, mark with Stale label.