Convert Redwood page readers to standard coroutines

[tclinkenbeard-oai]

Summary

• Convert DWALPager::readPhysicalExtent and VersionedBTree::readPage from Flow actors to C++ coroutines.
• Preserve extent-read throttling, disk-read scheduling, page-read metrics, and the existing actor-compiled Redwood tests.

Validation

• clang-format-19 --dry-run --Werror fdbserver/kvstore/VersionedBTree.actor.cpp
• fdbserver_kvstore_test -f /redwood/correctness/btreeCloseWithQueuedCommits --seed 424242
• fdbserver_kvstore_test -f /redwood/correctness/btreeCloseWithQueuedCommits --seed 424242 --simulation
• fdbserver_kvstore_test -f /redwood/correctness --seed 424242

[tclinkenbeard-oai]

Generated by Codex.

What is it trying to do?

Convert DWALPager::readPhysicalExtent and VersionedBTree::readPage from Flow actors to standard C++ coroutines while preserving Redwood’s existing page-read behavior and actor-compiled tests.

Is it correct?

The conversion preserves the original suspension points, disk-read priority, parallel-read ordering, page-reference lifetimes, error propagation, metrics, and extent-read throttling. The extent lock remains released by its existing consumer rather than by the converted coroutine.

auto extent still deduces Reference<ArenaPage>, and co_return page implicitly moves the local through FoundationDB’s coroutine promise. Existing actorcompiler scaffolding remains intact for the actor-style tests in the same file.

Public clang-format and clang-tidy checks have passed. Windows, FoundationDB builder variants, and cluster tests are still pending; no checks are currently failing.

This review was based on source inspection and public PR information only. I did not run builds, tests, or simulations for this review.

Are there bugs?

I did not find any correctness bugs.

Are there omissions?

There is no blocking omission. The validation commands listed in the PR do not explicitly select the existing Lredwood/correctness/btree or :/redwood/performance/extentQueue cases because the unit-test runner matches literal prefixes. Direct coverage of extent-read throttling, partial extents, and lock handoff is therefore the main remaining testing risk.

Are there better ways of doing things?

As an optional cleanup, readPage could assign each awaited result directly to page:

page = co_await snapshot->getPhysicalPage(...);

The same simplification applies to the multipage branch and would remove the intermediate p variables and explicit std::move(p) calls. This is not required for correctness.

Should this CL be LGTMd?

Yes, LGTM based on source inspection. Wait for the pending FoundationDB builders, Windows check, and cluster tests before merging.

[foundationdb-ci]

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

• Commit ID: de8833b
• Duration 0:29:17
• 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: de8833b
• Duration 0:47:34
• 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: de8833b
• Duration 1:07:00
• 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: de8833b
• Duration 1:13:10
• 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: de8833b
• Duration 1:22:09
• 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: de8833b
• Duration 1:28:54
• 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)

[foundationdb-ci]

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

• Commit ID: de8833b
• Duration 4:13:14
• Result: ❌ FAILED
• Error: `Error while executing command: # compatible with bash and zsh
  ssh -o StrictHostKeyChecking=no -o UserKnownHostsFile=/dev/null -i ${HOME}/.ssh_key
  -o ServerAliveInterval=9 ec2-user@${MAC_EC2_HOST} "
  set -e -o noclobber
  BUILDNUM=${CODEBUILD_BUILD_NUMBER}
  WAIT_TIME=14400 # 4 hours
  LOCK_STALE=4500 # 75 minutes
  LOCK=/tmp/ci.lock
  "'

macOS stat flags, not linux compatible

lock_mtime() { stat -f %m $LOCK || echo 0 }
I=0
while ! (echo $BUILDNUM >$LOCK) 2>/dev/null ; do
if (( ++I >= WAIT_TIME )); then
echo "timeout waiting for ci.lock after $I seconds"
exit 1
fi
if (( $(date +%s) - $(lock_mtime) >= LOCK_STALE )); then
echo "lock expired after $LOCK_STALE seconds ..."
# racy but sleep so chances are low
sleep $(( 3 + RANDOM % 8 ))
if (( $(date +%s) - $(lock_mtime) >= LOCK_STALE )); then
echo "lock holder is gone"
rm -f $LOCK
else
echo "new lock holder"
fi
fi
sleep 1
done
echo "acquired ci.lock"
'
. Reason: exit status 1`

• Build Log terminal output (available for 30 days)
• Build Workspace zip file of the working directory (available for 30 days)