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.

Sign up for GitHub

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

Jump to bottom

Fixed possible deadlock in the initialization of MLTransactionLog #11194

Merged
merged 2 commits into from
Jul 2, 2021
Merged

Fixed possible deadlock in the initialization of MLTransactionLog #11194

merged 2 commits into from
Jul 2, 2021

Conversation

merlimat
Copy link
Contributor

@merlimat merlimat commented Jul 2, 2021

Motivation

There is a mix of async and sync operations when initializing the MLTransactionLog. Instead we should be doing all the steps asynchronously in order to avoid deadlocks.

This is one example taken from a CI job:

"main" #1 prio=5 os_prio=0 cpu=88731.92ms elapsed=3179.26s tid=0x00007fb6cc027800 nid=0xc94 waiting on condition  [0x00007fb6d2cdc000]
   java.lang.Thread.State: WAITING (parking)
	at jdk.internal.misc.Unsafe.park(java.base@11.0.11/Native Method)
	- parking to wait for  <0x00000000c6e71030> (a java.util.concurrent.CompletableFuture$Signaller)
	at java.util.concurrent.locks.LockSupport.park(java.base@11.0.11/LockSupport.java:194)
	at java.util.concurrent.CompletableFuture$Signaller.block(java.base@11.0.11/CompletableFuture.java:1796)
	at java.util.concurrent.ForkJoinPool.managedBlock(java.base@11.0.11/ForkJoinPool.java:3128)
	at java.util.concurrent.CompletableFuture.waitingGet(java.base@11.0.11/CompletableFuture.java:1823)
	at java.util.concurrent.CompletableFuture.join(java.base@11.0.11/CompletableFuture.java:2043)
	at org.apache.pulsar.metadata.coordination.impl.LockManagerImpl.close(LockManagerImpl.java:130)
	at org.apache.pulsar.broker.loadbalance.impl.ModularLoadManagerImpl.stop(ModularLoadManagerImpl.java:897)
	at org.apache.pulsar.broker.loadbalance.impl.ModularLoadManagerWrapper.stop(ModularLoadManagerWrapper.java:107)
	at org.apache.pulsar.broker.PulsarService.closeAsync(PulsarService.java:393)
	at org.apache.pulsar.broker.PulsarService$MockitoMock$1571683367.closeAsync$accessor$YpHOMt77(Unknown Source)
	at org.apache.pulsar.broker.PulsarService$MockitoMock$1571683367$auxiliary$pDEUJfee.call(Unknown Source)
	at org.mockito.internal.invocation.RealMethod$FromCallable$1.call(RealMethod.java:40)
	at org.mockito.internal.invocation.RealMethod$FromBehavior.invoke(RealMethod.java:62)
	at org.mockito.internal.invocation.InterceptedInvocation.callRealMethod(InterceptedInvocation.java:141)
	at org.mockito.internal.stubbing.answers.CallsRealMethods.answer(CallsRealMethods.java:44)
	at org.mockito.Answers.answer(Answers.java:98)
	at org.mockito.internal.handler.MockHandlerImpl.handle(MockHandlerImpl.java:106)
	at org.mockito.internal.handler.NullResultGuardian.handle(NullResultGuardian.java:29)
	at org.mockito.internal.handler.InvocationNotifierHandler.handle(InvocationNotifierHandler.java:33)
	at org.mockito.internal.creation.bytebuddy.MockMethodInterceptor.doIntercept(MockMethodInterceptor.java:82)
	at org.mockito.internal.creation.bytebuddy.MockMethodInterceptor.doIntercept(MockMethodInterceptor.java:56)
	at org.mockito.internal.creation.bytebuddy.MockMethodInterceptor$DispatcherDefaultingToRealMethod.interceptSuperCallable(MockMethodInterceptor.java:141)
	at org.apache.pulsar.broker.PulsarService$MockitoMock$1571683367.closeAsync(Unknown Source)
	at org.apache.pulsar.broker.PulsarService.close(PulsarService.java:329)
	at org.apache.pulsar.broker.PulsarService$MockitoMock$1571683367.close$accessor$YpHOMt77(Unknown Source)
	at org.apache.pulsar.broker.PulsarService$MockitoMock$1571683367$auxiliary$rxRaSI6M.call(Unknown Source)
	at org.mockito.internal.invocation.RealMethod$FromCallable$1.call(RealMethod.java:40)
	at org.mockito.internal.invocation.RealMethod$FromBehavior.invoke(RealMethod.java:62)
	at org.mockito.internal.invocation.InterceptedInvocation.callRealMethod(InterceptedInvocation.java:141)
	at org.mockito.internal.stubbing.answers.CallsRealMethods.answer(CallsRealMethods.java:44)
	at org.mockito.Answers.answer(Answers.java:98)
	at org.mockito.internal.handler.MockHandlerImpl.handle(MockHandlerImpl.java:106)
	at org.mockito.internal.handler.NullResultGuardian.handle(NullResultGuardian.java:29)
	at org.mockito.internal.handler.InvocationNotifierHandler.handle(InvocationNotifierHandler.java:33)
	at org.mockito.internal.creation.bytebuddy.MockMethodInterceptor.doIntercept(MockMethodInterceptor.java:82)
	at org.mockito.internal.creation.bytebuddy.MockMethodInterceptor.doIntercept(MockMethodInterceptor.java:56)
	at org.mockito.internal.creation.bytebuddy.MockMethodInterceptor$DispatcherDefaultingToRealMethod.interceptSuperCallable(MockMethodInterceptor.java:141)
	at org.apache.pulsar.broker.PulsarService$MockitoMock$1571683367.close(Unknown Source)
	at org.apache.pulsar.broker.transaction.TransactionTestBase.internalCleanup(TransactionTestBase.java:252)
	at org.apache.pulsar.broker.transaction.TransactionProduceTest.cleanup(TransactionProduceTest.java:119)
	at jdk.internal.reflect.NativeMethodAccessorImpl.invoke0(java.base@11.0.11/Native Method)
	at jdk.internal.reflect.NativeMethodAccessorImpl.invoke(java.base@11.0.11/NativeMethodAccessorImpl.java:62)
	at jdk.internal.reflect.DelegatingMethodAccessorImpl.invoke(java.base@11.0.11/DelegatingMethodAccessorImpl.java:43)
	at java.lang.reflect.Method.invoke(java.base@11.0.11/Method.java:566)
	at org.testng.internal.MethodInvocationHelper.invokeMethod(MethodInvocationHelper.java:132)
	at org.testng.internal.MethodInvocationHelper.invokeMethodConsideringTimeout(MethodInvocationHelper.java:61)
	at org.testng.internal.ConfigInvoker.invokeConfigurationMethod(ConfigInvoker.java:366)
	at org.testng.internal.ConfigInvoker.invokeConfigurations(ConfigInvoker.java:320)
	at org.testng.internal.TestInvoker.runConfigMethods(TestInvoker.java:701)
	at org.testng.internal.TestInvoker.runAfterGroupsConfigurations(TestInvoker.java:677)
	at org.testng.internal.TestInvoker.invokeMethod(TestInvoker.java:548)
	at org.testng.internal.TestInvoker.invokeTestMethod(TestInvoker.java:174)
	at org.testng.internal.MethodRunner.runInSequence(MethodRunner.java:46)
	at org.testng.internal.TestInvoker$MethodInvocationAgent.invoke(TestInvoker.java:822)
	at org.testng.internal.TestInvoker.invokeTestMethods(TestInvoker.java:147)
	at org.testng.internal.TestMethodWorker.invokeTestMethods(TestMethodWorker.java:146)
	at org.testng.internal.TestMethodWorker.run(TestMethodWorker.java:128)
	at org.testng.TestRunner$$Lambda$184/0x0000000100317440.accept(Unknown Source)
	at java.util.ArrayList.forEach(java.base@11.0.11/ArrayList.java:1541)
	at org.testng.TestRunner.privateRun(TestRunner.java:764)
	at org.testng.TestRunner.run(TestRunner.java:585)
	at org.testng.SuiteRunner.runTest(SuiteRunner.java:384)
	at org.testng.SuiteRunner.runSequentially(SuiteRunner.java:378)
	at org.testng.SuiteRunner.privateRun(SuiteRunner.java:337)
	at org.testng.SuiteRunner.run(SuiteRunner.java:286)
	at org.testng.SuiteRunnerWorker.runSuite(SuiteRunnerWorker.java:53)
	at org.testng.SuiteRunnerWorker.run(SuiteRunnerWorker.java:96)
	at org.testng.TestNG.runSuitesSequentially(TestNG.java:1218)
	at org.testng.TestNG.runSuitesLocally(TestNG.java:1140)
	at org.testng.TestNG.runSuites(TestNG.java:1069)
	at org.testng.TestNG.run(TestNG.java:1037)
	at org.apache.maven.surefire.testng.TestNGExecutor.run(TestNGExecutor.java:135)
	at org.apache.maven.surefire.testng.TestNGDirectoryTestSuite.executeSingleClass(TestNGDirectoryTestSuite.java:112)
	at org.apache.maven.surefire.testng.TestNGDirectoryTestSuite.executeLazy(TestNGDirectoryTestSuite.java:123)
	at org.apache.maven.surefire.testng.TestNGDirectoryTestSuite.execute(TestNGDirectoryTestSuite.java:90)
	at org.apache.maven.surefire.testng.TestNGProvider.invoke(TestNGProvider.java:146)
	at org.apache.maven.surefire.booter.ForkedBooter.invokeProviderInSameClassLoader(ForkedBooter.java:384)
	at org.apache.maven.surefire.booter.ForkedBooter.runSuitesInProcess(ForkedBooter.java:345)
	at org.apache.maven.surefire.booter.ForkedBooter.execute(ForkedBooter.java:126)
	at org.apache.maven.surefire.booter.ForkedBooter.main(ForkedBooter.java:418)

