Skip to content

Commit fe70303

Browse files
rustyconoverclaude
andcommitted
feat(cache): HTTP conditional revalidation via /init-request validators
Result-cache conditional revalidation (304 / not_modified) now works over HTTP, not just subprocess. The validators (vgi.cache.if_none_match / if_modified_since) rode the first producer tick, which reaches the worker before it produces on subprocess. Over HTTP the first producer turn folds into the /init request, so the tick arrived too late and the worker always recomputed. Fix: on the HTTP path, attach the validators to the /init request's custom_metadata. SerializeRpcRequest gains an optional extra_metadata param; HttpFunctionConnection::PerformInit passes the armed validators (guarded by cond_sent_ for single delivery). Subprocess is unchanged (keeps first-tick delivery). Detection/consumption is already transport-agnostic — the worker's 0-row not_modified batch comes back in the /init response buffer and flows through GetLastCacheControl → MaybeSlideRevalidatedEntry → CachedReplayConnection swap, exactly as on subprocess. The worker side is handled in vgi-rpc (surfacing init-request metadata to the producer's first process()); the vgi-python worker needs no change (TableProducerState.process already reads the validators). revalidate.test now `require httpfs` so it runs on HTTP; full cache suite green subprocess (727) + HTTP (668, revalidate included). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
1 parent f38b138 commit fe70303

5 files changed

Lines changed: 59 additions & 25 deletions

File tree

CLAUDE.md

Lines changed: 11 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -581,13 +581,17 @@ log carries `tier=disk_streaming` vs `tier=memory` so the streaming path is test
581581

582582
**Conditional revalidation (304).** A stale-but-`revalidatable` entry is probed by
583583
`LookupForRevalidation` *before* `Lookup` (which drops stale) — if the payload ≥
584-
`vgi_result_cache_revalidate_min_bytes`, the first producer tick carries `if_none_match` /
585-
`if_modified_since`. If the worker replies with a 0-row `not_modified` batch, `GetNextBatch` slides
586-
the entry's TTL and swaps to a `CachedReplayConnection` over the stored bytes (single-threaded, no
587-
re-stream); if it streams fresh data instead, the parallel capture commits a replacement.
588-
Revalidatable entries survive TTL reaping (refreshed on access, not dropped). **Subprocess only**
589-
(over HTTP the first producer exchange folds into `/init` before any tick, so the validator can't
590-
reach the worker's first `process()` — a follow-up).
584+
`vgi_result_cache_revalidate_min_bytes`, the client sends `if_none_match` / `if_modified_since`. If
585+
the worker replies with a 0-row `not_modified` batch, `GetNextBatch` slides the entry's TTL and swaps
586+
to a `CachedReplayConnection` over the stored bytes (single-threaded, no re-stream); if it streams
587+
fresh data instead, the parallel capture commits a replacement. Revalidatable entries survive TTL
588+
reaping (refreshed on access, not dropped). **Works on both transports.** Subprocess carries the
589+
validators on the first producer tick. Over HTTP the first producer turn folds into the `/init`
590+
request, so the C++ client attaches the validators to the `/init` request `custom_metadata`
591+
(`SerializeRpcRequest` `extra_metadata`) and the worker's framework (vgi-rpc `_run_http_producer_init`)
592+
surfaces that init-request metadata to the producer's first `process()` — so `not_modified` fires
593+
identically. Detection/consumption (`GetLastCacheControl``MaybeSlideRevalidatedEntry` → replay swap)
594+
is transport-agnostic (the 304 arrives in the `/init` response buffer over HTTP).
591595

592596
**Files:** `src/vgi_result_cache.cpp` (singleton + disk tier + revalidation lookup),
593597
`src/vgi_cached_replay_connection.cpp` (serve), `src/vgi_result_cache_functions.cpp` (diagnostics),

src/include/vgi_rpc_client.hpp

Lines changed: 10 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@
77

88
#include <memory>
99
#include <string>
10+
#include <utility>
1011
#include <vector>
1112

1213
#include "duckdb/main/client_context.hpp"
@@ -158,9 +159,15 @@ StreamHeaderResult ReadStreamHeader(int fd, ClientContext *context,
158159
// `vgi_rpc.protocol_version` metadata instead of the global VGI_PROTOCOL_VERSION.
159160
// Used by the separately-versioned secret protocol (VGI_SECRET_PROTOCOL_VERSION);
160161
// all worker/catalog call sites leave it empty.
161-
std::vector<uint8_t> SerializeRpcRequest(const std::string &method_name,
162-
const std::shared_ptr<arrow::RecordBatch> &params_batch,
163-
const std::string &protocol_version_override = "");
162+
// extra_metadata: additional (key,value) pairs folded into the request's
163+
// custom_metadata alongside the method/version keys. Used by the HTTP transport to
164+
// carry the result-cache conditional-revalidation validators (vgi.cache.if_none_match
165+
// / if_modified_since) on the `/init` request, since over HTTP the worker's first
166+
// producer turn runs inside /init and must see the validators before it produces.
167+
std::vector<uint8_t> SerializeRpcRequest(
168+
const std::string &method_name, const std::shared_ptr<arrow::RecordBatch> &params_batch,
169+
const std::string &protocol_version_override = "",
170+
const std::vector<std::pair<std::string, std::string>> &extra_metadata = {});
164171

165172
// Serialize an RPC request with no parameters (zero-field schema, 1-row batch).
166173
std::vector<uint8_t> SerializeEmptyRpcRequest(const std::string &method_name);

src/vgi_http_function_connection.cpp

Lines changed: 18 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -470,8 +470,24 @@ InitResult HttpFunctionConnection::PerformInit(const BindResult &bind_result,
470470
#ifdef __EMSCRIPTEN__
471471
#endif
472472

473-
// Serialize the init RPC request to Arrow IPC
474-
auto body = SerializeRpcRequest("init", rpc_params);
473+
// Serialize the init RPC request to Arrow IPC. Over HTTP the worker's first
474+
// producer turn runs INSIDE this /init request (its output folds into the
475+
// response), so a conditional-revalidation validator must ride the init request
476+
// itself — the subprocess "first tick" arrives too late here. Attach
477+
// vgi.cache.if_none_match / if_modified_since to the init request metadata when
478+
// armed; the worker reads them off the init request and can answer 304
479+
// not_modified in its first (init-time) batch. cond_sent_ guards single delivery.
480+
std::vector<std::pair<std::string, std::string>> extra_metadata;
481+
if (!cond_sent_ && (!cond_if_none_match_.empty() || !cond_if_modified_since_.empty())) {
482+
if (!cond_if_none_match_.empty()) {
483+
extra_metadata.emplace_back(VGI_CACHE_IF_NONE_MATCH_KEY, cond_if_none_match_);
484+
}
485+
if (!cond_if_modified_since_.empty()) {
486+
extra_metadata.emplace_back(VGI_CACHE_IF_MODIFIED_SINCE_KEY, cond_if_modified_since_);
487+
}
488+
cond_sent_ = true;
489+
}
490+
auto body = SerializeRpcRequest("init", rpc_params, /*protocol_version_override=*/"", extra_metadata);
475491
#ifdef __EMSCRIPTEN__
476492
#endif
477493

src/vgi_rpc_client.cpp

Lines changed: 12 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -351,9 +351,10 @@ StreamHeaderResult ReadStreamHeader(int fd, ClientContext *context,
351351
// Buffer-based Serialization/Deserialization (for HTTP transport)
352352
// ============================================================================
353353

354-
std::vector<uint8_t> SerializeRpcRequest(const std::string &method_name,
355-
const std::shared_ptr<arrow::RecordBatch> &params_batch,
356-
const std::string &protocol_version_override) {
354+
std::vector<uint8_t> SerializeRpcRequest(
355+
const std::string &method_name, const std::shared_ptr<arrow::RecordBatch> &params_batch,
356+
const std::string &protocol_version_override,
357+
const std::vector<std::pair<std::string, std::string>> &extra_metadata) {
357358
auto sink_result = arrow::io::BufferOutputStream::Create();
358359
if (!sink_result.ok()) {
359360
throw IOException("Failed to create buffer for RPC request: " + sink_result.status().ToString());
@@ -373,9 +374,14 @@ std::vector<uint8_t> SerializeRpcRequest(const std::string &method_name,
373374
std::string protocol_version = protocol_version_override.empty()
374375
? std::string(::duckdb::vgi::generated::VGI_PROTOCOL_VERSION)
375376
: protocol_version_override;
376-
auto metadata = arrow::KeyValueMetadata::Make(
377-
{RPC_METHOD_KEY, RPC_REQUEST_VERSION_KEY, RPC_PROTOCOL_VERSION_KEY},
378-
{method_name, RPC_REQUEST_VERSION_VALUE, protocol_version});
377+
std::vector<std::string> meta_keys = {RPC_METHOD_KEY, RPC_REQUEST_VERSION_KEY,
378+
RPC_PROTOCOL_VERSION_KEY};
379+
std::vector<std::string> meta_values = {method_name, RPC_REQUEST_VERSION_VALUE, protocol_version};
380+
for (const auto &kv : extra_metadata) {
381+
meta_keys.push_back(kv.first);
382+
meta_values.push_back(kv.second);
383+
}
384+
auto metadata = arrow::KeyValueMetadata::Make(std::move(meta_keys), std::move(meta_values));
379385

380386
auto status = writer->WriteRecordBatch(*params_batch, metadata);
381387
if (!status.ok()) {

test/sql/integration/cache/revalidate.test

Lines changed: 8 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -2,18 +2,19 @@
22
# description: VGI result cache (M6) — conditional revalidation (304 / not_modified)
33
# group: [vgi_integration_cache]
44
#
5-
# Subprocess-only: the conditional-request validators ride the first producer
6-
# tick, which the worker's process() reads. Over HTTP the first producer exchange
7-
# is folded into the /init POST (its output is buffered before any tick is sent),
8-
# so the if_none_match validator never reaches the worker's first process() call
9-
# and the not_modified path can't fire. Omitting `require httpfs` makes this file
10-
# skip cleanly under the HTTP harness (the http:// ATTACH errors → auto-skip)
11-
# while still running on subprocess. See docs — HTTP revalidation is a follow-up.
5+
# Works on BOTH transports. The conditional-request validators
6+
# (vgi.cache.if_none_match / if_modified_since) ride the first producer tick on the
7+
# subprocess transport. Over HTTP the first producer turn is folded into the /init
8+
# request, so the C++ client attaches the validators to the /init request metadata
9+
# and the worker (via vgi_rpc) surfaces that metadata to its first process() call —
10+
# so the not_modified path fires identically over HTTP.
1211

1312
require-env VGI_TEST_WORKER
1413

1514
require vgi
1615

16+
require httpfs
17+
1718
statement ok
1819
CALL enable_logging(level := 'debug');
1920

0 commit comments

Comments
 (0)