Create configureReadReplication and deprecate ensureReplicas/disableReplicas

[jtolio]

No description provided.

[ask-bonk]

I'm Bonk, and I've done a quick review of your PR.

This PR adds configureReadReplication() to DurableObjectStorage as a replacement for ensureReplicas()/disableReplicas().

Issues

1. Unused traceContext (medium): The trace span is created but never attached to the promise, so tracing won't cover this operation. Every other storage method in this file attaches its traceContext via context.attachSpans() or .attach().

2. No tests (medium): No tests were added for the new method. Even though the backend implementation is behind a virtual interface, it would be valuable to have at least a basic test that validates argument parsing (unknown mode, missing mode, replica rejection).

3. Error message omits the rejected value (low): Including the actual mode string in the "unknown mode" error would aid debugging.

[ask-bonk]

Review posted successfully on PR #6555. Here's a summary of what I found:

1. Unused traceContext (medium): The trace span at actor-state.c++:894 is created but never attached to the returned promise. Every other storage method in the file uses context.attachSpans() or .attach() to keep the span alive through the I/O operation. I posted a suggestion to fix this by wrapping the return with context.attachSpans().

2. No tests (medium): No tests were added for the new configureReadReplication() method. Argument parsing validation (unknown mode, missing mode, replica rejection) would be good to test.

3. Error message omits the rejected value (low): The "unknown mode" error at line 905 doesn't include the actual value passed, which would help debugging. I posted a suggestion using kj::str() to interpolate it.

github run

[codspeed-hq]

Merging this PR will improve performance by 41.01%

⚠️ Different runtime environments detected

Some benchmarks with significant performance changes were compared across different runtime environments,
which may affect the accuracy of the results.

Open the report in CodSpeed to investigate

⚡ 1 improved benchmark
✅ 71 untouched benchmarks
⏩ 129 skipped benchmarks¹

Performance Changes

	Benchmark	BASE	HEAD	Efficiency
⚡	simpleStringBody[Response]	27.2 µs	19.3 µs	+41.01%

---

_{Comparing jolio/add-configure-read-replication (e14caa5) with main (22c1ca0)}

Footnotes

1. 129 benchmarks were skipped, so the baseline results were used instead. If they were deleted from the codebase, click here and archive them to remove them from the performance reports. ↩

[jtolio]

trivial rebase ^

[codecov-commenter]

Codecov Report

❌ Patch coverage is 0% with 23 lines in your changes missing coverage. Please review.
✅ Project coverage is 70.84%. Comparing base (22c1ca0) to head (e14caa5).

Files with missing lines	Patch %	Lines
src/workerd/api/actor-state.c++	0.00%	21 Missing ⚠️
src/workerd/io/actor-cache.h	0.00%	2 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #6555      +/-   ##
==========================================
- Coverage   70.85%   70.84%   -0.01%     
==========================================
  Files         438      438              
  Lines      123642   123663      +21     
  Branches    19455    19458       +3     
==========================================
+ Hits        87604    87610       +6     
- Misses      24514    24532      +18     
+ Partials    11524    11521       -3     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
• 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.