HDDS-13218. Add integration tests for snapshot defrag checkpoint footprint savings - #11094
HDDS-13218. Add integration tests for snapshot defrag checkpoint footprint savings#11094arunsarin85 wants to merge 3 commits into
Conversation
chihsuan
left a comment
chihsuan
left a comment
There was a problem hiding this comment.
Thanks for working on this! @arunsarin85 I left a couple of inline comments.
One broader thought: the suite ends up building five 3-datanode clusters, and every assertion is on OM-side checkpoint dirs, so I wonder if we can trim the runtime a bit.
I also noticed that quite a bit of the test scaffolding overlaps with TestOmSnapshotCheckpointDbContent. Consolidating the shared helpers might make a good follow-up task under HDDS-13003.
| int s2Version = readSnapshotVersion(snapshots.get(1)); | ||
| int s3Version = readSnapshotVersion(snapshots.get(2)); | ||
|
|
||
| triggerDefragUntilDone(snapshots); |
There was a problem hiding this comment.
This second pass looks like doesn't actually runs a defrag. triggerDefragUntilDone returns early once every snapshot is already defrag-complete, which is exactly the state the first call leaves behind. Would calling om.triggerSnapshotDefrag(false) directly here work for what you had in mind?
| } | ||
| try { | ||
| om.triggerSnapshotDefrag(false); | ||
| } catch (IOException e) { |
There was a problem hiding this comment.
Could we preserve the last IOException while polling and include it in the timeout failure? At the moment, a real defrag error may appear as “not completed yet” and only surface as a generic timeout after 10 minutes, which could make CI failures difficult to diagnose.
| */ | ||
| @Test | ||
| public void testSnapshotDefragReducesCheckpointFootprintWithChurn() throws Exception { | ||
| runChurnFootprintScenario(BucketLayout.OBJECT_STORE); |
There was a problem hiding this comment.
Since both layouts run in one @Test and the messages in assertDefragReducedChainFootprint don't carry the layout, a CI failure wouldn't tell us whether OBS or FSO broke. Could we parameterize this test by bucket layout, or include the layout in the assertion messages?
| conf.setInt(SNAPSHOT_DEFRAG_LIMIT_PER_TASK, 10); | ||
| conf.setTimeDuration(OZONE_SNAPSHOT_DELETING_SERVICE_INTERVAL, 1, TimeUnit.SECONDS); | ||
|
|
||
| cluster = MiniOzoneCluster.newBuilder(conf).setNumDatanodes(3).build(); |
There was a problem hiding this comment.
nit: Since these assertions only inspect OM checkpoint state and do not depend on block replication, could the test use a single datanode with replication factor one? This may reduce the runtime considerably, given that the suite starts several clusters.
Why do we need a new cluster for each test? |
3 participants
What changes were proposed in this pull request?
Please describe your PR in detail:
Added correctness coverage in TestOmSnapshotCheckpointDbContent; this PR adds space savings / footprint coverage in a separate test class.
Measurement approach
Test scenarios (4 tests)
testSnapshotDefragReducesCheckpointFootprintWithChurnS1/S2/S3) with AOScompactDB()plus insert/overwrite/delete churn. Runs on OBS, then afterrestartCluster()on FSO. Asserts SST redundancy drops after defrag. OBS pass also verifies one full defrag (chain head) and two incremental defrags (S2,S3) viaOmSnapshotInternalMetricsandpathPreviousSnapshotId.testObsSnapshotDefragReducesFootprintAfterMiddleSnapshotPurgeS2, waits for purge, triggers follow-up defrag onS3. AssertsS3footprint does not grow materially and the remainingS1+S3chain footprint shrinks vs the original three-snapshot chain.testObsSingleSnapshotFullDefragReducesCheckpointFootprintv0checkpoint dir is removed, defrag completes (needsDefrag=false).testObsRepeatedDefragDoesNotIncreaseCheckpointFootprintWhat is the link to the Apache JIRA
https://issues.apache.org/jira/browse/HDDS-13218
(Parent epic: https://issues.apache.org/jira/browse/HDDS-13003)
How was this patch tested?
https://github.com/arunsarin85/ozone/actions/runs/32661013503