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

HDDS-11989. Enable SCM Ratis in tests related to DeletedBlockLog #7615

Open
wants to merge 7 commits into
base: master
Choose a base branch
from
Open

HDDS-11989. Enable SCM Ratis in tests related to DeletedBlockLog #7615

wants to merge 7 commits into from
+72 −23

Conversation

chungen0126
Copy link
Contributor

@chungen0126 chungen0126 commented Dec 25, 2024 •
edited
Loading

What changes were proposed in this pull request?

Tests related to DeletedBlockLog needs to be tweaked to work with SCM Ratis enabled:

Error:  Tests run: 4, Failures: 0, Errors: 4, Skipped: 0, Time elapsed: 225.079 s <<< FAILURE! - in org.apache.hadoop.ozone.container.common.statemachine.commandhandler.TestBlockDeletion
20468Error:  org.apache.hadoop.ozone.container.common.statemachine.commandhandler.TestBlockDeletion.testBlockDeleteCommandParallelProcess  Time elapsed: 45.39 s  <<< ERROR!

Error:  Tests run: 1, Failures: 1, Errors: 0, Skipped: 0, Time elapsed: 65.86 s <<< FAILURE! - in org.apache.hadoop.ozone.client.rpc.TestDeleteWithInAdequateDN
2897Error:  org.apache.hadoop.ozone.client.rpc.TestDeleteWithInAdequateDN.testDeleteKeyWithInAdequateDN  Time elapsed: 27.673 s  <<< FAILURE!
2898org.opentest4j.AssertionFailedError: Expected org.apache.hadoop.hdds.scm.container.common.helpers.StorageContainerException to be thrown, but nothing was thrown.
	at org.junit.jupiter.api.AssertionFailureBuilder.build(AssertionFailureBuilder.java:152)
	at org.junit.jupiter.api.AssertThrows.assertThrows(AssertThrows.java:73)
	at org.junit.jupiter.api.AssertThrows.assertThrows(AssertThrows.java:35)
	at org.junit.jupiter.api.Assertions.assertThrows(Assertions.java:3128)
	at org.apache.hadoop.ozone.client.rpc.TestDeleteWithInAdequateDN.testDeleteKeyWithInAdequateDN(TestDeleteWithInAdequateDN.java:314)
	at java.base/java.lang.reflect.Method.invoke(Method.java:580)
	at java.base/java.util.ArrayList.forEach(ArrayList.java:1596)
	at java.base/java.util.ArrayList.forEach(ArrayList.java:1596)

Error:  Tests run: 6, Failures: 0, Errors: 3, Skipped: 0, Time elapsed: 108.02 s <<< FAILURE! - in org.apache.hadoop.ozone.container.common.statemachine.commandhandler.TestDeleteContainerHandler
3162Error:  org.apache.hadoop.ozone.container.common.statemachine.commandhandler.TestDeleteContainerHandler.testDeleteContainerRequestHandlerOnClosedContainer  Time elapsed: 16.098 s  <<< ERROR!

What is the link to the Apache JIRA

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

How was this patch tested?

CI:
https://github.com/chungen0126/ozone/actions/runs/12570960386

jojochuang
jojochuang reviewed Dec 31, 2024
View reviewed changes
hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/ozone/OzoneTestUtils.java
/**
* Flush deleted block log & wait till something was flushed.
*/
public static void waitForDeletedBlockLog(StorageContainerManager scm)
Copy link
Contributor

Choose a reason for hiding this comment

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

Out of scope of this PR but I guess this test https://github.com/apache/ozone/blob/master/hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/hdds/scm/TestStorageContainerManager.java#L324-L334
can be rewritten using the new utility method here.

Can be done as part of HDDS-11867

jojochuang
jojochuang reviewed Dec 31, 2024
View reviewed changes
Copy link
Contributor

@jojochuang jojochuang left a comment

Choose a reason for hiding this comment

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

PR makes sense. Is it ready for review?

@chungen0126 chungen0126 changed the title HDDS-11989. Enable SCM Ratis in TestBlockDeletion HDDS-11989. Enable SCM Ratis in tests related to DeletedBlockLog Jan 1, 2025
@chungen0126 chungen0126 marked this pull request as ready for review January 1, 2025 15:58
@chungen0126
Copy link
Contributor Author

PR makes sense. Is it ready for review?

Thanks @jojochuang for the review. Marked it as ready for review.

@adoroszlai adoroszlai requested a review from jojochuang January 2, 2025 14:34
jojochuang
jojochuang reviewed Jan 2, 2025
View reviewed changes
...e/integration-test/src/test/java/org/apache/hadoop/hdds/scm/TestStorageContainerManager.java
return false;
}
}, 1000, 22000);
OzoneTestUtils.waitBlockDeleted(cluster.getStorageContainerManager());
Copy link
Contributor

Choose a reason for hiding this comment

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

hmm this isn't an exact replacement because OzoneTestUtils.waitBlockDeleted() doesn't flush SCMHADBTransactionBuffer

.../org/apache/hadoop/ozone/container/common/statemachine/commandhandler/TestBlockDeletion.java
@@ -817,6 +829,7 @@ public void testBlockDeleteCommandParallelProcess() throws Exception {
// Wait for block delete command sent from OM
GenericTestUtils.waitFor(() -> {
Copy link
Contributor

Choose a reason for hiding this comment

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

can this be replaced by waitForDeletedBlockLog()?
Also it loosk like the next GenericTestUtils.waitFor() can be replaced by waitBlockDeleted()

hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/ozone/OzoneTestUtils.java
/**
* Flush deleted block log & wait till something was flushed.
*/
public static void waitForDeletedBlockLog(StorageContainerManager scm)
Copy link
Contributor

Choose a reason for hiding this comment

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

suggest to rename the method as "flushAndWaitForDeletedBlockLog()"

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

@jojochuang jojochuang jojochuang left review comments

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@chungen0126 @jojochuang