Preserve DD maintenance start versions across transaction retries

[tclinkenbeard-oai]

Summary

• Preserve the first observed maintenance-window start version when a transaction must be retried.
• Adopt an already committed start after ambiguous commits, concurrent writers, or data-distributor recruitment.
• Retry stale-start cleanup when maintenance ends while preserving the existing maintenance tracing and durable storage format.
• Cover failed starts, ambiguous commits, competing writers, failed clears, changed maintenance zones, and recruitment with six focused regression tests.

Testing

• GCC 13, -DUSE_WERROR=ON -DWITH_ROCKSDB=OFF: fdbserver_datadistributor_test -f /DataDistribution/Maintenance/ - 6 passed, 0 failed.
• GCC 13, -DUSE_WERROR=ON -DWITH_ROCKSDB=OFF: fdbserver_datadistributor_test - 56 passed, 0 failed.
• clang-format 19 and git diff --check passed.

[tclinkenbeard-oai]

Generated by Codex.

What is it trying to do?

Preserve maintenance-window start versions and accurate end-duration telemetry across transaction retries, ambiguous commits, distributor handoffs, primary/remote team collections, and rapidly changing maintenance windows.

Is it correct?

The primary verifies the current move-keys owner in the same transaction before modifying durable maintenance metadata, while remote collections remain read-only and watch both relevant keys. Conservative generation tracking prevents followers from attributing stale starts across replacements, retries, clear-and-restart races, skipped windows, reused zone identifiers, and coalesced window endings.

Ownership-loss clear paths preserve the existing timeout, manual-clear, and storage-server-failure end events without performing unauthorized writes. Persisted encodings remain unchanged, and coroutine-held state has external linkage compatible with GCC.

The PR documents 22 focused tests and 72 complete data-distribution tests passing under both GCC 13 and Clang, plus a passing buggified simulation. This final review was source-only and did not itself execute builds, tests, or simulations.

Are there bugs?

No actionable correctness bugs were found after checking transaction conflicts, ownership transfer, retries, actor cancellation, follower state transitions, coalesced watches, maintenance-duration attribution, terminal event payloads, persisted compatibility, and regression-test effectiveness.

Are there omissions?

None that block this change. The existing persisted value contains only a start version, so completely coalesced histories can remain indistinguishable. The implementation preserves compatibility and returns the existing unknown-duration sentinel when attribution cannot be justified.

Are there better ways of doing things?

Persisting an explicit zone or generation alongside the start version could eliminate additional ambiguity, but would require a broader durable-format migration and compatibility changes. Conservative use of the existing -1 sentinel is appropriate for this narrowly scoped fix.

Should this CL be LGTMd?

Yes, LGTM for source correctness. Allow the current commit's pending required CI checks to complete before merging.

[tclinkenbeard-oai]

Generated by Codex.

What is it trying to do?

Keep a maintenance window's original start version stable across transaction retries, ambiguous commits, competing data-distributor writers, maintenance cleanup, and distributor recruitment without changing existing maintenance telemetry or persisted formats.

Is it correct?

MaintenanceStartVersionState retains the first observed version across retries, recognizes an ambiguously committed start, adopts a competing writer's committed value, and retries stale-start cleanup even after the local zone has been cleared.

Both the maintenance-zone and durable-start keys are read before writes or clears, establishing transaction conflict ranges for concurrent writers. Existing maintenance-end events and durations are preserved. Recruitment adopts a previously persisted start or repairs a missing one, and the durable encoding is unchanged.

The PR reports six focused maintenance tests and 56 complete data-distribution tests passing under GCC 13 with warnings treated as errors. This review was source-only and did not itself run builds, tests, or simulations.

Are there bugs?

I found no actionable correctness bugs after reviewing transaction retries, ambiguous commits, concurrent-writer conflicts, cleanup failures, zone replacement, recruitment, actor lifetimes, event ordering, persisted compatibility, and the six targeted regressions.

Are there omissions?

No blocking omissions. Existing primary and remote team collections retain their original shared-writer behavior, with stronger transaction conflict detection from the added durable-start read. That read adds constant work to an existing watcher transaction without introducing additional watchers or actors.

Are there better ways of doing things?

The compact retry-state class is appropriately scoped. Additional ownership fencing, follower-generation tracking, extra watches, or durable-format changes would make this fix considerably more complicated without being necessary for its stated purpose.

Should this CL be LGTMd?

Yes, LGTM for source correctness. Wait for the new commit's required CI checks to complete before merging.

[foundationdb-ci]

Result of foundationdb-pr-clang-ide on Linux RHEL 9

• Commit ID: 8f63b72
• Duration 0:22:46
• Result: ✅ SUCCEEDED
• Error: N/A
• Build Log terminal output (available for 30 days)
• Build Workspace zip file of the working directory (available for 30 days)

[foundationdb-ci]

Result of foundationdb-pr-macos-m1 on macOS 14.x

• Commit ID: 8f63b72
• Duration 0:38:20
• Result: ✅ SUCCEEDED
• Error: N/A
• Build Log terminal output (available for 30 days)
• Build Workspace zip file of the working directory (available for 30 days)

[foundationdb-ci]

Result of foundationdb-pr-clang-arm on Linux RHEL 9

• Commit ID: 8f63b72
• Duration 0:47:15
• Result: ✅ SUCCEEDED
• Error: N/A
• Build Log terminal output (available for 30 days)
• Build Workspace zip file of the working directory (available for 30 days)

[foundationdb-ci]

Result of foundationdb-pr-clang on Linux RHEL 9

• Commit ID: 8f63b72
• Duration 0:54:26
• Result: ✅ SUCCEEDED
• Error: N/A
• Build Log terminal output (available for 30 days)
• Build Workspace zip file of the working directory (available for 30 days)

[foundationdb-ci]

Result of foundationdb-pr on Linux RHEL 9

• Commit ID: 8f63b72
• Duration 0:58:46
• Result: ✅ SUCCEEDED
• Error: N/A
• Build Log terminal output (available for 30 days)
• Build Workspace zip file of the working directory (available for 30 days)

[foundationdb-ci]

Result of foundationdb-pr-macos on macOS 14.x

• Commit ID: 8f63b72
• Duration 1:08:02
• Result: ✅ SUCCEEDED
• Error: N/A
• Build Log terminal output (available for 30 days)
• Build Workspace zip file of the working directory (available for 30 days)

[foundationdb-ci]

Result of foundationdb-pr-cluster-tests on Linux RHEL 9

• Commit ID: 8f63b72
• Duration 1:25:46
• Result: ✅ SUCCEEDED
• Error: N/A
• Build Log terminal output (available for 30 days)
• Build Workspace zip file of the working directory (available for 30 days)
• Cluster Test Logs zip file of the test logs (available for 30 days)