feat(relay): enforce RelayCountOnNodeError and add separate protocol error retries #2201
+813
−41
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
Codecov Report✅ All modified and coverable lines are covered by tests.
Flags with carried forward coverage won't be shown. Click here to find out more.
... and 186 files with indirect coverage changes 🚀 New features to boost your workflow:
|
nimrod-teich
requested review from
NadavLevi,
Tomelia1999 and
avitenzer
and removed request for
Tomelia1999
NadavLevi
previously approved these changes
Jan 27, 2026
Test Results 6 files - 1 83 suites - 2 31m 5s ⏱️ -44s For more details on these errors, see this check. Results for commit e19729e. ± Comparison against base commit 2ae0fd6. This pull request removes 177 and adds 12 tests. Note that renamed tests count towards both.♻️ This comment has been updated with latest results. |
nimrod-teich
force-pushed
the
fix/relay-count-on-node-error-retry-limit
branch
from
b1fa4d2 to
6a7f5b3
Compare
nimrod-teich
force-pushed
the
fix/relay-count-on-node-error-retry-limit
branch
2 times, most recently
from
4426b64 to
3844817
Compare
nimrod-teich
changed the title
…error retries When quorum is disabled (no lava-quorum-* headers), RelayCountOnNodeError now acts as a hard limit on the total number of retry batches. Setting RelayCountOnNodeError=0 effectively disables all retries on node errors. When quorum is enabled, quorumParams.Max is used as the retry limit instead, allowing quorum functionality to work independently of RelayCountOnNodeError. Additionally, introduces a new `--set-retry-count-on-protocol-error` CLI flag that controls protocol error retries independently from node error retries. Changes: - Modified retryCondition() in both smart router and consumer state machines to respect RelayCountOnNodeError when quorum is disabled - Updated HasRequiredNodeResults() to count node errors as "results" when RelayCountOnNodeError=0 to prevent quorum logic from triggering retries - Added HasRequiredNodeResults() return value for protocol error count - Update retryCondition to apply appropriate limit based on error type: - Only node errors: use RelayCountOnNodeError - Only protocol errors: use RelayCountOnProtocolError - Both: use max of both limits - Add RelayCountOnProtocolError variable and CLI flags - Added comprehensive tests for retry limit behavior This allows users to disable node error retries (--set-retry-count-on-node-error=0) while still allowing protocol error retries for transient connection issues.
nimrod-teich
force-pushed
the
fix/relay-count-on-node-error-retry-limit
branch
from
3844817 to
317904a
Compare
Tomelia1999
reviewed
Jan 28, 2026
| // Quorum feature disabled: check if we have enough results for quorum | ||
| retryForQuorumNeeded = !(resultsCount >= neededForQuorum) | ||
| // When RelayCountOnNodeError is 0, treat node errors as "results" for quorum purposes | ||
| // This ensures that when retries are disabled, we don't retry to replace node errors |
Contributor
There was a problem hiding this comment.
There will still be retry on nodeErrors.
Imagine the following case:
RelayCountOnNodeError = 2
it will go to the ELSE and won't take into account how much nodeErrors there are, only how much resultsCount there are
Updated the retryCondition function in both ConsumerRelayStateMachine and SmartRouterRelayStateMachine to streamline the handling of retry limits. Removed redundant checks for both node and protocol errors, allowing the maximum of both limits to be used in cases of either error type or no errors. This change enhances clarity and maintains the intended functionality of retry behavior.
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.
When quorum is disabled (no lava-quorum-* headers), RelayCountOnNodeError
now acts as a hard limit on the total number of retry batches. Setting
RelayCountOnNodeError=0 effectively disables all retries on node errors.
When quorum is enabled, quorumParams.Max is used as the retry limit instead,
allowing quorum functionality to work independently of RelayCountOnNodeError.
Additionally, introduces a new
--set-retry-count-on-protocol-errorCLI flagthat controls protocol error retries independently from node error retries.
Changes:
to respect RelayCountOnNodeError when quorum is disabled
RelayCountOnNodeError=0 to prevent quorum logic from triggering retries
This allows users to disable node error retries (--set-retry-count-on-node-error=0)
while still allowing protocol error retries for transient connection issues.
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...