-
Notifications
You must be signed in to change notification settings - Fork 14
Major Refactoring: Interceptor Architecture & Iterator-based Responses #245
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
The Result[T] type replaces the non-generic, unexported response type. The reason to export the type is to make it easier to implement interceptors that need to use this type. Note that Result[T] is currently only used as Result[proto.Message] instances (in the current channel and as called from the existing calltypes (like quorumcall, multicast etc). In a future commit Result[T] will also be instansiated as Result[Resp], or Result[any], which can be used at the interceptor level.
This updates the testSrv implmentation to support the GetValue method. It also adds the echoSrv implmentation. These will be used for testing client-side interceptors.
Linters have been complaining about this style issue for a while.
Implement a flexible, composable interceptor architecture for quorum calls that provides better type safety, modularity, and extensibility compared to the legacy QuorumCall approach. Core Architecture: - QuorumInterceptor: Generic type for interceptor functions that wrap QuorumFunc - QuorumFunc: Function signature for processing quorum calls and returning results - ClientCtx: Context object providing access to request, config, and response iterator - Chain: Utility function to compose interceptors around a base handler Key Features: 1. Full type safety with generics (Req, Resp, Out type parameters) 2. Support for custom return types via Out parameter 3. Lazy message sending - transforms applied before dispatch 4. Iterator-based response handling with early termination support 5. Composable middleware pattern for building complex quorum logic Base Quorum Functions (Aggregators): - MajorityQuorum: Returns first response after ⌈(n+1)/2⌉ successful replies - FirstResponse: Returns first successful response (read-any pattern) - AllResponses: Waits for all nodes to respond (write-all pattern) - ThresholdQuorum: Generic threshold-based quorum with configurable count - CollectAllResponses: Returns map of all successful responses by node ID Interceptors (Middleware): - PerNodeTransform: Applies per-node request transformations with skip support - QuorumSpecInterceptor: Adapter for legacy QuorumSpec-style functions Iterator Helpers: - IgnoreErrors: Filters iterator to yield only successful responses - CollectN: Collects up to n responses into a map - CollectAll: Collects all responses into a map Implementation Details: - Lazy sending via sync.OnceFunc ensures transforms registered before dispatch - RegisterTransformFunc allows chaining multiple request transformations - applyTransforms applies registered transforms in order, skips invalid results - Responses() iterator yields node responses as they arrive - Type-safe conversion from Result[proto.Message] to Result[Resp] Backward Compatibility: - Legacy QuorumCall remains unchanged - QuorumSpecInterceptor bridges old and new approaches - No breaking changes to existing code Testing: - 17 comprehensive test functions covering unit and integration scenarios - Tests for iterator utilities, interceptor chaining, custom aggregation - Tests for per-node transformation with node skipping - Tests for all base quorum functions with various error conditions - Integration tests with real gRPC servers - Helper functions for consistent test patterns (testContext, checkError, etc.) This architecture enables gradual migration from the legacy approach and provides a foundation for future code generation template updates. Files changed: - client_interceptor.go (new, 446 lines) - client_interceptor_test.go (new, 794 lines)
This fixes lint issues raised by deepsource and golangci-lint.
This changes the iterator API from iter.Seq2[uint32, Result[T]] to a cleaner iter.Seq[Result[T]] pattern, and a type alias Results[T] which can serve as receiver for methods on said iterator. This simplifies the API by consolidating node ID and result information into a single Result[T] value, making iteration more ergonomic, despite not following Go's function-based iterator patterns. Key changes: - Introduce Results[T] type alias for iter.Seq[Result[T]] - Change ClientCtx.Responses() return type from iter.Seq2 to Results[T] - Update iterator helper methods to be methods on Results[T]: * IgnoreErrors() now returns Results[T] instead of iter.Seq2[uint32, T] * Add Filter() method for generic result filtering * CollectN() and CollectAll() now methods on Results[T] - Update all iterator consumers to use single-value iteration pattern - Constrain ClientCtx type parameters to msg (proto.Message) type Benefits: - Simpler iteration: `for result := range ctx.Responses()` vs `for nodeID, result := range ctx.Responses()` - More composable: method chaining like `ctx.Responses().IgnoreErrors().CollectAll()` - Consistent: Result[T] already contains NodeID, no need to pass separately - Cleaner: Filter() operates on complete Result[T] values This borrows from Asbjørn Salhus's design in PR #230, which I now agree is better than Go's function-based iterator pattern because of its significantly better composability. That is, you avoid composing with functions that would look like: gorums.IgnoreErrors(ctx.Responses())... and even worse when there are many iterators being composed.
Tests should fail with a deadline exceeded if they block; this is an indication of a deadlock issue that needs to be investigated.
Deepsource wants ctx to be the first argument, even in tests helpers.
This helps to more clearly distinguish the difference between an individual NodeResponse (previously Result) from the complete set of Responses (previously Results).
This removes CallData and QuorumCallData from some of the calltypes, but still left async and quorumcall as is.
This avoids passing the callOptions type to the channel, avoiding spilling type parameters into the request struct.
This renames: - WithQuorumInterceptors to Interceptors - WithPerNodeTransform to Transform
This also support mapping return values from the incoming responses.
This avoids using a func() in the ClientCtx struct that must be called to get the iter.Seq with responses; now we can just use the ctx.responses field directly to get the iterator.
This was intended to support MapRequest as an interceptor, but multicast does not yet support interceptors. Nonetheless, there were a couple simplifications that were lingering around from before.
- Replaced WithNodeContext with Node.Context for creating NodeContext instances. - Replaced WithConfigContext with Configuration.Context for creating ConfigContext instances. - Adjusted related documentation to reflect the new context creation methods.
This adds several examples of recommended patterns for custom quorum functions, including with custom return types.
This changes the intercepter design to this: type QuorumInterceptor[Req, Resp msg] func(ctx *ClientCtx[Req, Resp], next ResponseSeq[Resp]) ResponseSeq[Resp] Allowing more flexibility through chained invocations. This was necessary to allow user-defined interceptors, which previously was not possible to implement because ClientCtx was not exported and also the responses iterator was also not exported previously. This is now possible by exporting ClientCtx and augmenting the QuorumInterceptor func type with the next argument. Thus, we avoid reaching into the ClientCtx to get access to the "previous" response iterator.
This makes the ordering of the output consistent and simplifies the method and type generation.
This updates only the protobuf generated code. The _gorums.pb.go files will be updated in a follow up commit.
This also fixes a typo and returns status.Error instead of Errorf().
…rial This adds the storage.proto file used in the user-guide.md to the examples/gorumsexample folder. At the same time we ignore the generated code for this file to avoid churn when updating proto version or updating the proto-gen-gorums generator.
We no longer need MemoryStatList to be defined in the proto file, since no messages or RPCs use it and with the new iterator design we avoid the intricacies of the old custom return type having to be defined as a method option and as an actual proto message type.
This avoids the protobuf vs Go impedance mismatch for initialisms like message_id (in protobuf) which is compiled to MessageId in Go, which is not a recommended identifier in Go. To avoid this problem altogether, we simply change the field to message_seq_no, which becomes a valid Go identifier MessageSeqNo.
This adds the System helper for registration, starting and stopping servers and closing connections. This also implements proper io.Closer support in the Manager's Close() method, and prevents duplicate calls to the channel's close().
The ensureStream() method had a check-then-act race condition where concurrent callers (test goroutine and sender's eager connect) could both see no stream exists and each create separate streams. Move the lock acquisition and connection status check into newNodeStream() to make the operation atomic. Also remove a racy test assertion that checked for nil stream before the action, since the sender goroutine's eager connect could complete before the check.
This just moves the ensureStream method closer to the related newNodeStream method to help readability.
This replaces RWMutex with Mutex for stream and storage synchronization.
There is a small overhead with using RWMutex unless there is a
significant difference in read/write ratio in favor of reads. Benchmarks
indicate that there is a slight benefit to using Mutex (~1.6% geomean).
goos: darwin
goarch: arm64
pkg: github.com/relab/gorums
cpu: Apple M2 Max
│ old.txt │ new.txt │
│ sec/op │ sec/op vs base │
GetCallOptions/options=0-12 19.06n ± 1% 18.60n ± 5% ~ (p=0.105 n=10)
GetCallOptions/options=1-12 39.76n ± 1% 37.54n ± 4% -5.60% (p=0.000 n=10)
GetCallOptions/options=2-12 66.29n ± 5% 61.55n ± 0% -7.14% (p=0.000 n=10)
GetCallOptions/options=3-12 94.26n ± 6% 88.91n ± 1% -5.68% (p=0.011 n=10)
GetCallOptions/options=4-12 93.31n ± 1% 92.31n ± 1% ~ (p=0.159 n=10)
GetCallOptions/options=5-12 129.1n ± 1% 128.2n ± 1% -0.66% (p=0.009 n=10)
ChannelStreamReadyFirstRequest-12 495.8µ ± 21% 566.5µ ± 13% ~ (p=0.089 n=10)
ChannelStreamReadySubsequentRequest-12 48.20µ ± 1% 48.09µ ± 1% ~ (p=0.796 n=10)
ChannelStreamReadyReconnect-12 195.6µ ± 0% 196.3µ ± 1% ~ (p=0.063 n=10)
AsyncQuorumCall/AsyncMajority/3-12 97.27µ ± 2% 97.01µ ± 0% -0.27% (p=0.015 n=10)
AsyncQuorumCall/BlockingMajority/3-12 95.90µ ± 0% 95.73µ ± 3% ~ (p=0.853 n=10)
AsyncQuorumCall/AsyncMajority/5-12 145.1µ ± 1% 144.0µ ± 1% -0.76% (p=0.006 n=10)
AsyncQuorumCall/BlockingMajority/5-12 143.7µ ± 1% 142.8µ ± 1% -0.57% (p=0.043 n=10)
AsyncQuorumCall/AsyncMajority/7-12 171.7µ ± 1% 168.3µ ± 1% -1.96% (p=0.004 n=10)
AsyncQuorumCall/BlockingMajority/7-12 171.7µ ± 2% 166.7µ ± 1% -2.91% (p=0.000 n=10)
AsyncQuorumCall/AsyncMajority/9-12 183.8µ ± 2% 181.9µ ± 1% ~ (p=0.123 n=10)
AsyncQuorumCall/BlockingMajority/9-12 182.5µ ± 2% 181.9µ ± 1% ~ (p=0.631 n=10)
Correctable/QuorumCall/3-12 101.0µ ± 1% 101.6µ ± 1% ~ (p=0.075 n=10)
Correctable/QuorumCallIterator/3-12 96.37µ ± 2% 95.72µ ± 1% -0.67% (p=0.029 n=10)
Correctable/QuorumCallStream/3-12 97.91µ ± 1% 95.36µ ± 1% -2.61% (p=0.000 n=10)
Correctable/QuorumCallStreamIterator/3-12 93.67µ ± 0% 91.63µ ± 3% -2.18% (p=0.015 n=10)
Correctable/QuorumCall/5-12 152.9µ ± 3% 154.2µ ± 4% ~ (p=0.971 n=10)
Correctable/QuorumCallIterator/5-12 144.9µ ± 1% 148.3µ ± 1% +2.36% (p=0.000 n=10)
Correctable/QuorumCallStream/5-12 127.2µ ± 1% 129.2µ ± 1% +1.60% (p=0.002 n=10)
Correctable/QuorumCallStreamIterator/5-12 124.1µ ± 2% 124.4µ ± 2% ~ (p=0.481 n=10)
Correctable/QuorumCall/7-12 180.5µ ± 4% 179.5µ ± 2% ~ (p=0.353 n=10)
Correctable/QuorumCallIterator/7-12 176.9µ ± 2% 175.4µ ± 3% ~ (p=0.739 n=10)
Correctable/QuorumCallStream/7-12 150.7µ ± 2% 150.2µ ± 2% ~ (p=0.796 n=10)
Correctable/QuorumCallStreamIterator/7-12 146.9µ ± 1% 146.0µ ± 1% ~ (p=0.190 n=10)
Correctable/QuorumCall/9-12 196.5µ ± 4% 187.6µ ± 3% -4.51% (p=0.043 n=10)
Correctable/QuorumCallIterator/9-12 188.0µ ± 1% 186.9µ ± 3% ~ (p=0.247 n=10)
Correctable/QuorumCallStream/9-12 157.7µ ± 3% 156.8µ ± 2% ~ (p=0.739 n=10)
Correctable/QuorumCallStreamIterator/9-12 155.4µ ± 2% 154.5µ ± 2% ~ (p=0.315 n=10)
QuorumCallTerminalMethods/Majority/3-12 98.63µ ± 0% 98.32µ ± 1% ~ (p=0.796 n=10)
QuorumCallTerminalMethods/Threshold/3-12 101.95µ ± 3% 97.73µ ± 2% -4.13% (p=0.000 n=10)
QuorumCallTerminalMethods/First/3-12 87.17µ ± 4% 85.32µ ± 1% -2.12% (p=0.000 n=10)
QuorumCallTerminalMethods/All/3-12 110.9µ ± 1% 110.5µ ± 0% ~ (p=0.165 n=10)
QuorumCallTerminalMethods/Majority/5-12 147.3µ ± 1% 142.6µ ± 0% -3.14% (p=0.000 n=10)
QuorumCallTerminalMethods/Threshold/5-12 147.1µ ± 1% 142.6µ ± 0% -3.06% (p=0.000 n=10)
QuorumCallTerminalMethods/First/5-12 119.1µ ± 1% 118.1µ ± 1% ~ (p=0.063 n=10)
QuorumCallTerminalMethods/All/5-12 177.5µ ± 1% 176.1µ ± 2% ~ (p=0.101 n=10)
QuorumCallTerminalMethods/Majority/7-12 169.9µ ± 1% 168.1µ ± 1% ~ (p=0.052 n=10)
QuorumCallTerminalMethods/Threshold/7-12 170.5µ ± 2% 168.0µ ± 1% -1.44% (p=0.011 n=10)
QuorumCallTerminalMethods/First/7-12 133.0µ ± 2% 128.5µ ± 1% -3.40% (p=0.000 n=10)
QuorumCallTerminalMethods/All/7-12 234.3µ ± 2% 227.3µ ± 1% -2.95% (p=0.000 n=10)
QuorumCallTerminalMethods/Majority/9-12 184.4µ ± 1% 180.9µ ± 1% -1.92% (p=0.000 n=10)
QuorumCallTerminalMethods/Threshold/9-12 184.6µ ± 1% 181.3µ ± 1% -1.79% (p=0.000 n=10)
QuorumCallTerminalMethods/First/9-12 139.0µ ± 3% 137.4µ ± 0% ~ (p=0.143 n=10)
QuorumCallTerminalMethods/All/9-12 254.1µ ± 1% 254.8µ ± 0% ~ (p=0.075 n=10)
QuorumCallTerminalMethods/Majority/13-12 212.0µ ± 1% 211.6µ ± 1% ~ (p=1.000 n=10)
QuorumCallTerminalMethods/Threshold/13-12 211.6µ ± 1% 212.6µ ± 4% ~ (p=0.393 n=10)
QuorumCallTerminalMethods/First/13-12 162.3µ ± 1% 161.1µ ± 1% -0.75% (p=0.000 n=10)
QuorumCallTerminalMethods/All/13-12 294.4µ ± 1% 292.4µ ± 1% ~ (p=0.063 n=10)
QuorumCallTerminalMethods/Majority/17-12 248.1µ ± 0% 244.6µ ± 1% -1.39% (p=0.000 n=10)
QuorumCallTerminalMethods/Threshold/17-12 248.0µ ± 1% 243.5µ ± 2% -1.83% (p=0.000 n=10)
QuorumCallTerminalMethods/First/17-12 194.1µ ± 1% 189.4µ ± 1% -2.39% (p=0.000 n=10)
QuorumCallTerminalMethods/All/17-12 345.0µ ± 1% 328.8µ ± 2% -4.70% (p=0.000 n=10)
QuorumCallTerminalMethods/Majority/19-12 272.1µ ± 2% 261.6µ ± 0% -3.84% (p=0.000 n=10)
QuorumCallTerminalMethods/Threshold/19-12 271.9µ ± 1% 260.7µ ± 1% -4.13% (p=0.000 n=10)
QuorumCallTerminalMethods/First/19-12 208.3µ ± 1% 202.0µ ± 1% -3.02% (p=0.000 n=10)
QuorumCallTerminalMethods/All/19-12 352.6µ ± 1% 347.3µ ± 3% ~ (p=0.105 n=10)
QuorumCall/CollectAllThenCheck/3-12 111.4µ ± 3% 109.0µ ± 0% -2.12% (p=0.000 n=10)
QuorumCall/CollectN/3-12 96.21µ ± 0% 94.92µ ± 0% -1.34% (p=0.000 n=10)
QuorumCall/Iterator/3-12 96.11µ ± 0% 94.67µ ± 0% -1.50% (p=0.000 n=10)
QuorumCall/CollectAllThenCheck/5-12 175.7µ ± 0% 174.4µ ± 0% -0.77% (p=0.001 n=10)
QuorumCall/CollectN/5-12 142.7µ ± 2% 141.1µ ± 0% ~ (p=0.101 n=10)
QuorumCall/Iterator/5-12 144.3µ ± 2% 141.4µ ± 1% -2.01% (p=0.000 n=10)
QuorumCall/CollectAllThenCheck/7-12 231.1µ ± 1% 224.6µ ± 1% -2.80% (p=0.000 n=10)
QuorumCall/CollectN/7-12 170.8µ ± 2% 166.0µ ± 1% -2.81% (p=0.000 n=10)
QuorumCall/Iterator/7-12 171.2µ ± 2% 165.7µ ± 1% -3.26% (p=0.000 n=10)
QuorumCall/CollectAllThenCheck/9-12 260.1µ ± 2% 253.3µ ± 2% -2.62% (p=0.000 n=10)
QuorumCall/CollectN/9-12 183.9µ ± 1% 178.0µ ± 1% -3.20% (p=0.000 n=10)
QuorumCall/Iterator/9-12 183.8µ ± 1% 178.7µ ± 0% -2.76% (p=0.000 n=10)
QuorumCall/CollectAllThenCheck/13-12 296.7µ ± 1% 289.9µ ± 0% -2.32% (p=0.000 n=10)
QuorumCall/CollectN/13-12 212.1µ ± 1% 209.3µ ± 1% -1.33% (p=0.005 n=10)
QuorumCall/Iterator/13-12 212.4µ ± 1% 208.1µ ± 2% -2.02% (p=0.000 n=10)
QuorumCall/CollectAllThenCheck/17-12 336.0µ ± 2% 328.5µ ± 1% -2.23% (p=0.000 n=10)
QuorumCall/CollectN/17-12 248.2µ ± 0% 243.7µ ± 1% -1.82% (p=0.000 n=10)
QuorumCall/Iterator/17-12 249.7µ ± 1% 242.2µ ± 0% -3.03% (p=0.000 n=10)
QuorumCall/CollectAllThenCheck/19-12 368.1µ ± 1% 347.0µ ± 0% -5.73% (p=0.000 n=10)
QuorumCall/CollectN/19-12 271.7µ ± 3% 262.9µ ± 2% -3.24% (p=0.001 n=10)
QuorumCall/Iterator/19-12 271.5µ ± 1% 261.1µ ± 1% -3.83% (p=0.000 n=10)
geomean 96.14µ 94.61µ -1.60%
This renames the newNodeStream method to ensureConnectedNodeStream and updates the doc comments.
This was referenced Dec 26, 2025
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Fixes #245
Fixes #226
Fixes #217
Fixes #214
Fixes #208
Fixes #195
Fixes #192
Fixes #37
Fixes #11
Closes #35 as no longer relevant.
This PR introduces a significant architectural shift for Gorums, moving towards a more flexible, interceptor-based design and adopting modern Go iterator patterns. These changes simplify the codebase, improve type safety, and offer better composability for users.
🚀 Key Changes
1. Client-Side Interceptor Architecture
The core request/response logic has been refactored to use a composable interceptor pattern, similar to
connectrpc.QuorumFunc&Interceptor: Replaces the rigidQuorumSpecinterface. Users can now compose logic usingChainand standard interceptors.Multicast,Unicast, andQuorumCallnow share a more unified underlying implementation.2. Iterator-Based Response Handling (
Results[T])We have adopted a fluent, iterator-based API for handling responses, replacing the previous hardcoded callback/QuorumSpec approach.
Results[T]: A new type alias foriter.Seq[NodeResponse[T]]that serves as the receiver for helper methods.Fluent API: You can now chain methods for cleaner code:
Unified Node ID: The
NodeResponse[T]struct now contains theNodeID.3. Template Overhaul & Simplification
The
protoc-gen-gorumstemplates have been significantly simplified.QuorumSpecGeneration: The complexQuorumSpecinterface generationhas been removed. The runtime now handles this via generic
QuorumCallimplementations.4. Correctable Call Refactoring
Correctablecalls have been split into two distinct types to improve clarity and type safety:Correctable: For unary correctable calls (one request, progressive responses).CorrectableStream: For streaming correctable calls.This removes the ambiguous
serverStreamboolean toggle and simplifies the generated code.QuorumSpecInterface Removed: The generatedQuorumSpecinterface is gone.Custom quorum logic should now be implemented using the new
QuorumFuncorInterceptorpatterns, or by using the built-in aggregators (Majority,All,First).(like
(*Response, error)) now return a*Responsesobject, requiring a terminalmethod call (like
.Majority()) or iteration to retrieve the result.📝 Migration Guide
Migrating from QuorumSpec
Calls that relied on
QuorumSpecvalidation now require explicit aggregation or iteration.Old (Implicit QuorumSpec logic):
New (Explicit Aggregation):