Skip to content

perf: route remote HTTP puts to owner - #274

Closed
perfloop[bot] wants to merge 7 commits into
mainfrom
perfloop-pr-open-fn980pdxs8
Closed

perfloop[bot] wants to merge 7 commits into
mainfrom
perfloop-pr-open-fn980pdxs8

Conversation

@perfloop

@perfloop perfloop Bot commented Sep 9, 2026 •

Copy link
Copy Markdown
Contributor

The HTTP gateway used to send every decoded PutObject through the gateway node’s local gRPC handler. For nonlocal keys, that added a loopback dispatch before the owner write. This change sends eligible unary bodies to the resolved owner while keeping the existing local and streaming fallbacks.

Change

  • Wrap the generated CacheServiceClient and override PutObject for nonlocal bodies up to 4 MiB. Resolve the owner once and use the existing routed peer connection, retry, health, and circuit-breaker path.
  • Keep local handling for cluster-disabled, invalid, unresolved, local-owner, over-limit, and hop-limit requests. Preserve TTL, forwarding metadata, cancellation, response errors, and the owner’s normal routing check and streaming fallback.
  • Add focused gateway and coordinator tests plus a reproducible three-node HTTP benchmark for the dispatch counters and latency/CPU profile.
  • Log routing retries and successful peer-connection establishment at Info during development and testing. The success event is emitted when a peer connection is established instead of on every cached request, avoiding per-request log output on the routed write path.

Verification

  • make -j1 check
  • make lint
  • make license-check
  • make test-server TEST=TestGatewayCacheServiceClient
  • make test-coordinator TEST=TestRouter
  • make test-e2e-cluster remains environment-blocked at its pre-operation timeout 10 ./ocachecli --addr ... list connectivity gate; target CI can run this check.

Workload: warmed three-node nonlocal HTTP PutObject 100 B

Metric Before After Change
http-put-p50-us 842.2 615.2 27% lower
gateway-putobject-per-request 1 0 100% lower
owner-putobject-per-request 0 1 higher
owner-putstream-per-request 1 0 100% lower

Workload: warmed three-node nonlocal HTTP PutObject 64 KiB

Metric Before After Change
http-put-p50-us 2267 1899 16.2% lower
gateway-putobject-per-request 1 0 100% lower
owner-putobject-per-request 0 1 higher
owner-putstream-per-request 1 0 100% lower

Workload: warmed three-node nonlocal HTTP PutObject 1 MiB

Metric Before After Change
http-put-p50-us 20249 17642 12.9% lower
gateway-putobject-per-request 1 0 100% lower
owner-putobject-per-request 0 1 higher
owner-putstream-per-request 1 0 100% lower
Checks: 3 passed.

Generated by Perfloop. Measurements and checks.


Note

Medium Risk
Changes cluster write routing and hop/metadata behavior on the HTTP path; fallbacks preserve prior behavior for large payloads and hop limits, but mis-routing or metadata bugs could affect remote puts.

Overview
Eligible HTTP PutObject requests (nonlocal keys, body ≤ 4 MiB, within hop limit) now resolve the owner once and call that node’s unary PutObject over the existing router path, instead of always loopbacking through the gateway node’s local handler (which previously forwarded via streaming).

The HTTP grpc-gateway registers a wrapping CacheServiceClient that overrides only PutObject; other RPCs and oversized puts still use the local client and existing Operations forwarding. Coordinator routing adds Resolve / RouteToNode so ownership lookup and peer client acquisition can be split without re-hashing the key.

Forwarding metadata handling is adjusted so ExtractRequestMetadata reads outgoing context (gateway client path) and AttachForwardingMetadata merges keys without dropping unrelated outgoing metadata (e.g. auth). Tests cover gateway routing, hop metadata, and router RouteToNode; a three-node HTTP benchmark script measures latency and RPC dispatch counters.

Reviewed by Cursor Bugbot for commit bcd2798. Bugbot is set up for automated code reviews on this repo. Configure here.

@greptile-apps

greptile-apps Bot commented Sep 9, 2026 •

Copy link
Copy Markdown

Greptile Summary

This PR optimizes HTTP PutObject handling by routing eligible unary requests directly to the resolved owner while retaining local and streaming fallbacks.

  • Adds separate owner resolution and peer-client acquisition APIs while preserving retry, health, and circuit-breaker behavior.
  • Preserves forwarding metadata, cancellation, TTL, error responses, hop limits, and unrelated outgoing metadata.
  • Adds focused gateway, router, metadata, and three-node benchmark coverage.
  • No actionable new correctness or security issues were identified.

Confidence Score: 5/5

The PR appears safe to merge, with no actionable new correctness, security, or repository-rule violations found.

Direct-owner HTTP writes retain the existing routing safeguards and service response contracts, and the changes made since the previous review do not introduce a demonstrated failure.

Important Files Changed

Filename Overview
server/service/gateway.go Introduces direct-owner routing for eligible HTTP unary puts with guarded local fallbacks and preserved forwarding semantics.
server/service/service.go Registers the grpc-gateway through the routing client wrapper and closes its local connection when the server context ends.
coordinator/router.go Separates ownership resolution from peer-client acquisition while retaining existing routing protections.
coordinator/interceptors.go Supports gateway outgoing metadata and replaces forwarding fields without discarding unrelated metadata.
scripts/benchmark_http_gateway.py Adds a manually invoked three-node benchmark with safe argument-list process execution and routing counter measurements.
server/service/gateway_test.go Covers direct routing, fallback conditions, metadata propagation, cancellation, and response behavior.
coordinator/router_test.go Verifies resolving an owner and acquiring its client through the new split routing API.

Reviews (3): Last reviewed commit: "fix(coordinator): keep the router's per-..." | Re-trigger Greptile

Comment thread coordinator/router.go
greptile-apps[bot]
greptile-apps Bot previously approved these changes Sep 9, 2026
perfloop-agent and others added 7 commits September 9, 2026 15:46
Signed-off-by: Perfloop Agent <agent@perfloop.ai>
Signed-off-by: Perfloop Agent <agent@perfloop.ai>
Signed-off-by: Perfloop Agent <agent@perfloop.ai>
Signed-off-by: Perfloop Agent <agent@perfloop.ai>
Promote direct-owner routing events to Info during validation.

Signed-off-by: Perfloop Agent <agent@perfloop.ai>
Signed-off-by: Perfloop Agent <agent@perfloop.ai>
The retry line fires on every reconnect attempt and the success line on
every connection established; both were raised to info for development.
Per-event info logging floods under an outage, so they stay at debug
like the rest of the router.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01YCoN4Him5ZY4hZovVp37ra
Signed-off-by: Ovais Tariq <ot@tigrisdata.com>
@ovaistariq

Copy link
Copy Markdown
Contributor

Rebased onto main and demoted the two router log lines this PR had raised to info (bcd2798): the retry line fires per reconnect attempt and the success line per connection established, and per-event info logging floods under an outage, so both stay at debug like the rest of the router. The gateway change itself is untouched. Coordinator and server suites pass. Ready to merge.

@ovaistariq

Copy link
Copy Markdown
Contributor

The HTTP path is not used by production services.

@ovaistariq ovaistariq closed this Sep 9, 2026
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.

2 participants