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
ISPN-9469 Implement timeout for queries #8382
ISPN-9469 Implement timeout for queries #8382
Conversation
CI is GREEN |
@@ -82,19 +86,15 @@ public void resetQuery() { | |||
} else { | |||
Comparator<Comparable<FilterResult>[]> comparator = getComparator(); | |||
if (comparator == null) { | |||
results = StreamSupport.stream(spliterator(), false).collect(Collectors.toList()); | |||
results = StreamSupport.stream(spliterator(), false).collect(new TimedCollector<>(Collectors.toList(), timeout)); |
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.
So the timeout is detected when collecting. But what if the splitterator blocks indefinitely when trying to fetch next?
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.
It will probably timeout after the RpcTimeout
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.
I could also set the timeout in the stream itself, but only distributed streams support timeout
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.
changed
f0af2df
to
e597450
Compare
* | ||
* @since 11.0 | ||
*/ | ||
public class SearchTimeoutException extends RuntimeException { |
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.
I guess the days of dsl package are numbered. could we maybe move this to top level query package ?
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.
moved
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.
Ok. Now I'm going to worry about split packages :) Not sure if that is still relevant for java modules, or osgi.
e597450
to
5f5600c
Compare
Merged in master. Thanks @gustavonalle ! |
https://issues.redhat.com/browse/ISPN-9469