feat(backend,dist-http): add internal HTTP server & HTTP transport with context-aware replication #47
Conversation
…th context-aware replication Introduce internal Fiber-based HTTP server (set/get/remove, health) and HTTP transport for cross-node forwarding. Refactor DistMemory to auto-start server/transport when nodeAddr set, add heartbeat start helper, and split initialization into helper methods (ensureShardConfig, initMembershipIfNeeded, tryStartHTTP, startHeartbeatIfEnabled). Add context propagation through ForwardSet/ForwardRemove, replication, repair, and internal applySet/applyRemove paths; enhance read-repair to use context. Replace direct JSON decoding in get path with map+mirror struct to satisfy lint (musttag) and wrap all network/JSON errors via ewrap. Extend DistTransport interface to include context in write/remove operations. Introduce health probe, structured error wrapping, and best‑effort graceful shutdown in Stop. Switch tests (and cmap test) to goccy/go-json for consistency and performance. Update cspell word list (fctx, hreq, lamport). Add lamport-like version counter note and clarify replication semantics. Key changes: New files: dist_http_server.go, dist_http_transport.go, dist_http_types.go DistMemory: added httpServer field, context-aware Set/Remove/replication, helper init methods DistTransport: ForwardSet/ForwardRemove now accept context InProcessTransport adjusted to new interface Added health endpoint and internal request DTO consolidation Improved error handling & lint compliance (errcheck, wrapcheck, noctx, musttag, etc.) BREAKING CHANGE: DistTransport interface now requires context for ForwardSet and ForwardRemove; internal helpers applySet/applyRemove and replicateTo signatures changed accordingly. External custom transport implementations must be updated.
|
Running Code Quality on PRs by uploading data to Trunk will soon be removed. You can still run checks on your PRs using trunk-action - see the migration guide for more information. |
There was a problem hiding this comment.
Pull Request Overview
This PR introduces internal HTTP server capabilities and HTTP transport for distributed caching with context propagation. The changes enable cross-node communication via HTTP when node addresses are configured, adding automatic server startup and context-aware operations throughout the distributed cache system.
- Adds internal HTTP server for cache operations (set/get/remove) and health checks
- Implements HTTP transport for cross-node forwarding with context propagation
- Refactors DistMemory initialization into helper methods for better organization
Reviewed Changes
Copilot reviewed 8 out of 8 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| tests/management_http_test.go | Switches from standard library JSON to goccy/go-json |
| tests/hypercache_mgmt_dist_test.go | Switches from standard library JSON to goccy/go-json |
| pkg/cache/cmap_test.go | Switches from standard library JSON to goccy/go-json |
| pkg/backend/dist_memory.go | Adds HTTP server field, context-aware operations, and initialization refactoring |
| pkg/backend/dist_http_types.go | Defines shared HTTP request/response DTOs |
| pkg/backend/dist_http_transport.go | Implements HTTP transport with context propagation |
| pkg/backend/dist_http_server.go | Implements internal HTTP server for cache operations |
| cspell.config.yaml | Adds new technical terms to spell check dictionary |
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
| return sentinel.ErrBackendNotFound | ||
| } | ||
|
|
||
| const statusThreshold = 300 // local redeclare for linter clarity |
There was a problem hiding this comment.
The constant statusThreshold is redeclared locally with the same value as the package-level constant. This creates duplication and potential for inconsistency. Remove the local declaration and use the package-level constant.
| const statusThreshold = 300 // local redeclare for linter clarity |
| return fctx.JSON(httpSetResponse{}) | ||
| } | ||
|
|
||
| dm.applySet(ctx, it, false) |
There was a problem hiding this comment.
The context ctx passed to applySet is the server startup context, not the request context. This could cause operations to be cancelled when the server shuts down rather than respecting request timeouts. Use fctx.UserContext() to get the request context instead.
| dm.applySet(ctx, it, false) | |
| dm.applySet(fctx.UserContext(), it, true) | |
| return fctx.JSON(httpSetResponse{}) | |
| } | |
| dm.applySet(fctx.UserContext(), it, false) |
| return fctx.JSON(httpSetResponse{}) | ||
| } | ||
|
|
||
| dm.applySet(ctx, it, false) |
There was a problem hiding this comment.
The context ctx passed to applySet is the server startup context, not the request context. This could cause operations to be cancelled when the server shuts down rather than respecting request timeouts. Use fctx.UserContext() to get the request context instead.
| dm.applySet(ctx, it, false) | |
| dm.applySet(fctx.UserContext(), it, true) | |
| return fctx.JSON(httpSetResponse{}) | |
| } | |
| dm.applySet(fctx.UserContext(), it, false) |
| return fctx.Status(fiber.StatusBadRequest).JSON(fiber.Map{"error": "invalid replicate"}) | ||
| } | ||
|
|
||
| dm.applyRemove(ctx, key, replicate) |
There was a problem hiding this comment.
The context ctx passed to applyRemove is the server startup context, not the request context. This could cause operations to be cancelled when the server shuts down rather than respecting request timeouts. Use fctx.UserContext() to get the request context instead.
| dm.applyRemove(ctx, key, replicate) | |
| dm.applyRemove(fctx.UserContext(), key, replicate) |
Introduce internal Fiber-based HTTP server (set/get/remove, health) and HTTP transport for cross-node forwarding. Refactor DistMemory to auto-start server/transport when nodeAddr set, add heartbeat start helper, and split initialization into helper methods (ensureShardConfig, initMembershipIfNeeded, tryStartHTTP, startHeartbeatIfEnabled). Add context propagation through ForwardSet/ForwardRemove, replication, repair, and internal applySet/applyRemove paths; enhance read-repair to use context. Replace direct JSON decoding in get path with map+mirror struct to satisfy lint (musttag) and wrap all network/JSON errors via ewrap. Extend DistTransport interface to include context in write/remove operations. Introduce health probe, structured error wrapping, and best‑effort graceful shutdown in Stop. Switch tests (and cmap test) to goccy/go-json for consistency and performance. Update cspell word list (fctx, hreq, lamport). Add lamport-like version counter note and clarify replication semantics.
Key changes:
New files: dist_http_server.go, dist_http_transport.go, dist_http_types.go DistMemory: added httpServer field, context-aware Set/Remove/replication, helper init methods DistTransport: ForwardSet/ForwardRemove now accept context InProcessTransport adjusted to new interface
Added health endpoint and internal request DTO consolidation Improved error handling & lint compliance (errcheck, wrapcheck, noctx, musttag, etc.) BREAKING CHANGE: DistTransport interface now requires context for ForwardSet and ForwardRemove; internal helpers applySet/applyRemove and replicateTo signatures changed accordingly. External custom transport implementations must be updated.