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

[Feature Request] cancel_after_time_interval and timeout parameters should remain one #11642

Open
kkewwei opened this issue Dec 19, 2023 · 5 comments
Labels
enhancement Enhancement or improvement to existing feature or request Search Search query, autocomplete ...etc

Comments

@kkewwei
Copy link
Contributor

kkewwei commented Dec 19, 2023

Describe the solution you'd like

As a user, when setting timeout, we want to return after timeout, regardless of whether it is a shard-level timeout or a request-level timeout. now we support cancel_after_time_interval and timeout, which are a bit redundant. In addition, if the user sets cancel_after_time_interval smaller than timeout, then timeout will have no effect.

Can we replace cancel_after_time_interval with timeout in some case? Implement timeout cancellation function on timeout, and remove the meaning of shard-level timeout on timeout.

Related component

Search

Describe alternatives you've considered

No response

Additional context

No response

@kkewwei kkewwei added enhancement Enhancement or improvement to existing feature or request untriaged labels Dec 19, 2023
@github-actions github-actions bot added the Search Search query, autocomplete ...etc label Dec 19, 2023
@macohen macohen removed the untriaged label Dec 20, 2023
@noCharger
Copy link
Contributor

@kkewwei there's a post on forum which might answer your question https://forum.opensearch.org/t/cancel-after-time-interval-clarification/15915/4

@kkewwei
Copy link
Contributor Author

kkewwei commented Dec 21, 2023

@noCharger https://forum.opensearch.org/t/cancel-after-time-interval-clarification/15915/4 only describe the meaning of configuration, what I want to describe is whether we should remove the shard-level timeout and only keep request-level timeout, this doesn't seem like one thing.

@sohami
Copy link
Collaborator

sohami commented Mar 29, 2024

@kkewwei you can read the discussion here around the same topic: #817. There are nuances around partial results and both the settings has different meaning.

@kkewwei
Copy link
Contributor Author

kkewwei commented Apr 5, 2024

@sohami I know the nuances, and I also think cancel_after_timeinterval is useful.

But from the user's perspective, I don't care shard-level timeout and request-level timeout, if a timeout is set, then we want to return directly(partial result or failure) after timeout. If we can merge cancel_after_time_interval and timeout?

@getsaurabh02 getsaurabh02 moved this from 🆕 New to Later (6 months plus) in Search Project Board Aug 15, 2024
@kkewwei
Copy link
Contributor Author

kkewwei commented Nov 17, 2024

@sohami @Bukhtawar @getsaurabh02 please help check, It seems that if timeout can be equal to cancel_after_time_interval in RestSearchAction.parseSearchRequest in some case:

        if (request.hasParam("cancel_after_time_interval")) {
            TimeValue cancelAfterTimeInterval = request.paramAsTime("cancel_after_time_interval", null);
            searchRequest.setCancelAfterTimeInterval(cancelAfterTimeInterval);
            TimeValue timeValue = searchRequest.source().timeout();
            if (timeValue == null || timeValue.compareTo(cancelAfterTimeInterval) > 0) {
                searchRequest.source().timeout(cancelAfterTimeInterval);
            }
        }

In product, timeout is widely used, but cancel_after_time_interval is too new to use rarely, so the important feature cannot be used.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement Enhancement or improvement to existing feature or request Search Search query, autocomplete ...etc
Projects
Status: Later (6 months plus)
Development

No branches or pull requests

4 participants