Skip to content
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

Part 1: Support for cancel_after_time_interval parameter in search and… #1085

Merged
merged 1 commit into from
Aug 12, 2021

Conversation

sohami
Copy link
Collaborator

@sohami sohami commented Aug 12, 2021

… msearch request (#986)

  • Part 1: Support for cancel_after_timeinterval parameter in search and msearch request

This commit introduces the new request level parameter to configure the timeout interval after which
a search request will be cancelled. For msearch request the parameter is supported both at parent
request and at sub child search requests. If it is provided at parent level and child search request
doesn't have it then the parent level value is set at such child request. The parent level msearch
is not used to cancel the parent request as it may be tricky to come up with correct value in cases
when child search request can have different runtimes

TEST: Added test for ser/de with new parameter

Signed-off-by: Sorabh Hamirwasia [email protected]

  • Part 2: Support for cancel_after_timeinterval parameter in search and msearch request

This commit adds the handling of the new request level parameter and schedule cancellation task. It
also adds a cluster setting to set a global cancellation timeout for search request which will be
used in absence of request level timeout.

TEST: Added new tests in SearchCancellationIT
Signed-off-by: Sorabh Hamirwasia [email protected]

  • Address Review feedback for Part 1

Signed-off-by: Sorabh Hamirwasia [email protected]

  • Address review feedback for Part 2

Signed-off-by: Sorabh Hamirwasia [email protected]

  • Update CancellableTask to remove the cancelOnTimeout boolean flag

Signed-off-by: Sorabh Hamirwasia [email protected]

  • Replace search.cancellation.timeout cluster setting with search.enforce_server.timeout.cancellation to control if cluster level cancel_after_time_interval should take precedence over request level cancel_after_time_interval value

Signed-off-by: Sorabh Hamirwasia [email protected]

  • Removing the search.enforce_server.timeout.cancellation cluster setting and just keeping search.cancel_after_time_interval setting with request level parameter taking the precedence.

Signed-off-by: Sorabh Hamirwasia [email protected]

Co-authored-by: Sorabh Hamirwasia [email protected]

Description

This is cherry-pick of this commit from main branch

Issues Resolved

#817

Check List

  • [Y] New functionality includes testing.
    • [Y] All tests pass
  • New functionality has been documented.
    • New functionality has javadoc added
  • [Y] Commits are signed per the DCO using --signoff

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.

… msearch request (opensearch-project#986)

* Part 1: Support for cancel_after_timeinterval parameter in search and msearch request

This commit introduces the new request level parameter to configure the timeout interval after which
a search request will be cancelled. For msearch request the parameter is supported both at parent
request and at sub child search requests. If it is provided at parent level and child search request
doesn't have it then the parent level value is set at such child request. The parent level msearch
is not used to cancel the parent request as it may be tricky to come up with correct value in cases
when child search request can have different runtimes

TEST: Added test for ser/de with new parameter

Signed-off-by: Sorabh Hamirwasia <[email protected]>

* Part 2: Support for cancel_after_timeinterval parameter in search and msearch request

This commit adds the handling of the new request level parameter and schedule cancellation task. It
also adds a cluster setting to set a global cancellation timeout for search request which will be
used in absence of request level timeout.

TEST: Added new tests in SearchCancellationIT
Signed-off-by: Sorabh Hamirwasia <[email protected]>

* Address Review feedback for Part 1

Signed-off-by: Sorabh Hamirwasia <[email protected]>

* Address review feedback for Part 2

Signed-off-by: Sorabh Hamirwasia <[email protected]>

* Update CancellableTask to remove the cancelOnTimeout boolean flag

Signed-off-by: Sorabh Hamirwasia <[email protected]>

* Replace search.cancellation.timeout cluster setting with search.enforce_server.timeout.cancellation to control if cluster level cancel_after_time_interval should take precedence over request level cancel_after_time_interval value

Signed-off-by: Sorabh Hamirwasia <[email protected]>

* Removing the search.enforce_server.timeout.cancellation cluster setting and just keeping search.cancel_after_time_interval setting with request level parameter taking the precedence.

Signed-off-by: Sorabh Hamirwasia <[email protected]>

Co-authored-by: Sorabh Hamirwasia <[email protected]>
@opensearch-ci-bot
Copy link
Collaborator

✅   Gradle Wrapper Validation success 70eabe5

@opensearch-ci-bot
Copy link
Collaborator

✅   DCO Check Passed 70eabe5

@dblock
Copy link
Member

dblock commented Aug 12, 2021

start gradle check

@dblock dblock changed the title Part 1: Support for cancel_after_timeinterval parameter in search and… Part 1: Support for cancel_after_time_interval parameter in search and… Aug 12, 2021
@opensearch-ci-bot
Copy link
Collaborator

✅   Gradle Precommit success 70eabe5

@opensearch-ci-bot
Copy link
Collaborator

✅   Gradle Check success 70eabe5
Log 391

Reports 391

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants