Skip to content

HDDS-15449. Avoid leaked event-processing thread and async work outliving tests in TestReconTaskControllerImpl#10452

Merged
Russole merged 2 commits into
apache:masterfrom
Russole:HDDS-15449
Jun 8, 2026
Merged

HDDS-15449. Avoid leaked event-processing thread and async work outliving tests in TestReconTaskControllerImpl#10452
Russole merged 2 commits into
apache:masterfrom
Russole:HDDS-15449

Conversation

@Russole

@Russole Russole commented Jun 6, 2026

Copy link
Copy Markdown
Contributor

What changes were proposed in this pull request?

  • Added teardown in TestReconTaskControllerImpl to stop the task controller after each test.
  • Updated queue-only reinitialization tests to stop the background event processor before enqueueing events.
  • Replaced async wait logic with deterministic waits for task status updates so test work does not outlive the test method.

What is the link to the Apache JIRA

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

How was this patch tested?

  • All CI test passed

@Russole

Russole commented Jun 6, 2026

Copy link
Copy Markdown
Contributor Author

Hi @chihsuan, You originally spotted this issue, so I’d appreciate it if you could take a look and review this patch.

@chihsuan chihsuan left a comment

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.

Thanks @Russole for picking this up! Just left a small non-blocking nit, but overall LGTM. 👍

} catch (Exception e) {
return false;
}
}, 100, 5000);

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.

nit: This waitFor block is duplicated verbatim in testConsumeOMEvents and testFailedTaskRetryLogic. We can consider extracting a small helper.

Not a blocker, happy to merge as is.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Thanks for pointing this out. I agree a helper could clean this up, but I’ll keep it as-is for this patch since the duplication is small and the current form keeps the wait condition explicit.

@Russole

Russole commented Jun 7, 2026

Copy link
Copy Markdown
Contributor Author

Thanks @chihsuan for the review.

@Gargi-jais11 Gargi-jais11 left a comment

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.

Thanks @Russole for the patch. LGTM!

@Russole

Russole commented Jun 8, 2026

Copy link
Copy Markdown
Contributor Author

Thanks @Gargi-jais11 for the review.

@Russole Russole merged commit 8f64efb into apache:master Jun 8, 2026
32 checks passed
@Russole Russole deleted the HDDS-15449 branch June 8, 2026 15:40
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.

3 participants