Update to oom flags on search commands and fix error propagation in coordinator / iscancelled cases#901
Merged
KarthikSubbarao merged 16 commits intovalkey-io:mainfrom Mar 12, 2026
Merged
Conversation
…ext-rax-target-mutex-pool-size to Dev configs Signed-off-by: KarthikSubbarao <karthikrs2021@gmail.com>
Signed-off-by: KarthikSubbarao <karthikrs2021@gmail.com>
daddaman-amz
approved these changes
Mar 12, 2026
Member
Author
|
The PR here was merged incorrectly (using Merge a Commit), so I have this PR now as a redo |
rileydes-improving
pushed a commit
to Bit-Quill/valkey-search
that referenced
this pull request
Mar 16, 2026
…oordinator / iscancelled cases (valkey-io#901) The PR does the following: Update to oom flags on search commands. We are marking search commands with the denyoom flags following what was done since 1.0. Updated Error Tracking / Propagation to ensure we do not always return the "timeout" message even we cancel a fanout operation. Updated the normal code path of search commands (commands.cc) to first attempt replying with an error message based on the status. If no error, then check if canceled and return the standard timeout error. Note: The logic for early cancellation when partial results are disabled is still retained in this PR. If there is a timeout when partial results are disabled, we cancel without waiting and return the timed out error message. The parts which are new are the following: We have added handling for the case (with partial results disabled) and one of the fanouts fails with OOM. We now have correct with error propagation for it It also adds handling for the case (with partial results enabled) where we have not timed out, and all nodes have failed. Since it tracks the first error message, we are able to reply with the error now --------- Signed-off-by: KarthikSubbarao <karthikrs2021@gmail.com>
rileydes-improving
pushed a commit
to Bit-Quill/valkey-search
that referenced
this pull request
Mar 16, 2026
…oordinator / iscancelled cases (valkey-io#901) The PR does the following: Update to oom flags on search commands. We are marking search commands with the denyoom flags following what was done since 1.0. Updated Error Tracking / Propagation to ensure we do not always return the "timeout" message even we cancel a fanout operation. Updated the normal code path of search commands (commands.cc) to first attempt replying with an error message based on the status. If no error, then check if canceled and return the standard timeout error. Note: The logic for early cancellation when partial results are disabled is still retained in this PR. If there is a timeout when partial results are disabled, we cancel without waiting and return the timed out error message. The parts which are new are the following: We have added handling for the case (with partial results disabled) and one of the fanouts fails with OOM. We now have correct with error propagation for it It also adds handling for the case (with partial results enabled) where we have not timed out, and all nodes have failed. Since it tracks the first error message, we are able to reply with the error now --------- Signed-off-by: KarthikSubbarao <karthikrs2021@gmail.com>
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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
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.
The PR does the following:
Update to oom flags on search commands. We are marking search commands with the denyoom flags following what was done since 1.0.
Updated Error Tracking / Propagation to ensure we do not always return the "timeout" message even we cancel a fanout operation.
Updated the normal code path of search commands (commands.cc) to first attempt replying with an error message based on the status. If no error, then check if canceled and return the standard timeout error.
Note: The logic for early cancellation when partial results are disabled is still retained in this PR. If there is a timeout when partial results are disabled, we cancel without waiting and return the timed out error message.
The parts which are new are the following:
We have added handling for the case (with partial results disabled) and one of the fanouts fails with OOM. We now have correct with error propagation for it
It also adds handling for the case (with partial results enabled) where we have not timed out, and all nodes have failed. Since it tracks the first error message, we are able to reply with the error now