Skip to content

Feat: Async - #51

Merged
CSenshi merged 30 commits into
mainfrom
feat/async
May 4, 2026
Merged

Feat: Async#51
CSenshi merged 30 commits into
mainfrom
feat/async

Conversation

@CSenshi

@CSenshi CSenshi commented May 3, 2026

Copy link
Copy Markdown
Owner

Summary

Adds grpc_call_async() support

Closes #50

@CSenshi
CSenshi requested a review from Copilot May 3, 2026 21:19

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Adds asynchronous gRPC call support to pg_grpc by introducing a background worker + queue/results tables, along with public SQL APIs and accompanying documentation/website updates. This addresses #50 by allowing callers to enqueue work and fetch results later instead of blocking on the network.

Changes:

  • Introduces async execution infrastructure (bg worker, shared memory signaling, queue/result persistence, async GUCs).
  • Adds new public SQL functions: grpc_call_async(), grpc_call_result(), grpc_wait_until_running().
  • Updates docs/site (new “Async calls” guide, reference updates, sidebar/hero updates) and bumps website dependencies.

Reviewed changes

Copilot reviewed 20 out of 21 changed files in this pull request and generated 3 comments.

Show a summary per file
File Description
src/lib.rs Registers the background worker; adds SQL APIs for async enqueue/result/wait (and a restart helper).
src/worker.rs Implements the bg worker loop: latch wake, dequeue, concurrent execution, persist results, TTL cleanup.
src/queue.rs Implements dequeue/lookup/result persistence and TTL cleanup logic.
src/shmem.rs Shared-memory state for worker status, wake flag, and latch pointer storage.
src/guc.rs Adds async-related GUCs (database_name, username, batch_size, ttl).
src/call.rs Adds call_async_row() to execute a queued call with timeout and return a structured outcome.
src/async_schema.rs Creates grpc.call_queue / grpc._call_result tables (and related types) on extension bootstrap.
src/tests/* Adds tests for worker readiness, GUC defaults, queue operations, and async call end-to-end flows.
website/docs/guides/async-calls.md New guide explaining async calls, worker behavior, and configuration.
website/docs/reference.md Documents new SQL functions and async GUCs.
website/static/llms*.txt Updates the LLM index/docs bundle with async call content and guide link.
website/src/components/Hero/Hero.tsx Adds an “async” tab/pane to the homepage terminal demo.
website/sidebars.ts Adds the async guide to the Guides sidebar.
website/package.json / website/package-lock.json Updates Docusaurus-related dependency versions/ranges.
website/docusaurus.config.ts Updates OG/Twitter alt text formatting.
Comments suppressed due to low confidence (1)

website/package.json:33

  • The @docusaurus dependencies were bumped to 3.10.1, but the related @docusaurus devDependencies (@docusaurus/module-type-aliases, @docusaurus/tsconfig, @docusaurus/types) are still pinned to 3.10.0. Docusaurus expects its packages to stay on the same version line, and this mismatch can cause type/build inconsistencies. Update the devDependencies to the same version range/version as the runtime @docusaurus packages (or intentionally pin everything to a single exact version).
    "@docusaurus/core": "^3.10.1",
    "@docusaurus/faster": "^3.10.1",
    "@docusaurus/preset-classic": "^3.10.1",
    "@docusaurus/theme-mermaid": "^3.10.1",
    "@easyops-cn/docusaurus-search-local": "^0.55.1",
    "@mdx-js/react": "^3.0.0",
    "clsx": "^2.0.0",
    "prism-react-renderer": "^2.3.0",
    "react": "^19.0.0",
    "react-dom": "^19.0.0"
  },
  "devDependencies": {
    "@docusaurus/module-type-aliases": "3.10.0",
    "@docusaurus/tsconfig": "3.10.0",
    "@docusaurus/types": "3.10.0",
    "@types/react": "^19.0.0",

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread src/worker.rs Outdated
Comment thread src/lib.rs Outdated
Comment thread src/lib.rs Outdated

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 19 out of 20 changed files in this pull request and generated 11 comments.


💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread website/static/llms-full.txt Outdated
Comment thread src/tests/call_async.rs
Comment thread website/package.json
Comment thread website/docs/reference.md Outdated
Comment thread website/docs/guides/async-calls.md Outdated
Comment thread src/lib.rs Outdated
Comment thread src/worker.rs Outdated
Comment thread src/worker.rs Outdated
Comment thread src/queue.rs Outdated
Comment thread src/guc.rs

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 19 out of 20 changed files in this pull request and generated 6 comments.

Files not reviewed (1)
  • website/package-lock.json: Language not supported

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread src/call.rs Outdated
pub(crate) async fn call_async_row(row: crate::queue::QueueRow) -> crate::queue::CallResult {
let opts = match &row.options {
None => crate::options::OptionsConfig::default(),
Some(v) => crate::options::OptionsConfig::parse(v).unwrap_or_default(),
Comment thread src/worker.rs Outdated
Comment on lines +83 to +89
for row in rows {
set.spawn(call::call_async_row(row));
}
let mut out = Vec::new();
while let Some(res) = set.join_next().await {
if let Ok(r) = res {
out.push(r);
Comment thread src/queue.rs Outdated
Comment on lines +58 to +62
.map(|j| serde_json::from_str(&j.0.to_string()).unwrap()),
options: row["options"]
.value::<pgrx::JsonB>()
.unwrap()
.map(|j| serde_json::from_str(&j.0.to_string()).unwrap()),
Comment thread src/queue.rs Outdated
Comment on lines +185 to +186
v.map(|j| serde_json::from_str(&j.0.to_string()).unwrap())
.unwrap_or(JsonValue::Null)
Comment thread website/src/components/Hero/Hero.tsx Outdated
@@ -1,25 +1,26 @@
import React, {useEffect, useRef, useState} from 'react';
import React, { JSX, useEffect, useRef, useState } from 'react';
Comment thread src/lib.rs Outdated
Comment on lines +97 to +105
// Register commit-time wake so the worker is signaled only when the
// enqueuing transaction actually commits (rolled-back calls never wake it).
// The flag prevents registering more than once per transaction even when
// grpc_call_async() is called many times within the same transaction.
if !WAKE_CB_ACTIVE.swap(true, Ordering::Relaxed) {
unsafe {
pg_sys::RegisterXactCallback(Some(wake_worker_on_commit), std::ptr::null_mut());
}
}
@claude

claude Bot commented May 4, 2026

Copy link
Copy Markdown

Claude Code is working…

I'll analyze this and get back to you.

View job run

@claude

claude Bot commented May 4, 2026

Copy link
Copy Markdown

Claude encountered an error —— View job


I'll analyze this and get back to you.

@claude

claude Bot commented May 4, 2026

Copy link
Copy Markdown

PR Review: Feat/Async

  • Read core implementation files
  • Read lib.rs changes and async_schema.rs
  • Read test files
  • Analyze and form review

View job run

@claude

claude Bot commented May 4, 2026

Copy link
Copy Markdown

Claude finished @CSenshi's task in 2m 54s —— View job


PR Review: Feat/Async

  • Set up task list
  • Read core implementation files (worker.rs, queue.rs, shmem.rs, guc.rs, call.rs, lib.rs, async_schema.rs)
  • Read test files (call_async.rs, queue.rs, guc.rs)
  • Analyze issues and form review

This PR adds a solid async gRPC call infrastructure. The architecture is clean: UNLOGGED queue tables, a background worker, shared-memory latch signaling, and a single xact callback registered at _PG_init. The implementation addresses all the issues raised in earlier Copilot reviews. Below are remaining concerns and observations, ordered by severity.


Bugs

1. worker.rs:87-90 — Dropped JoinError silently loses results

When a spawned task panics or is cancelled, set.join_next().await returns Err(JoinError). The code silently skips it:

while let Some(res) = set.join_next().await {
    if let Ok(r) = res {
        out.push(r);
    }
}

The queue row was already DELETEd by dequeue(), but no _call_result row is written. Any caller of grpc_call_result(id) will see ERROR: not found — result may have expired, which is misleading — the call was actually lost to a panic/cancel.

Fix: match on the Err arm and synthesise a CallResult { id, outcome: CallOutcome::Error(...) }. You'll need to track the call ID when spawning — either store it alongside the future or move the ID into the closure. Fix this →

2. call.rs:103-106 — Options parse failure silently falls back to defaults

Some(v) => crate::options::OptionsConfig::parse(v).unwrap_or_default(),

If the options JSONB stored in call_queue is syntactically valid but fails OptionsConfig::parse (e.g., unknown key from a schema migration, manual insertion), the call runs with default options instead of surfacing a clear error. The caller has no way to diagnose this. Fix this →


Tests

3. call_async.rs:280 — Hardcoded grpcb.in:9000 in test_worker_pipeline_processes_grpc_call

VALUES (20001, 'grpcb.in:9000', 'grpcbin.GRPCBin/DummyUnary', ...)

This test directly inserts a row with a hardcoded endpoint, bypassing the GRPCBIN_ENDPOINT env override that makes the suite hermetic in CI. Other tests in the same file correctly use grpcbin_endpoint() (lines 4, 139, 194). Fix this →


Design / Operational observations

4. call_queue is UNLOGGED — data loss on crash is silent

Both grpc.call_queue and grpc._call_result are UNLOGGED. This means any rows enqueued but not yet processed are silently lost on a PostgreSQL crash/restart. The guide (async-calls.md) mentions "fire-and-forget" semantics, but this should be prominently documented since it's surprising behaviour for users who expect at-least-once delivery.

5. shmem.rs:26-31 — TOCTOU window between latch pointer read and SetLatch call

pub fn set_latch() {
    let ptr = *WORKER_LATCH.share();   // lock acquired, ptr copied, lock released
    if ptr != 0 {
        unsafe { pg_sys::SetLatch(ptr as *mut pg_sys::Latch) }; // lock already gone
    }
}

The PgLwLock guard drops after *WORKER_LATCH.share() is dereferenced, releasing the lock before SetLatch is called. A concurrent clear_latch() from an exiting worker could zero the shared slot after the read but before SetLatch. In that scenario ptr is a stale (dangling) pointer and SetLatch would be called on recycled memory. This is a pre-existing pattern used in extensions like pg_net and PostgreSQL accepts the operational risk, but it's worth documenting the invariant: set_latch() may no-op (benign) or wake a recycled slot (low probability, benign in practice if the PGPROC slot is reused for a different backend that wakes up spuriously).

6. dequeue() has no ordering guarantee under concurrency

The query SELECT id FROM grpc.call_queue ORDER BY id LIMIT $1 assumes a single consumer. If a second worker instance ever runs (e.g., user manually registers one), two workers will race on the same rows. A FOR UPDATE SKIP LOCKED guard would make this future-safe. This is low priority given the single-worker architecture, but worth noting if multi-worker support is ever planned.

7. grpc_call_result(id, async => false) holds the connection in a polling loop

The synchronous wait mode polls every 50ms with WaitLatch. This is fine for typical use, but there's no cap on total wait time — if the worker crashes and the row is neither in _call_result nor in call_queue (e.g., was dequeued then the worker died before writing the result), this function would return immediately with "not found". The async => false path only exits Pending once the row leaves call_queue, so it does handle the normal flow correctly.


Nits

  • call_async.rs tests for test_call_async_enqueues_row and test_call_async_rollback_no_row (lines 3–8, 23–26) use the verbose crate::tests::tests::grpcbin_endpoint() path. The other tests in the same file use grpcbin_endpoint() directly (they're include!-d into mod tests), so the shorter form works.
  • queue.rs:100-103: insert_results uses .unwrap() on the SPI result — consistent with the rest of the codebase but a worker crash here would leave a transaction open. Consider logging a warning and aborting the transaction gracefully.

Overall this is a well-structured feature with good test coverage. The three fixes above (JoinError handling, options parse errors, hardcoded endpoint) are the most important before merging.

@claude

claude Bot commented May 4, 2026

Copy link
Copy Markdown

Claude finished @CSenshi's task in 2m 32s —— View job


PR Review: Feat/Async

  • Set up task list
  • Read core implementation files (worker.rs, queue.rs, shmem.rs, guc.rs, call.rs)
  • Read lib.rs changes and async_schema.rs
  • Read test files (call_async.rs, queue.rs, guc.rs)
  • Check if previous review issues were addressed
  • Analyze and form updated review

All three bugs flagged in the previous Claude review have been fixed, along with every actionable Copilot comment. This is looking merge-ready. A few remaining observations below.

Fixed since last review

Issue Status
JoinError silently dropped ✅ Fixed — join_next_with_id() + HashMap tracks IDs; panics yield a deterministic CallOutcome::Error("task panicked")
OptionsConfig::parse silently falls back to defaults ✅ Fixed — parse failure returns early with CallOutcome::Error(e.to_string()) (call.rs:105-113)
Hardcoded grpcb.in:9000 in pipeline test ✅ Fixed — let endpoint = grpcbin_endpoint() + format string in call_async.rs:278-282
RegisterXactCallback called on every grpc_call_async ✅ Fixed — callback registered once in _PG_init (lib.rs:30); WAKE_CB_ACTIVE guards the per-transaction signal
grpc_worker_restart / grpc_wait_until_running in public API ✅ Removed
grpc.call_queue docs showed phantom status column ✅ Fixed in async-calls.md and llms-full.txt
timeout_ms cast without upper-bound check ✅ Fixed — lib.rs:69-71 validates against i32::MAX before casting
Worker only processes one batch per wake (should_wake consumed) ✅ Fixed — inner loop drains until rows.is_empty() (worker.rs:58-114)
TTL cleanup skipped when queue was empty on wake ✅ Fixed — ttl_cleanup called inside the rows.is_empty() branch (worker.rs:72)

Remaining observations

1. test_call_async_enqueues_row / test_call_async_rollback_no_row — verbose path

call_async.rs:4 and call_async.rs:24 use crate::tests::tests::grpcbin_endpoint(). Since call_async.rs is include!-d into mod tests, the unqualified grpcbin_endpoint() works (as used on lines 139 and 278). Minor style inconsistency. Fix this →

2. insert_results panics on SPI failure

queue.rs:102 calls .unwrap() on the bulk-insert SPI result. In a background worker, an unhandled panic triggers a crash + 1-second restart. If the INSERT into grpc._call_result fails (disk full, schema corruption, etc.), all results in that batch are lost silently and the worker restarts. ttl_cleanup handles this gracefully with a warning!insert_results could follow the same pattern, at minimum logging the error. This matches the nit from the previous review that is still open.

3. shmem::set_latch() — TOCTOU window (low priority, pre-existing pattern)

shmem.rs:27-30: the PgLwLock guard drops after dereferencing *WORKER_LATCH.share(), releasing the lock before SetLatch is called. A concurrent clear_latch() from an exiting worker could zero the slot between the read and the SetLatch call, producing a stale pointer dereference. This is the same low-risk pattern used by pg_net and is operationally acceptable — in practice the spurious wakeup or no-op is harmless — but it's worth a comment documenting the invariant.

4. dequeue() not guarded against concurrent consumers (low priority, design note)

queue.rs:36-43: the CTE-based DELETE ... RETURNING is correct for a single worker but doesn't use FOR UPDATE SKIP LOCKED on the inner SELECT. If a second worker ever ran concurrently, both could select the same IDs and block on each other's DELETE. Adding FOR UPDATE SKIP LOCKED to the CTE's SELECT would make the dequeue future-safe at zero cost. Not a bug today, but a cheap one-line hardening if multi-worker support is ever planned.

5. UNLOGGED tables — crash semantics not prominently documented

Both grpc.call_queue and grpc._call_result are UNLOGGED. After a PostgreSQL crash/restart, any rows that were in the queue (enqueued but not yet processed) are silently truncated. The guide (async-calls.md:119) describes the tables but doesn't surface the crash-loss implication. Users who expect fire-and-forget but are surprised by lost rows on crash would benefit from an explicit note in the Schema section.


Overall this is a clean, well-structured implementation with solid test coverage. The three critical bugs from the previous review are all resolved. The remaining items are low-severity (nit, operational note, safety hardening, docs) and can be addressed as follow-ups.

@CSenshi
CSenshi merged commit 9023078 into main May 4, 2026
3 checks passed
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.

Add async call support

2 participants