The Wayback Machine - https://web.archive.org/web/20220409163913/https://github.com/apache/kafka/pull/11710
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.

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

MINOR: MiniTrogdorCluster mutates objects from other threads #11710

Merged
merged 1 commit into from Feb 3, 2022
Merged

MINOR: MiniTrogdorCluster mutates objects from other threads #11710

merged 1 commit into from Feb 3, 2022

Conversation

lbradstreet
Copy link
Contributor

@lbradstreet lbradstreet commented Jan 25, 2022

MiniTrogdorCluster spins up agents from a different thread when
scheduling them, but does not use volatiles in these objects. It's not
clear that the updated fields are visible.

The hope is that this will fix a failure where it test becomes stuck waiting for MiniTrogdorCluster to shutdown:

	Suppressed: java.lang.InterruptedException
		at java.util.concurrent.locks.AbstractQueuedSynchronizer$ConditionObject.reportInterruptAfterWait(AbstractQueuedSynchronizer.java:2014)
		at java.util.concurrent.locks.AbstractQueuedSynchronizer$ConditionObject.awaitNanos(AbstractQueuedSynchronizer.java:2088)
		at java.util.concurrent.ThreadPoolExecutor.awaitTermination(ThreadPoolExecutor.java:1475)
		at java.util.concurrent.Executors$DelegatedExecutorService.awaitTermination(Executors.java:675)
		at org.apache.kafka.trogdor.coordinator.TaskManager.waitForShutdown(TaskManager.java:699)
		at org.apache.kafka.trogdor.coordinator.Coordinator.waitForShutdown(Coordinator.java:133)
		at org.apache.kafka.trogdor.common.MiniTrogdorCluster.close(MiniTrogdorCluster.java:289)
		at org.apache.kafka.trogdor.coordinator.CoordinatorTest.testTaskRequestWithOldStartMsGetsUpdated(CoordinatorTest.java:595)
		at sun.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
		at sun.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:62)
		at sun.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43)
		at java.lang.reflect.Method.invoke(Method.java:498)
		at org.junit.platform.commons.util.ReflectionUtils.invokeMethod(ReflectionUtils.java:688)
		at org.junit.jupiter.engine.execution.MethodInvocation.proceed(MethodInvocation.java:60)
		at org.junit.jupiter.engine.execution.InvocationInterceptorChain$ValidatingInvocation.proceed(InvocationInterceptorChain.java:131)
		at org.junit.jupiter.engine.extension.TimeoutInvocation.proceed(TimeoutInvocation.java:46)

@lbradstreet
MINOR: MiniTrogdorCluster mutates objects from other threads
ff74aa2 
MiniTrogdorCluster spins up agents from a different thread when
scheduling them, but does not use volatiles in these objects. It's not
clear that the updated fields are visible.
@cmccabe
Copy link
Contributor

@cmccabe cmccabe commented Jan 25, 2022

It would be nice to redo this without the volatiles, but for now, LGTM. Thanks, @lbradstreet .

Kvicii
Kvicii approved these changes Feb 2, 2022
View changes
Copy link
Contributor

@Kvicii Kvicii left a comment

LGTM

dajac
dajac approved these changes Feb 3, 2022
View changes
Copy link
Contributor

@dajac dajac left a comment

LGTM

@dajac dajac merged commit 3781117 into apache:trunk Feb 3, 2022
6 of 10 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@dajac dajac

@cmccabe cmccabe

@Kvicii Kvicii

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
4 participants
@lbradstreet @cmccabe @dajac @Kvicii