errose28 commented on code in PR #6875:
URL: https://github.com/apache/ozone/pull/6875#discussion_r1697250696
##########
hadoop-hdds/container-service/src/test/java/org/apache/hadoop/ozone/container/common/TestBlockDeletingService.java:
##########
@@ -1154,4 +1208,27 @@ private void
setLayoutAndSchemaForTest(ContainerTestVersionInfo versionInfo) {
this.schemaVersion = versionInfo.getSchemaVersion();
ContainerTestVersionInfo.setTestSchemaVersion(schemaVersion, conf);
}
+
+ private void assertDeletionsInChecksumFile(ContainerData data, int
numBlocks) {
+ File checksumFile =
ContainerChecksumTreeManager.getContainerChecksumFile(data);
+ ContainerProtos.ContainerChecksumInfo checksumInfo = null;
+ try (FileInputStream inStream = new FileInputStream(checksumFile)) {
+ checksumInfo = ContainerProtos.ContainerChecksumInfo.parseFrom(inStream);
+ } catch (IOException ex) {
+ fail("Failed to read container checksum tree file", ex);
+ }
Review Comment:
There's a few aspects of this comment that can be separated out:
- Test vs. production code
Proper synchronization during read is not required as part of the test
because we are controlling what operations are happening to the file. Using the
locks in the test requires using the same `ContainerChecksumTreeManager`
instance the datanode uses, which will be more difficult to extract in higher
level tests. We should probably disable the scanner in this test to make sure
it isn't flaky though.
- Code duplication
Production code has no code duplication around file reading. For tests this
and other tree verification helper methods are needed for multiple tests. I
will move them to a test util class to address code duplication in tests.
- Minimizing chances of corruption during write and proper handling of
corrupt files
For both of these I have filed HDDS-11253 since it is a separate issue that
is not related to block delete.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
