Skip to content

HDDS-16131. Intermittent failure in TestBlockDeletingService#testBlockDeletionMetricsUpdatedProperlyAfterEachExecution - #11013

Open
echonesis wants to merge 2 commits into
apache:masterfrom
echonesis:HDDS-16131
Open

HDDS-16131. Intermittent failure in TestBlockDeletingService#testBlockDeletionMetricsUpdatedProperlyAfterEachExecution#11013
echonesis wants to merge 2 commits into
apache:masterfrom
echonesis:HDDS-16131

Conversation

@echonesis

Copy link
Copy Markdown
Contributor

What changes were proposed in this pull request?

This patch ensures that BlockDeletingServiceTestImpl is shut down in a finally block. Service shutdown waits for the deletion tasks to finish and release their container DB references before test cleanup runs.

The same lifecycle handling is also added to testChecksumFileUpdatedWhenDeleteRetried, which starts the service without shutting it down and is susceptible to the same teardown race.

What is the link to the Apache JIRA

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

How was this patch tested?

Local Test

mvn -pl :hdds-container-service test \
    -Dtest='TestBlockDeletingService#testBlockDeletionMetricsUpdatedProperlyAfterEachExecution+testChecksumFileUpdatedWhenDeleteRetried' \
    -DskipShade -DskipRecon -DskipDocs

GitHub Actions CI

…kDeletionMetricsUpdatedProperlyAfterEachExecution
@ss77892

ss77892 commented Aug 18, 2026

Copy link
Copy Markdown
Contributor

Overall it looks good. One small thing. It would be better to move service shutdown in @AfterEach. That would prevent this problem in future tests (if someone miss the shutdown)

@echonesis

Copy link
Copy Markdown
Contributor Author

Overall it looks good. One small thing. It would be better to move service shutdown in @AfterEach. That would prevent this problem in future tests (if someone miss the shutdown)

Thanks for the suggestion.
I moved the service shutdown to @AfterEach and registered the service when it is created by the test helper. This ensures the asynchronous deletion tasks finish before the container cache is cleaned up, including when a test fails or a future test misses an explicit shutdown.

private String schemaVersion;
private int blockLimitPerInterval;
private MutableVolumeSet volumeSet;
private BlockDeletingService blockDeletingService;

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: reset this to null in init(). It is never cleared, so a test that runs after a service-creating one without creating its own will re-shutdown() the prior (already-terminated) instance in cleanup. Harmless since shutdown() is idempotent, but resetting drops the cross-test coupling.

@rich7420

Copy link
Copy Markdown
Contributor

@echonesis thanks for the patch!

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.

4 participants