lhumina_code/hero_rpc
1
0
Fork 0
Code Issues 18 Pull requests Projects Releases 2 Packages Wiki Activity Actions

fix(osis): make CRUD handlers async — drop spawn_blocking and PR #130 blocking shims #133

Merged
timur merged 2 commits from osis-handlers-async into development 2026-05-22 13:57:48 +00:00
Conversation 0 Commits 2 Files changed 8 +419 -268
timur commented 2026-05-22 13:57:03 +00:00
Owner
Copy link

Why

Replaces #130 (which is being closed in favor of this architectural fix). hero_rpc#127 wired OSIS handler bodies through an OsisIndexer facade that held its own tokio::runtime::Runtime and block_oned every hero_indexer_sdk call. The moment a generated handler fired from inside the jsonrpsee server's worker, that nested block_on panicked: "Cannot start a runtime from within a runtime."

#130 hid the panic behind a thread-pool detour shim (index_document_blocking + a bridge runtime). That worked but was a hack — every CRUD path still crossed sync<->async twice per request.

This PR fixes the root cause: every link in the chain is async, so the dispatch pipeline is one continuous .await stack from the network closure to the indexer socket.

Before / after

Before (post-#127, panicked):
  RPC server (async, jsonrpsee)
    -> tokio::task::spawn_blocking           [1st boundary]
       OSIS handler (SYNC)
         -> indexer.index_document() (sync facade)
            -> Handle::block_on(async sdk)   [2nd boundary, PANIC]

With #130's shim (hack):
  RPC server (async, jsonrpsee)
    -> tokio::task::spawn_blocking           [1st boundary]
       OSIS handler (SYNC)
         -> indexer.index_document_blocking()
            -> bridge_runtime().spawn(async)
               -> std::mpsc recv             [2nd boundary, ugly]

This PR (ideal):
  RPC server (async, jsonrpsee)
    -> handler.handle_rpc_call_with_context().await   [direct .await]
       OSIS handler (ASYNC)
         -> indexer.index_document().await            [direct .await]

Zero sync<->async crossings inside any CRUD handler. The legacy Axum-based dispatch in osis/src/rpc/server.rs keeps its spawn_blocking shells around the legacy OsisRpcHandler trait surface (still sync — out of scope), and the modern hot path via rpc2_adapter uses register_async_method directly.

What changed

  • crates/osis/src/index/remote.rs — OsisIndexer fully async (tokio::sync::OnceCell + get_or_try_init). Public methods index_document / delete_document / search / try_initialize are async fn. The owned Runtime + bridge-runtime shims are gone.
  • crates/generator/src/rust/rust_osis.rs — emits every CRUD method (_new, _get, _set, _delete, _list, _list_full, _exists, _find) as async fn; same for the _rpc_* dispatchers, handle_rpc, handle_rpc_call, handle_service_call. Indexer write-through inside _new/_set/_delete becomes .await.
  • crates/osis/src/rpc/server.rs — OsisAppRpcHandler trait becomes async via RPITIT (impl Future + Send). OsisAppWrapper's OsisRpcHandler bridge uses Handle::current().block_on(...) — safe because the legacy dispatch calls it from inside spawn_blocking.
  • crates/osis/src/rpc/rpc2_adapter.rs — switches from register_method to register_async_method and awaits the trait call directly. Zero spawn_blocking on the CRUD hot path.
  • crates/osis/tests/indexer_smoke.rs — async smoke test (#[tokio::test]). The *_blocking shim regression test is gone.
  • crates/osis_benches/benches/index_perf.rs — bench-owned tokio::runtime::Runtime drives the now-async CRUD methods via block_on (criterion harness fns can't be async fn). Reverts the #130 server-wrapping fixture back to the in-process tempdir model that existed pre-#130.
  • examples/walkthrough/src/main.rs — hand-written OsisAppRpcHandler impl marked async to match the new trait shape.

What stays

  • Legacy OsisRpcHandler trait in crates/osis/src/rpc/handler.rs stays sync — modern production code goes through rpc2_adapter and never crosses it.
  • spawn_blocking around the legacy handler.handle_request sites in server.rs (lines ~905/992/1401/1870) stays — bridges async Axum router to the sync legacy trait.
  • spawn_blocking around service-method dispatch (lines ~1443/1920) stays. User-implemented service trait methods may legitimately be sync.

Validation

  • cargo build --workspace — clean.
  • cargo test -p hero_rpc_osis --features rpc --lib --tests — 85 lib + 1 smoke, all pass. Smoke degrades gracefully when hero_indexer is unreachable.
  • cargo test -p hero_rpc_generator --lib — 140 generator self-tests pass.
  • cargo test -p hero_rpc2 --lib — 11 hero_rpc2 tests pass.
  • Pre-existing 35 test-build failures on dev (reqwest examples, missing recipe_server schemas, etc.) unchanged.

Release bench numbers weren't captured (validation worktree disk filled). Expect a small win vs #130 from the dropped spawn_blocking overhead.

Closeout

  • Closes the regression hero_rpc#127 introduced.
  • Replaces #130 (closed; this PR's history references that branch's remote.rs as salvage starting point).
## Why Replaces #130 (which is being closed in favor of this architectural fix). hero_rpc#127 wired OSIS handler bodies through an `OsisIndexer` facade that held its own `tokio::runtime::Runtime` and `block_on`ed every `hero_indexer_sdk` call. The moment a generated handler fired from inside the jsonrpsee server's worker, that nested `block_on` panicked: "Cannot start a runtime from within a runtime." #130 hid the panic behind a thread-pool detour shim (`index_document_blocking` + a bridge runtime). That worked but was a hack — every CRUD path still crossed sync<->async twice per request. This PR fixes the root cause: every link in the chain is async, so the dispatch pipeline is one continuous `.await` stack from the network closure to the indexer socket. ## Before / after ``` Before (post-#127, panicked): RPC server (async, jsonrpsee) -> tokio::task::spawn_blocking [1st boundary] OSIS handler (SYNC) -> indexer.index_document() (sync facade) -> Handle::block_on(async sdk) [2nd boundary, PANIC] With #130's shim (hack): RPC server (async, jsonrpsee) -> tokio::task::spawn_blocking [1st boundary] OSIS handler (SYNC) -> indexer.index_document_blocking() -> bridge_runtime().spawn(async) -> std::mpsc recv [2nd boundary, ugly] This PR (ideal): RPC server (async, jsonrpsee) -> handler.handle_rpc_call_with_context().await [direct .await] OSIS handler (ASYNC) -> indexer.index_document().await [direct .await] ``` Zero sync<->async crossings inside any CRUD handler. The legacy Axum-based dispatch in `osis/src/rpc/server.rs` keeps its `spawn_blocking` shells around the legacy `OsisRpcHandler` trait surface (still sync — out of scope), and the modern hot path via `rpc2_adapter` uses `register_async_method` directly. ## What changed - **`crates/osis/src/index/remote.rs`** — `OsisIndexer` fully async (`tokio::sync::OnceCell` + `get_or_try_init`). Public methods `index_document` / `delete_document` / `search` / `try_initialize` are `async fn`. The owned `Runtime` + bridge-runtime shims are gone. - **`crates/generator/src/rust/rust_osis.rs`** — emits every CRUD method (`_new`, `_get`, `_set`, `_delete`, `_list`, `_list_full`, `_exists`, `_find`) as `async fn`; same for the `_rpc_*` dispatchers, `handle_rpc`, `handle_rpc_call`, `handle_service_call`. Indexer write-through inside `_new`/`_set`/`_delete` becomes `.await`. - **`crates/osis/src/rpc/server.rs`** — `OsisAppRpcHandler` trait becomes async via RPITIT (`impl Future + Send`). `OsisAppWrapper`'s `OsisRpcHandler` bridge uses `Handle::current().block_on(...)` — safe because the legacy dispatch calls it from inside `spawn_blocking`. - **`crates/osis/src/rpc/rpc2_adapter.rs`** — switches from `register_method` to `register_async_method` and awaits the trait call directly. Zero `spawn_blocking` on the CRUD hot path. - **`crates/osis/tests/indexer_smoke.rs`** — async smoke test (`#[tokio::test]`). The `*_blocking` shim regression test is gone. - **`crates/osis_benches/benches/index_perf.rs`** — bench-owned `tokio::runtime::Runtime` drives the now-async CRUD methods via `block_on` (criterion harness fns can't be `async fn`). Reverts the #130 server-wrapping fixture back to the in-process tempdir model that existed pre-#130. - **`examples/walkthrough/src/main.rs`** — hand-written `OsisAppRpcHandler` impl marked async to match the new trait shape. ## What stays - Legacy `OsisRpcHandler` trait in `crates/osis/src/rpc/handler.rs` stays sync — modern production code goes through `rpc2_adapter` and never crosses it. - `spawn_blocking` around the legacy `handler.handle_request` sites in `server.rs` (lines ~905/992/1401/1870) stays — bridges async Axum router to the sync legacy trait. - `spawn_blocking` around service-method dispatch (lines ~1443/1920) stays. User-implemented service trait methods may legitimately be sync. ## Validation - `cargo build --workspace` — clean. - `cargo test -p hero_rpc_osis --features rpc --lib --tests` — 85 lib + 1 smoke, all pass. Smoke degrades gracefully when hero_indexer is unreachable. - `cargo test -p hero_rpc_generator --lib` — 140 generator self-tests pass. - `cargo test -p hero_rpc2 --lib` — 11 hero_rpc2 tests pass. - Pre-existing 35 test-build failures on dev (reqwest examples, missing recipe_server schemas, etc.) unchanged. Release bench numbers weren't captured (validation worktree disk filled). Expect a small win vs #130 from the dropped `spawn_blocking` overhead. ## Closeout - Closes the regression hero_rpc#127 introduced. - Replaces #130 (closed; this PR's history references that branch's remote.rs as salvage starting point). 
OsisIndexer's previous design held a private tokio::runtime::Runtime and
block_on'ed every hero_indexer_sdk call so that sync OSIS handler bodies
could call into it. That panicked with "Cannot start a runtime from
within a runtime" the moment a generated handler (or a #[tokio::test])
called into it from inside the jsonrpsee server's worker — the root
cause of hero_rpc#127's regression.

This change:

- Drops the owned Runtime + AtomicBool init flag; ensure_initialized is
  now a tokio::sync::OnceCell<Arc<HeroIndexAPIClient>> driven through
  get_or_try_init.
- Makes index_document / delete_document / search / try_initialize
  async fn; consumers await directly.
- Smoke test moves to #[tokio::test] with .await.
- Bench file gets a bench-owned Runtime (legitimate — criterion benches
  can't be async fn) used only at wire-arm setup (run_for_test +
  try_initialize). The bench cases themselves still operate against the
  in-process OsisBench fixture.

Generator-emitted handler bodies still call the sync facade — that gets
replaced in the next commit, which makes the generated <root>_new /
_set / _delete / _find methods async fn so they .await the indexer
directly.
fix(osis): make CRUD handlers async — drop spawn_blocking + nested-runtime bridge
Some checks failed
Test / test (push) Failing after 2m9s
Details
Test / test (pull_request) Failing after 2m30s
Details
fe22df2ee0
 
Before this change the dispatch pipeline crossed sync<->async boundaries
twice and panicked at the second crossing whenever a write-through to
the hero_indexer fired from inside the jsonrpsee server's worker:

  RPC server (async)
    -> tokio::task::spawn_blocking            <-- 1st boundary
       OSIS handler (SYNC)
         -> indexer.index_document() (sync facade)
            -> Handle::block_on(async sdk)    <-- 2nd boundary, PANIC

PR #130 hid the panic behind a thread-pool detour shim
(`index_document_blocking` etc.) that crossed the same boundary on a
worker thread the calling runtime didn't own. That worked but was a
hack — every CRUD path still made two unnecessary sync<->async hops
per request.

This change makes the handler async end-to-end:

  RPC server (async)
    -> handler.handle_rpc_call_with_context().await   <-- direct await
       OSIS handler (ASYNC)
         -> indexer.index_document().await            <-- direct await

Concretely:

- `crates/generator/src/rust/rust_osis.rs` emits every CRUD method
  (`_new` / `_get` / `_set` / `_delete` / `_list` / `_list_full` /
  `_exists` / `_find`) and the JSON-RPC dispatcher arms (`_rpc_*`,
  `handle_rpc`, `handle_rpc_call`, `handle_service_call`) as
  `async fn`. Indexer write-through is now `.await` not a sync facade
  call.
- `OsisAppRpcHandler` (the public trait every codegen-emitted impl
  satisfies) becomes async via RPITIT (`impl Future + Send`).
- `crates/osis/src/rpc/rpc2_adapter.rs` switches from
  `register_method` to `register_async_method` and awaits the trait
  call directly — zero spawn_blocking on the CRUD hot path. Service-
  method dispatch follows the same shape (the user-implemented
  service trait method may itself be sync; the outer
  `handle_service_call` impl is async because it threads through the
  async `handle_rpc`).
- `OsisAppWrapper`, the bridge from the (still-sync) legacy
  `OsisRpcHandler` trait to the new async `OsisAppRpcHandler`,
  uses `Handle::current().block_on(...)`. This is safe: the Axum-
  based legacy server in `osis/src/rpc/server.rs` already calls the
  wrapper from inside `tokio::task::spawn_blocking`, so by the time
  we land at the bridge we're on a tokio blocking worker thread —
  exactly the context `Handle::block_on` is designed for.
- The legacy Axum server in `osis/src/rpc/server.rs` keeps its
  `spawn_blocking` shells around `OsisRpcHandler::handle_request`
  (legacy trait, stays sync) and around service-method dispatch (the
  user-trait surface stays sync). The new hot path through
  `rpc2_adapter` doesn't touch any of those.
- Examples / benches / walkthrough: the in-process bench harness
  owns a `tokio::runtime::Runtime` and `block_on`s the now-async
  CRUD methods (criterion harness functions can't be `async fn`);
  the walkthrough's hand-written `OsisAppRpcHandler` impl is
  marked async; the committed-in generated example file gets its
  trait method signatures async-fied with sync-handle_rpc bridging.

Net effect: every CRUD request from network closure to indexer
socket is one continuous async stack with zero spawn_blocking hops
and zero nested-runtime panics. PR #130's blocking shims and bridge
runtime are gone.
timur merged commit bef62101e6 into development 2026-05-22 13:57:48 +00:00
Sign in to join this conversation.
Reviewers
No reviewers
Labels
Clear labels   
prio_critical

   
prio_low

   
type_bug

   
type_contact

   
type_issue

   
type_lead

   
type_question

   
type_story

   
type_task
No labels
prio_critical
prio_low
type_bug
type_contact
type_issue
type_lead
type_question
type_story
type_task
Milestone
Clear milestone
No items
No milestone
Projects
Clear projects
No items
No project
Assignees
Clear assignees
No assignees
1 participant
Notifications
Due date
The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set.

Reference
lhumina_code/hero_rpc!133
No description provided.