-
Notifications
You must be signed in to change notification settings - Fork 105
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
set the cancelAfterTimeInterval parameter on SearchRequest object in … #1366
set the cancelAfterTimeInterval parameter on SearchRequest object in … #1366
Conversation
…all MonitorRunners Signed-off-by: Riya Saxena <[email protected]>
alerting/src/main/kotlin/org/opensearch/alerting/AlertService.kt
Outdated
Show resolved
Hide resolved
Signed-off-by: Riya Saxena <[email protected]>
86d45f8
to
c39d147
Compare
Signed-off-by: Riya Saxena <[email protected]>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Would it be a pain to add some tests for these changes?
// The default value for the cancelAfterTimeInterval is -1 and so, in this case | ||
// we should ignore processing on the value | ||
val givenInterval = MonitorRunnerService.monitorCtx.cancelAfterTimeInterval!!.minutes | ||
if (givenInterval == -1L) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Does -1 get interpreted as no cancelAfterTimeout? I'm wondering if we should have a sensible default instead of defaulting to no cancelAfter. Could you elaborate on what conditions would necessitate a user changing this setting from -1 to a specific value?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
yes -1 is a default for cancel_after_time_interval. Suppose search.cancel_after_time_interval
is set at cluster with some higher value and the plugin overrides it to a lower value. Thus, max(cluster setting search.cancel_after_time_interval, plugin's default minimum value) should be used while overriding before calling _search from plugin.
Signed-off-by: Riya <[email protected]>
Signed-off-by: Riya Saxena <[email protected]>
Signed-off-by: Riya Saxena <[email protected]>
…rror Fix fix cancellation timeout error
The backport to
To backport manually, run these commands in your terminal: # Navigate to the root of your repository
cd $(git rev-parse --show-toplevel)
# Fetch latest updates from GitHub
git fetch
# Create a new working tree
git worktree add ../.worktrees/alerting/backport-2.x 2.x
# Navigate to the new working tree
pushd ../.worktrees/alerting/backport-2.x
# Create a new branch
git switch --create backport-1366-to-2.x
# Cherry-pick the merged commit of this pull request and resolve the conflicts
git cherry-pick -x --mainline 1 d2a590e744c920820536efc5e1a88a4185eeed74
# Push it to GitHub
git push --set-upstream origin backport-1366-to-2.x
# Go back to the original working tree
popd
# Delete the working tree
git worktree remove ../.worktrees/alerting/backport-2.x Then, create a pull request where the |
The backport to
To backport manually, run these commands in your terminal: # Navigate to the root of your repository
cd $(git rev-parse --show-toplevel)
# Fetch latest updates from GitHub
git fetch
# Create a new working tree
git worktree add ../.worktrees/alerting/backport-2.11 2.11
# Navigate to the new working tree
pushd ../.worktrees/alerting/backport-2.11
# Create a new branch
git switch --create backport-1366-to-2.11
# Cherry-pick the merged commit of this pull request and resolve the conflicts
git cherry-pick -x --mainline 1 d2a590e744c920820536efc5e1a88a4185eeed74
# Push it to GitHub
git push --set-upstream origin backport-1366-to-2.11
# Go back to the original working tree
popd
# Delete the working tree
git worktree remove ../.worktrees/alerting/backport-2.11 Then, create a pull request where the |
The backport to
To backport manually, run these commands in your terminal: # Navigate to the root of your repository
cd $(git rev-parse --show-toplevel)
# Fetch latest updates from GitHub
git fetch
# Create a new working tree
git worktree add ../.worktrees/alerting/backport-2.12 2.12
# Navigate to the new working tree
pushd ../.worktrees/alerting/backport-2.12
# Create a new branch
git switch --create backport-1366-to-2.12
# Cherry-pick the merged commit of this pull request and resolve the conflicts
git cherry-pick -x --mainline 1 d2a590e744c920820536efc5e1a88a4185eeed74
# Push it to GitHub
git push --set-upstream origin backport-1366-to-2.12
# Go back to the original working tree
popd
# Delete the working tree
git worktree remove ../.worktrees/alerting/backport-2.12 Then, create a pull request where the |
opensearch-project#1366) * set the cancelAfterTimeInterval parameter on SearchRequest object in all MonitorRunners Signed-off-by: Riya Saxena <[email protected]> * address the comments for pr 1366 Signed-off-by: Riya Saxena <[email protected]> * address the comments for pr 1366 Signed-off-by: Riya Saxena <[email protected]> * fix merge conflicts * fix merge conflicts Signed-off-by: Riya Saxena <[email protected]> * fix merge conflicts Signed-off-by: Riya Saxena <[email protected]> --------- Signed-off-by: Riya Saxena <[email protected]> Signed-off-by: Riya <[email protected]> (cherry picked from commit d2a590e)
…all MonitorRunners
#827
Set the cancelAfterTimeInterval parameter on SearchRequest object in all the MonitorRunners
CheckList:
By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.
For more information on following Developer Certificate of Origin and signing off your commits, please check here.