@merlimat merlimat added the type/bug The PR fixed a bug or issue reported a bug label Jul 2, 2021
@merlimat merlimat self-assigned this Jul 2, 2021
@merlimat merlimat added this to the 2.9.0 milestone Jul 2, 2021
congbobo184
congbobo184 approved these changes Jul 2, 2021
View reviewed changes
Copy link
Contributor

@congbobo184 congbobo184 left a comment

Choose a reason for hiding this comment

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

LGTM!

@codelipenghui codelipenghui added release/2.8.1 doc-not-needed Your PR changes do not impact docs labels Jul 2, 2021
eolivelli
eolivelli approved these changes Jul 2, 2021
View reviewed changes
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.

+1

@merlimat merlimat merged commit 555042a into apache:master Jul 2, 2021
@merlimat merlimat deleted the fix-txn-sync-async branch July 2, 2021 16:56
codelipenghui pushed a commit that referenced this pull request Jul 7, 2021
@merlimat @codelipenghui
Fixed possible deadlock in the initialization of MLTransactionLog (#1…
14a731e 
…1194)

* Fixed possible deadlock in the initialization of MLTransactionLog

* Fixed tests

(cherry picked from commit 555042a)
@codelipenghui codelipenghui added the cherry-picked/branch-2.8 Archived: 2.8 is end of life label Jul 7, 2021
bharanic-dev pushed a commit to bharanic-dev/pulsar that referenced this pull request Mar 18, 2022
@merlimat @ciaocloud
Fixed possible deadlock in the initialization of MLTransactionLog (ap…
a043fcb 
…ache#11194)

* Fixed possible deadlock in the initialization of MLTransactionLog

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

@aahmed-se aahmed-se aahmed-se approved these changes

@eolivelli eolivelli eolivelli approved these changes

@codelipenghui codelipenghui codelipenghui approved these changes

@congbobo184 congbobo184 congbobo184 approved these changes

Assignees

@merlimat merlimat

Labels
cherry-picked/branch-2.8 Archived: 2.8 is end of life doc-not-needed Your PR changes do not impact docs release/2.8.1 type/bug The PR fixed a bug or issue reported a bug
Projects
None yet
Milestone
2.9.0
Development

Successfully merging this pull request may close these issues.

None yet
5 participants
@merlimat @aahmed-se @eolivelli @codelipenghui @congbobo184