feat: PRT: Client side load balancing with stickiness #1564
Conversation
WalkthroughThe recent updates significantly enhance the application's capabilities by integrating load testing functionalities, refining session management, and improving error handling. New identifiers for provider tracking and enhanced logging mechanisms bolster the system's robustness and usability. Overall, these changes boost performance tracking, reliability, and the overall usability of the system. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant CLI
participant LoadTest
participant ConsumerSession
participant RPCConsumerServer
User->>CLI: Execute load test command
CLI->>LoadTest: Start load test with parameters
LoadTest->>ConsumerSession: Manage session and connections
ConsumerSession->>RPCConsumerServer: Send relay requests
RPCConsumerServer-->>ConsumerSession: Return responses
ConsumerSession-->>LoadTest: Provide results
LoadTest-->>CLI: Display results to user
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (invoked as PR comments)
Additionally, you can add CodeRabbit Configuration File (
|
shleikes
force-pushed
the
PRT-client-side-load-balancing-with-stickiness
branch
from
20a52df
to
769c82f
Compare
shleikes
force-pushed
the
PRT-client-side-load-balancing-with-stickiness
branch
from
769c82f
to
38c7800
Compare
shleikes
force-pushed
the
PRT-client-side-load-balancing-with-stickiness
branch
from
f1117e5
to
38c7800
Compare
…ad-balancing-with-stickiness
shleikes
force-pushed
the
PRT-client-side-load-balancing-with-stickiness
branch
from
48220eb
to
f5ad786
Compare
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (2)
- protocol/loadtest/testing.go (1 hunks)
- protocol/rpcconsumer/rpcconsumer_server.go (3 hunks)
Files skipped from review as they are similar to previous changes (2)
- protocol/loadtest/testing.go
- protocol/rpcconsumer/rpcconsumer_server.go
There was a problem hiding this comment.
Actionable comments posted: 0
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (3)
- cmd/lavap/main.go (2 hunks)
- protocol/lavasession/single_consumer_session.go (2 hunks)
- protocol/rpcconsumer/rpcconsumer_server.go (8 hunks)
Files skipped from review due to trivial changes (1)
- cmd/lavap/main.go
Files skipped from review as they are similar to previous changes (1)
- protocol/lavasession/single_consumer_session.go
Additional comments not posted (8)
protocol/rpcconsumer/rpcconsumer_server.go (8)
37-37: LGTM!The constants
InitRelaysCountandSendRelayAttemptsare defined correctly.
142-152: LGTM!The
sendCraftedRelaysWrapperfunction is updated to include a loop for sending initial relays and retries.
Line range hint
252-269: LGTM!The
sendCraftedRelaysfunction is updated to callsendRelayWithRetrieswith the updated signature.
822-823: LGTM!The code correctly appends the load balancer unique ID to the outgoing context metadata.
835-860: LGTM!The code correctly logs and verifies the provider's unique ID.
959-960: LGTM!The code correctly appends the load balancer unique ID to the outgoing context metadata.
142-152: LGTM!The
sendCraftedRelaysWrapperfunction is updated to include a loop for sending initial relays and retries.
Line range hint
208-239: LGTM! But verify the function usage in the codebase.The
sendRelayWithRetriesfunction is updated to handle retries and log errors more effectively.However, ensure that all function calls to
sendRelayWithRetriesmatch the new signature.Verification successful
LGTM! The function
sendRelayWithRetriesis used consistently with the new signature across the codebase.
- The only occurrences of
sendRelayWithRetriesare within theprotocol/rpcconsumer/rpcconsumer_server.gofile.- The usage matches the new function signature.
Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify all function calls to `sendRelayWithRetries` match the new signature. # Test: Search for the function usage. Expect: Only occurrences of the new signature. rg --type go -A 5 $'sendRelayWithRetries'Length of output: 1416
Script:
#!/bin/bash # Description: Verify all function calls to `sendRelayWithRetries` match the new signature across the entire codebase. # Test: Search for all function usages. Expect: Only occurrences of the new signature. rg 'sendRelayWithRetries\(' --type goLength of output: 372
There was a problem hiding this comment.
Actionable comments posted: 0
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (12)
- protocol/integration/protocol_test.go (6 hunks)
- protocol/lavasession/consumer_session_manager.go (6 hunks)
- protocol/lavasession/consumer_types.go (5 hunks)
- protocol/monitoring/health.go (1 hunks)
- protocol/performance/connection/prober.go (3 hunks)
- protocol/rpcconsumer/consumer_state_tracker_mock.go (1 hunks)
- protocol/rpcconsumer/relayer_client_mock.go (1 hunks)
- protocol/rpcconsumer/rpcconsumer_server.go (10 hunks)
- protocol/rpcconsumer/rpcconsumer_server_test.go (1 hunks)
- protocol/rpcprovider/rpcprovider_server.go (4 hunks)
- protocol/rpcprovider/testing.go (2 hunks)
- utils/rand/rand.go (1 hunks)
Files skipped from review due to trivial changes (3)
- protocol/monitoring/health.go
- protocol/rpcconsumer/consumer_state_tracker_mock.go
- protocol/rpcconsumer/relayer_client_mock.go
Files skipped from review as they are similar to previous changes (3)
- protocol/lavasession/consumer_session_manager.go
- protocol/rpcconsumer/rpcconsumer_server.go
- protocol/rpcprovider/rpcprovider_server.go
Additional comments not posted (27)
protocol/performance/connection/prober.go (3)
Line range hint
42-52: LGTM! Simplifies the code.The update to directly use
p.relayerClientinstead of dereferencing it enhances readability and reduces potential errors.
25-27: LGTM! But verify the function calls tocreateConnection.The change to return a value type for
relayerClientensures consistency with theProberstruct.Ensure that all function calls to
createConnectioncorrectly handle the value type return.Verification successful
Function calls to
createConnectioninprober.gocorrectly handle the value type return.The function calls in
prober.gocorrectly assign the returnedrelayerClientvalue top.relayerClient, ensuring consistency with theProberstruct.
protocol/performance/connection/prober.go: relayer, conn, err := createConnection(ctx, p.address)Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify the function calls to `createConnection` handle the value type return. # Test: Search for the function calls to `createConnection`. Expect: Correct handling of value type return. rg --type go -A 5 $'createConnection'Length of output: 3984
18-18: LGTM! But verify the instantiation ofrelayerClient.The change to use a value type for
relayerClientimproves readability and reduces potential errors associated with pointer dereferencing.Ensure that the
relayerClientis correctly instantiated as a value type throughout the codebase.Verification successful
Verified: The instantiation of
relayerClientas a value type is correct.The
relayerClientis instantiated and used appropriately as a value type throughout the codebase, particularly inprotocol/performance/connection/prober.go.
protocol/performance/connection/prober.go: Lines whererelayerClientis assigned and used.Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify the instantiation of `relayerClient` as a value type. # Test: Search for the instantiation of `relayerClient`. Expect: Only value type instantiations. rg --type go -A 5 $'relayerClient: pairingtypes.RelayerClient'Length of output: 63
Script:
#!/bin/bash # Description: Verify the instantiation of `relayerClient` as a value type. # Test: Search for the instantiation of `relayerClient` with a broader pattern. rg --type go -A 10 'relayerClient'Length of output: 13492
utils/rand/rand.go (8)
94-98: LGTM! Enhances robustness.The addition of
PanicIfProtocolRandNotInitializedensures that the random number generation functions cannot be called unless the random seed has been properly initialized.
101-102: LGTM! Ensures proper initialization.The update to call
PanicIfProtocolRandNotInitializedat the beginning ofIntnensures that the random seed is properly initialized.
106-107: LGTM! Ensures proper initialization.The update to call
PanicIfProtocolRandNotInitializedat the beginning ofFloat64ensures that the random seed is properly initialized.
111-112: LGTM! Ensures proper initialization.The update to call
PanicIfProtocolRandNotInitializedat the beginning ofUint32ensures that the random seed is properly initialized.
116-117: LGTM! Ensures proper initialization.The update to call
PanicIfProtocolRandNotInitializedat the beginning ofUint64ensures that the random seed is properly initialized.
121-122: LGTM! Ensures proper initialization.The update to call
PanicIfProtocolRandNotInitializedat the beginning ofInt63ensures that the random seed is properly initialized.
126-127: LGTM! Ensures proper initialization.The update to call
PanicIfProtocolRandNotInitializedat the beginning ofInt63nensures that the random seed is properly initialized.
131-132: LGTM! Ensures proper initialization.The update to call
PanicIfProtocolRandNotInitializedat the beginning ofNormFloat64ensures that the random seed is properly initialized.protocol/rpcconsumer/rpcconsumer_server_test.go (3)
26-76: LGTM! Verify the initialization of components.The
createRpcConsumerfunction appears to be well-structured and sets up various components required for the operation ofRPCConsumerServer.Ensure that all components are correctly initialized and that any potential errors are handled appropriately.
Verification successful
Initialization of components in
createRpcConsumeris verified and correct.The function
createRpcConsumerinitializes various components and handles errors appropriately usingrequire.NoErrorandrequire.NotNil. The function is utilized in multiple test cases, confirming its correctness and importance in the test suite.
protocol/rpcconsumer/rpcconsumer_server_test.goprotocol/integration/protocol_test.goScripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify the initialization of components in `createRpcConsumer`. # Test: Search for the initialization of components. Expect: Correct initialization and error handling. rg --type go -A 5 $'createRpcConsumer'Length of output: 4893
78-103: LGTM! Verify the signing of the relay reply.The
handleRelayfunction appears to be well-structured and handles a relay request appropriately.Ensure that the relay reply is correctly signed and that any potential errors are handled appropriately.
Verification successful
Verification successful!
The
handleRelayfunction correctly signs the relay reply and handles potential errors appropriately. The test file includes assertions to verify the signing and error handling.
require.NoError(t, err)checks are present after signing operations within thehandleRelayfunction.- Additional assertions in the test setup confirm proper error handling and object initialization.
Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify the signing of the relay reply in `handleRelay`. # Test: Search for the signing of the relay reply. Expect: Correct signing and error handling. rg --type go -A 5 $'handleRelay'Length of output: 4801
Script:
#!/bin/bash # Description: Verify the usage and testing of the `handleRelay` function to ensure correct signing and error handling. # Search for the usage of `handleRelay` in the test files. rg 'handleRelay' --type go -A 10Length of output: 8745
Script:
#!/bin/bash # Description: Verify assertions or checks related to the relay reply's signature and error handling in the `rpcconsumer_server_test.go` file. # Search for assertions or checks in the test file. rg 'require\.(NoError|NotNil|Equal)' protocol/rpcconsumer/rpcconsumer_server_test.go -A 5Length of output: 2121
105-183: LGTM! Verify the test cases.The
TestRelayInnerProviderUniqueIdFlowfunction appears to be well-structured and comprehensive.Ensure that all test cases are covered and that any potential errors are handled appropriately.
Verification successful
LGTM! Verify the test cases.
The
TestRelayInnerProviderUniqueIdFlowfunction is comprehensive and well-structured, covering all necessary test cases for therelayInnermethod's handling of the provider unique ID.
- Initial setting of the provider unique ID.
- Verification that the same provider unique ID passes.
- Changing the provider unique ID and verifying that it fails.
- Reverting to the original provider unique ID and verifying that it passes again.
Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify the test cases in `TestRelayInnerProviderUniqueIdFlow`. # Test: Search for the test cases. Expect: Comprehensive test coverage and error handling. rg --type go -A 5 $'TestRelayInnerProviderUniqueIdFlow'Length of output: 993
Script:
#!/bin/bash # Description: Extract the complete content of the `TestRelayInnerProviderUniqueIdFlow` function for thorough review. # Search for the complete function definition and its content. rg --type go -A 50 $'func TestRelayInnerProviderUniqueIdFlow(t *testing.T) {'Length of output: 252
Script:
#!/bin/bash # Description: Extract the complete content of the `TestRelayInnerProviderUniqueIdFlow` function for thorough review. # Search for the complete function definition and its content. rg --type go -A 50 $'func TestRelayInnerProviderUniqueIdFlow\\(t \\*testing\\.T\\) \\{'Length of output: 4485
Script:
#!/bin/bash # Description: Extract the complete content of the `TestRelayInnerProviderUniqueIdFlow` function for thorough review. # Search for the complete function definition and its content, increasing the number of lines fetched. rg --type go -A 100 $'func TestRelayInnerProviderUniqueIdFlow\\(t \\*testing\\.T\\) \\{'Length of output: 6554
protocol/rpcprovider/testing.go (3)
134-134: LGTM! Simplified variable declaration.The
relayerClientvariable is now declared as a value type instead of a pointer, improving readability and maintainability.
142-142: LGTM! Simplified assignment.The
relayerClientvariable is directly assigned the result ofpairingtypes.NewRelayerClient(conn), removing the need for a temporary variable.
144-144: LGTM! Simplified assignment.The
relayerClientvariable is directly assigned the result ofcswp.ConnectRawClientWithTimeout, removing the need for a temporary variable.protocol/lavasession/consumer_types.go (4)
106-106: LGTM! Added field for tracking blocked endpoints.The
blockListedfield is added to theEndpointConnectionstruct, improving control flow during connection management.
107-107: LGTM! Added field for unique identification of connections.The
lbUniqueIdfield is added to theEndpointConnectionstruct, enhancing the uniqueness and traceability of connections.
113-115: LGTM! Added method for accessing unique identifier.The
GetLbUniqueIdmethod is added to theEndpointConnectionstruct, enhancing encapsulation of connection details.
476-479: LGTM! Added logic for skipping blocked connections.The
fetchEndpointConnectionFromConsumerSessionWithProvidermethod is modified to include logic for checking theblockListedstatus ofEndpointConnectioninstances, enhancing the robustness of connection handling.protocol/integration/protocol_test.go (6)
175-175: LGTM! Updated function signature to include unique identifier.The
createRpcProviderfunction signature is modified to include an additional parameter,providerUniqueId, enhancing its functionality.
203-206: LGTM! Utilized unique identifier parameter.The
providerUniqueIdparameter is used to set the unique identifier for therpcProviderServerif it is not an empty string, allowing for more granular identification of RPC providers.
299-299: LGTM! Updated function call to include unique identifier.The
createRpcProviderfunction call is modified to include theproviderUniqueIdparameter, which is dynamically assigned usingfmt.Sprintf("provider%d", i), ensuring that each provider instance can be distinctly identified.
376-376: LGTM! Updated function call to include unique identifier.The
createRpcProviderfunction call is modified to include theproviderUniqueIdparameter, which is dynamically assigned usingfmt.Sprintf("provider%d", i), ensuring that each provider instance can be distinctly identified.
511-511: LGTM! Updated function call to include unique identifier.The
createRpcProviderfunction call is modified to include theproviderUniqueIdparameter, which is dynamically assigned usingfmt.Sprintf("provider%d", i), ensuring that each provider instance can be distinctly identified.
618-618: LGTM! Updated function call to include unique identifier.The
createRpcProviderfunction call is modified to include theproviderUniqueIdparameter, which is dynamically assigned usingfmt.Sprintf("provider%d", i), ensuring that each provider instance can be distinctly identified.
There was a problem hiding this comment.
Actionable comments posted: 0
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (1)
- protocol/rpcconsumer/rpcconsumer_server.go (5 hunks)
Files skipped from review as they are similar to previous changes (1)
- protocol/rpcconsumer/rpcconsumer_server.go
There was a problem hiding this comment.
Actionable comments posted: 0
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files ignored due to path filters (1)
x/spec/types/api_collection.pb.gois excluded by!**/*.pb.go
Files selected for processing (10)
- protocol/chainlib/common.go (1 hunks)
- protocol/common/endpoints.go (1 hunks)
- protocol/integration/protocol_test.go (7 hunks)
- protocol/lavasession/consumer_session_manager.go (6 hunks)
- protocol/lavasession/used_providers.go (1 hunks)
- protocol/rpcconsumer/consumer_state_tracker_mock.go (1 hunks)
- protocol/rpcconsumer/rpcconsumer_server.go (5 hunks)
- protocol/rpcconsumer/rpcconsumer_server_test.go (1 hunks)
- protocol/rpcprovider/rpcprovider.go (3 hunks)
- protocol/rpcprovider/rpcprovider_server.go (4 hunks)
Files skipped from review due to trivial changes (1)
- protocol/rpcconsumer/consumer_state_tracker_mock.go
Files skipped from review as they are similar to previous changes (8)
- protocol/chainlib/common.go
- protocol/common/endpoints.go
- protocol/integration/protocol_test.go
- protocol/lavasession/consumer_session_manager.go
- protocol/lavasession/used_providers.go
- protocol/rpcconsumer/rpcconsumer_server_test.go
- protocol/rpcprovider/rpcprovider.go
- protocol/rpcprovider/rpcprovider_server.go
Additional comments not posted (4)
protocol/rpcconsumer/rpcconsumer_server.go (4)
239-245: Enhanced logging looks good.The structured logging approach improves readability and consistency.
907-912: Improved metadata handling looks good.Appending metadata to the outgoing context enhances tracking and debugging capabilities.
930-961: Provider unique ID verification is robust.The changes ensure the integrity of the provider's unique ID, improving the robustness of the relay process.
1063-1066: Enhanced metadata handling looks good.Appending the load balancer unique ID and consumer process GUID to the outgoing context improves tracking and debugging capabilities.
Description
Closes: #XXXX
Author Checklist
All items are required. Please add a note to the item if the item is not applicable and
please add links to any relevant follow up issues.
I have...
!in the type prefix if API or client breaking changemainbranchReviewers Checklist
All items are required. Please add a note if the item is not applicable and please add
your handle next to the items reviewed if you only reviewed selected items.
I have...
Summary by CodeRabbit
New Features
Bug Fixes
Enhancements
Chores