Skip to content

HDDS-6661. TestBackgroundPipelineScrubber.testRun() fails intermittently#3359

Merged
sodonnel merged 2 commits into
apache:masterfrom
sodonnel:HDDS-6661
Apr 28, 2022
Merged

HDDS-6661. TestBackgroundPipelineScrubber.testRun() fails intermittently#3359
sodonnel merged 2 commits into
apache:masterfrom
sodonnel:HDDS-6661

Conversation

@sodonnel

Copy link
Copy Markdown
Contributor

What changes were proposed in this pull request?

TestBackgroundPipelineScrubber.testRun() fails intermittently for me locally, with this trace:

2022-04-27 17:20:44,888 [main] INFO  pipeline.BackgroundPipelineScrubber (BackgroundPipelineScrubber.java:start(123)) - Starting Pipeline Scrubber Service.
2022-04-27 17:20:44,890 [main] INFO  pipeline.BackgroundPipelineScrubber (BackgroundPipelineScrubber.java:notifyStatusChanged(85)) - Service BackgroundPipelineScrubber transitions to RUNNING.
2022-04-27 17:20:47,905 [main] INFO  pipeline.BackgroundPipelineScrubber (BackgroundPipelineScrubber.java:stop(140)) - Stopping Pipeline Scrubber Service.
2022-04-27 17:20:47,905 [PipelineScrubberThread] WARN  pipeline.BackgroundPipelineScrubber (BackgroundPipelineScrubber.java:run(158)) - PipelineScrubberThread is interrupted, exit


Wanted but not invoked:
pipelineManager.scrubPipelines();
-> at org.apache.hadoop.hdds.scm.pipeline.TestBackgroundPipelineScrubber.testRun(TestBackgroundPipelineScrubber.java:97)
Actually, there were zero interactions with this mock.

Wanted but not invoked:
pipelineManager.scrubPipelines();
-> at org.apache.hadoop.hdds.scm.pipeline.TestBackgroundPipelineScrubber.testRun(TestBackgroundPipelineScrubber.java:97)
Actually, there were zero interactions with this mock.

I believe the reason is that when the notifyStatusChanged() method is called, the thread may be running and not waiting.

Then calling notifyAll() does not do anything. The thread will fall into the wait and stay stuck there until the wait interval expires, or another notify() call is received.

I think the test can be made to run reliably by using a volatile boolean to skip the wait along with the notify.

What is the link to the Apache JIRA

https://issues.apache.org/jira/browse/HDDS-6661

How was this patch tested?

Modified unit test

private final long intervalInMillis;
private final long waitTimeInMillis;
private long lastTimeToBeReadyInMillis = 0;
private volatile boolean runImmediately = false;

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Oh, it is a nice trick, so now the UT covers both the running and wait state of the scrubber thread.
Thanks a lot, LGTM.

@sodonnel
sodonnel merged commit 124723a into apache:master Apr 28, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants