Skip to content

HDDS-16598. Fix intermittent failure in TestContainerStateMachine - #11319

Merged
smengcl merged 2 commits into
apache:masterfrom
F64116045:HDDS-16598
Oct 7, 2026
Merged

HDDS-16598. Fix intermittent failure in TestContainerStateMachine#11319
smengcl merged 2 commits into
apache:masterfrom
F64116045:HDDS-16598

Conversation

@F64116045

@F64116045 F64116045 commented Sep 24, 2026 •
edited by peterxcli
Loading

Copy link
Copy Markdown
Contributor

What changes were proposed in this pull request?

testRatisSnapshotRetention assumes no snapshot exists before key writes.
With a snapshot threshold of 1, Ratis may create snapshot.1_0 while initializing the pipeline, causing the assertion to race with snapshot creation.

Recent failure: https://github.com/apache/ozone/actions/runs/35998440323/job/107633673581

This change records the last applied index before each write batch and waits
until a newer snapshot exists and the retained snapshot count is within the
expected range.

What is the link to the Apache JIRA

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

How was this patch tested?

CI: https://github.com/F64116045/ozone/actions/runs/36033043119
CI: https://github.com/F64116045/ozone/actions/runs/36343127301
Repeated TestContainerStateMachine#testRatisSnapshotRetention 10 times locally.

@adoroszlai
adoroszlai requested a review from szetszwo September 24, 2026 17:29
@F64116045
F64116045 marked this pull request as ready for review September 25, 2026 06:51
@Russole
Russole self-requested a review September 27, 2026 01:47
Russole
Russole reviewed Sep 27, 2026
View reviewed changes
Comment thread ...gration-test/src/test/java/org/apache/hadoop/ozone/client/rpc/TestContainerStateMachine.java Outdated
Comment on lines +226 to +234
private static void waitForSnapshotAfter(SimpleStateMachineStorage storage,
long index) throws Exception {
CheckedSupplier<Boolean, IOException> snapshotAdvanced = () -> {
SingleFileSnapshotInfo snapshot =
StatemachineImplTestUtil.findLatestSnapshot(storage);
return snapshot != null && snapshot.getTermIndex().getIndex() > index;
};
GenericTestUtils.waitFor(snapshotAdvanced, 100, 30_000);
}

@Russole Russole Sep 27, 2026

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.

waitForSnapshotAfter only waits for one new snapshot, while the following assertion requires the retained snapshot count to be within range. Could we include the count check in the polling condition to avoid a timing-dependent failure?

@F64116045 F64116045 Sep 28, 2026

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, updated the polling condition

Russole
Russole reviewed Sep 27, 2026
View reviewed changes

@Russole Russole 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 @F64116045 for working on this. Left some comments.

@F64116045
F64116045 marked this pull request as draft September 27, 2026 13:04
@F64116045
F64116045 marked this pull request as ready for review September 28, 2026 07:52
peterxcli
peterxcli reviewed Sep 28, 2026
View reviewed changes
Comment thread ...gration-test/src/test/java/org/apache/hadoop/ozone/client/rpc/TestContainerStateMachine.java
Comment on lines +220 to +222
return files != null
&& Math.abs(retainedSnapshots - files.length) <= 1;
};

@peterxcli peterxcli Sep 28, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Math.abs(retainedSnapshots - files.length) <= 1;

this should be moved to outside and do the assertion as original code do

@F64116045 F64116045 Sep 30, 2026

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. I may be missing something. If this is moved out, it can return as soon as the new snapshot appears, while old snapshots may not have been deleted yet.
I think that’s the case @Russole was concerned about.
Just wonder is there something that guarantees it is safe to move out?

smengcl
smengcl approved these changes Oct 7, 2026
View reviewed changes

@smengcl smengcl 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 @F64116045

@smengcl
smengcl merged commit 891b158 into apache:master Oct 7, 2026
42 of 46 checks passed
@smengcl

smengcl commented Oct 7, 2026

Copy link
Copy Markdown
Contributor

Thanks @F64116045 for the patch. Thanks @Russole @peterxcli for the reviews.

@F64116045

F64116045 commented Oct 8, 2026

Copy link
Copy Markdown
Contributor Author

Thanks @Russole @peterxcli @smengcl .

errose28 added a commit to errose28/ozone that referenced this pull request Oct 8, 2026
@errose28
Merge branch 'master' into HDDS-14496-zdu
4ef88e7 
* master: (23 commits)
  HDDS-15951. Reduce ofs getFileChecksum to a single OM RPC (apache#11380)
  HDDS-16746. Local Stream onCommand should drain buffers before commit PutBlock (apache#11427)
  HDDS-16781. Fix intermittent ReadChunk count mismatch in TestKeyInputStream (apache#11443)
  HDDS-16779. FSO rename of a file to itself should be a no-op (apache#11440)
  HDDS-16724. Add support for missing S3 apis to STS (apache#11413)
  HDDS-16620. Expose BlockReadCursor as an iterator of streaming ReadBlock ranges (apache#11442)
  HDDS-16624. Improve AbortIncompleteMultipartUpload lifecycle validation error message (apache#11344)
  HDDS-16367. Update threat model with information about internal protocols (apache#11185)
  HDDS-16304. Convert RpcMetrics latency counters to lock-free ConcurrentMutableStat (apache#11425)
  HDDS-15681. Switch to io.github.ascopes:protobuf-maven-plugin (apache#11429)
  HDDS-16258. Refactor streaming block reads and fix checksum verification for variable-sized chunks (apache#11302)
  HDDS-16598. Fix intermittent failure in TestContainerStateMachine (apache#11319)
  HDDS-15802. Add integration tests for per-volume push replication (apache#11146)
  HDDS-16657. Reject lifecycle Transition actions in PutBucketLifecycleConfiguration (apache#11430)
  HDDS-16714. Split kubernetes check by environment (apache#11406)
  HDDS-16752. Bump guava to 33.7.2-jre (apache#11433)
  HDDS-16581. Bump ozone-runner to 20260922-1 (apache#11431)
  HDDS-16611. Use MutableGaugeLong for total count metrics in OMMetrics (apache#11410)
  HDDS-16738. Let dependabot ignore jetty minor version upgrades (apache#11422)
  HDDS-16683. Upgrade Jetty from 12.0.38 to 12.1.14 (apache#11424)
  ...

Conflicts:
hadoop-hdds/common/src/main/java/org/apache/hadoop/ozone/OzoneManagerVersion.java
hadoop-hdds/container-service/src/test/java/org/apache/hadoop/ozone/container/keyvalue/TestKeyValueHandler.java
hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/container/replication/ReplicationManager.java
hadoop-hdds/server-scm/src/test/java/org/apache/hadoop/hdds/scm/container/replication/TestReplicationManager.java
hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/file/OMFileCreateRequestWithFSO.java
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@peterxcli peterxcli peterxcli left review comments
@Russole Russole Russole left review comments
@smengcl smengcl smengcl approved these changes
@szetszwo szetszwo Awaiting requested review from szetszwo

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@F64116045 @smengcl @peterxcli @Russole