Skip to content

Move experimental configureReadReplication from ctx.storage to ctx.#6649

Merged
jtolio merged 1 commit intomainfrom
jolio/move-config-read-replication
Apr 24, 2026
Merged

Move experimental configureReadReplication from ctx.storage to ctx.#6649
jtolio merged 1 commit intomainfrom
jolio/move-config-read-replication

Conversation

@jtolio
Copy link
Copy Markdown
Member

@jtolio jtolio commented Apr 23, 2026

Before read replication becomes generally available, make the API more friendly by moving configureReadReplication up. Logically, read replication is more a property of the DO itself than the storage subsystem.

Should be retargeted at main once #6663 is merged

@jtolio jtolio requested review from a team as code owners April 23, 2026 19:54
Comment thread src/workerd/api/actor-state.h Outdated
@ask-bonk
Copy link
Copy Markdown
Contributor

ask-bonk Bot commented Apr 23, 2026

This PR is clean. Let me summarize my analysis:

  1. C++ change: Removes configureReadReplication from DurableObjectStorage's JSG registration, adds it to DurableObjectState with a thin wrapper that delegates to the underlying storage. The C++ method on DurableObjectStorage is preserved (not deleted), only the JS binding is moved.

  2. Backward compatibility: Not a concern. The replicaRouting compat flag is $experimental, so this API hasn't been exposed in any stable release. Moving it before GA is exactly the right time.

  3. Safety: The JSG_REQUIRE_NONNULL(storage, ...) guard is correct since storage is kj::Maybe. The delegation with kj::mv(options) is proper for the struct parameter.

  4. Type snapshots: Correctly reflect the move - configureReadReplication added to DurableObjectState, removed from DurableObjectStorage, only in experimental/ (not latest/).

  5. No new issues found: The code is a straightforward, well-structured API move.

LGTM

github run

@codspeed-hq
Copy link
Copy Markdown

codspeed-hq Bot commented Apr 23, 2026

Merging this PR will not alter performance

✅ 72 untouched benchmarks
⏩ 129 skipped benchmarks1


Comparing jolio/move-config-read-replication (3a820d2) with jolio/add-ctx-primaryStub (4e7ffab)2

Open in CodSpeed

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.

  2. No successful run was found on jolio/add-ctx-primaryStub (82d96dc) during the generation of this report, so dce322d was used instead as the comparison base. There might be some changes unrelated to this pull request in this report.

@jtolio jtolio force-pushed the jolio/move-config-read-replication branch 2 times, most recently from 2ae88e3 to f86c757 Compare April 23, 2026 21:56
@jtolio jtolio requested a review from a team as a code owner April 23, 2026 21:56
@jtolio jtolio requested a review from jamesopstad April 23, 2026 21:56
@jtolio jtolio force-pushed the jolio/move-config-read-replication branch 2 times, most recently from f86c757 to 2684b54 Compare April 23, 2026 22:03
@jtolio jtolio requested a review from justin-mp April 23, 2026 22:46
Comment thread src/workerd/api/actor-state.h Outdated
Comment thread src/workerd/api/actor-state.h Outdated
@jtolio jtolio force-pushed the jolio/move-config-read-replication branch 3 times, most recently from 950bce5 to d668e2b Compare April 24, 2026 14:38
@jtolio jtolio changed the base branch from main to jolio/add-ctx-primaryStub April 24, 2026 14:42
@jtolio jtolio force-pushed the jolio/add-ctx-primaryStub branch from 5c04e43 to 4e7ffab Compare April 24, 2026 15:27
@jtolio jtolio force-pushed the jolio/move-config-read-replication branch from d668e2b to 0649f75 Compare April 24, 2026 15:27
Comment thread src/workerd/api/actor-state.c++ Outdated
Before read replication becomes generally available, make the
API more friendly by moving configureReadReplication up. Logically,
read replication is more a property of the DO itself than the
storage subsystem.

X-Dest-Branch: jolio/move-config-read-replication
@jtolio jtolio force-pushed the jolio/add-ctx-primaryStub branch from 4e7ffab to 82d96dc Compare April 24, 2026 18:23
@jtolio jtolio force-pushed the jolio/move-config-read-replication branch from 0649f75 to 3a820d2 Compare April 24, 2026 18:23
@codecov-commenter
Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 0% with 25 lines in your changes missing coverage. Please review.
⚠️ Please upload report for BASE (jolio/add-ctx-primaryStub@82d96dc). Learn more about missing BASE report.

Files with missing lines Patch % Lines
src/workerd/api/actor-state.c++ 0.00% 25 Missing ⚠️
Additional details and impacted files
@@                     Coverage Diff                      @@
##             jolio/add-ctx-primaryStub    #6649   +/-   ##
============================================================
  Coverage                             ?   66.50%           
============================================================
  Files                                ?      405           
  Lines                                ?   115976           
  Branches                             ?    19409           
============================================================
  Hits                                 ?    77133           
  Misses                               ?    27265           
  Partials                             ?    11578           

☔ 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.

Base automatically changed from jolio/add-ctx-primaryStub to main April 24, 2026 18:59
@jtolio jtolio merged commit f7557ed into main Apr 24, 2026
23 of 38 checks passed
@jtolio jtolio deleted the jolio/move-config-read-replication branch April 24, 2026 19:02
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants