Skip to content

Improve timeout handling in remote shards#5495

Merged
generall merged 2 commits intodevfrom
improve-timeout-remote-shard
Nov 22, 2024
Merged

Improve timeout handling in remote shards#5495
generall merged 2 commits intodevfrom
improve-timeout-remote-shard

Conversation

@agourlay
Copy link
Copy Markdown
Member

@agourlay agourlay commented Nov 20, 2024

Some queries require to perform several I/O action in a sequence before responding to the client.

Between each action, we decrease the timeout in order to track precisely the elapsed time to honor the timeout input from the client.

This logic triggers two different failure modes for the remote shard when:

  • the timeout is zero:
    • the remote shard does not accept timeout=0 as input
  • the timeout is between 0 and 1 sec
    • given that the value is floored, it triggers the previous case as well

This PR proposes to:

  • gate the access to remote shard with non zero timeout to shortcut the query processing
  • round up the timeout value to the next greater integer.
    • e.g 0.5 sec would become 1 sec instead of timing out
  • unify our timeout param. value with the one used for the gRPC connection timeout

This gives the following change:

Code Qdrant timeout gRPC timeout
dev floored sec untouched
PR round up sec round up sec

The new approach is more lenient as it gives more time to finish to all requests.

/// Used before making a request to the remote shard to avoid useless traffic.
pub fn check_timeout(timeout: Option<Duration>, operation: &str) -> CollectionResult<()> {
if timeout.map(|t| t.is_zero()).unwrap_or(false) {
Err(CollectionError::timeout(0, operation))
Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

we do not have the original timeout so the error is not great for the user here

@agourlay agourlay marked this pull request as ready for review November 21, 2024 16:22
@agourlay agourlay requested review from coszio and timvisee November 21, 2024 16:22
@generall generall merged commit 3dd66b3 into dev Nov 22, 2024
@generall generall deleted the improve-timeout-remote-shard branch November 22, 2024 21:18
timvisee pushed a commit that referenced this pull request Dec 9, 2024
* Improve timeout handling in remote shards

* cleaner impl
@timvisee timvisee mentioned this pull request Dec 9, 2024
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.

2 participants