Finish ReadYourWrites.actor.cpp coroutine conversion#13377
Finish ReadYourWrites.actor.cpp coroutine conversion#13377tclinkenbeard-oai wants to merge 2 commits into
ReadYourWrites.actor.cpp coroutine conversion#13377Conversation
tclinkenbeard-oai
left a comment
There was a problem hiding this comment.
Generated by Codex.
What is it trying to do?
Finish converting ReadYourWrites.actor.cpp to standard C++ coroutines, remove the actor compiler include, rename it to ReadYourWrites.cpp, and update design-doc references.
Is it correct?
The code appears correct by inspection. waitOrError preserves the original read-first choose ordering and reset error propagation. The iterator locals remain alive across co_await, and conflict-range insertion stays on the successful-read path. The removed trailing co_return; statements are equivalent to falling off the end of Future<Void> coroutines.
No serialized messages, durable formats, protocol boundaries, knobs, or status structures changed. The source glob picks up the renamed .cpp file.
No build or test validation was run. At review time, clang-format is green; Windows Boost CONFIG and FoundationDB CI builder/clang/clang-arm/clang-ide/macOS/cluster checks are still pending or in progress, with no visible failures.
Are there bugs?
I did not find any correctness bugs.
Are there omissions?
AGENTS.md:75 still references ReadYourWrites.actor.cpp; it should be updated to ReadYourWrites.cpp. This is documentation-only and does not block the change.
Are there better ways of doing things?
Update the remaining AGENTS.md filename reference in this PR. Otherwise, waitOrError is preferable to duplicating custom race/error handling in each coroutine.
Should this CL be LGTMd?
Yes, LGTM based on code inspection. I inspected the patch, waitOrError, reset completion paths, coroutine-local lifetimes, source discovery, and existing mapped-range coverage. The highest remaining risk is unvalidated constant-factor coroutine/wrapper overhead and reset-race behavior; merge should still wait for the pending CI checks.
Result of foundationdb-pr-clang-ide on Linux RHEL 9
|
Result of foundationdb-pr-macos-m1 on macOS 14.x
|
Result of foundationdb-pr-clang-arm on Linux RHEL 9
|
Result of foundationdb-pr-clang on Linux RHEL 9
|
Result of foundationdb-pr on Linux RHEL 9
|
Result of foundationdb-pr-cluster-tests on Linux RHEL 9
|
Result of foundationdb-pr-macos on macOS 14.x
|
Result of foundationdb-pr-clang on Linux RHEL 9
|
Result of foundationdb-pr on Linux RHEL 9
|
This PR uses the actor rewrite tool and the
waitOrErrorgeneric actor to finish theReadYourWrites.actor.cppcoroutine conversion