HDDS-16081. TestOMRatisSnapshots fails due to leader applied index taken before flush

[ss77892]

What changes were proposed in this pull request?

Flaky TestOMRatisSnapshots#testInstallSnapshotWithClientWrite: follower may not have all keys when the leader's last applied index is reached

The test starts a stopped follower OM, writes 200 keys while it catches up, waits for the follower to reach the leader, then reads the follower's RocksDB directly to confirm every key arrived. Sometimes the last few keys aren't there yet. That happens because there is a small gap in which a write has already been acked to the client (and committed to the Ratis log) but not yet flushed to RocksDB. The test relies on the leader's lastAppliedTermIndex, which only advances on flush, so the target it hands the follower can sit behind writes that already returned. Previously, a 5-second sleep made the flush all but certain by the time that index was read. HDDS-10310 replaced it with an index-based wait and dropped the per-key sleeps from the write loop, so the writes now outrun the flush and nothing closes the gap, leaving the test to check the follower's DB for keys that are still queued.
The waits now target the leader's Ratis commit index, which covers every acked write. Three tests change:

• testInstallSnapshotWithClientWrite: target was the leader's applied index.
• testInstallSnapshotWithClientRead: target was the persisted snapshot index. The commit index is captured after the TransactionInfo read, so it is never behind it, and the existing wait can simply be retargeted. leaderOMSnapshotIndex stays in the checkpoint assertions, which is what it is actually for.
• testInstallOldCheckpointFailure: same wrong target on a wait whose stated goal is "no in-flight transactions left".

What is the link to the Apache JIRA

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

How was this patch tested?

To reproduce it for sure, a small delay (1 ms) has been added to OzoneManagerDoubleBuffer#flush. The testInstallSnapshotWithClientWrite test fails often in this case.

[ivandika3]

Thanks @ss77892 for the patch.

[ivandika3]

Is this the same workaround of #10748 (i.e. flush the double buffer so that the key is visible)? If that's so, I don't agree with this as mentioned in that ticket since this means the OM linearizability can be violated. We should not need to wait for double buffer flush for the key to be visible. We should fix the root cause, not just trying to pass the tests. Feel free to correct me if I'm mistaken about the OM consistency guarantee since it's never properly expounded or proven by the original author (https://ozone.apache.org/docs/1.4.1/design/omha.html).

The only time manually awaitDoubleBufferFlush is valid is for OMResponse unit test, not in integration tests. I saw a lot of awaitDoubleBufferFlush(or worse, sleep workaround) in integration test which is worrying.

But if it's because of the appliedIndex check

// The recently started OM should be lagging behind the leader OM.
    // Wait & for follower to update transactions to leader snapshot index.
    // Timeout error if follower does not load update within 3s
    GenericTestUtils.waitFor(() -> {
      return followerOM.getOmRatisServer().getLastAppliedTermIndex().getIndex()
          >= leaderOMSnapshotIndex - 1;
    }, 100, 30_000);

I might be fine with it. In that case, please move the awaitDoubleBufferFlush after the key visibility check.

[ss77892]

@ivandika3 thank you for your review, sir! This isn't the #10748 case. Nothing here waits for a flush to make a key visible. TypedTable.get() checks the cache first, so a key is readable as soon as the write returns.
The bug is about the wait target. The test waited for the follower's applied index to reach the leader's applied index, but that index only moves when the double buffer flushes. The correct target should be Ratis commit index. Writes are acked only after their entries are committed, so this index should cover all of them.
I also fixed two other tests for consistency's sake.