Skip to content

Non blocking retrieve with timeout and cancellation support#4844

Merged
agourlay merged 2 commits intodevfrom
no-blocking-retrieve
Aug 8, 2024
Merged

Non blocking retrieve with timeout and cancellation support#4844
agourlay merged 2 commits intodevfrom
no-blocking-retrieve

Conversation

@agourlay
Copy link
Copy Markdown
Member

@agourlay agourlay commented Aug 7, 2024

Performing a retrieve operation at the segment level is a blocking operation:

  • the segment's read lock needs to be acquired
  • the vectors are fetched from storage
  • the payload are fetched from storage

This PR makes retrieving asynchronous & multithreaded so it does not block anymore Tokio threads which can cause various responsiveness issues.

It also applies additional measures to:

  • honor user timeout where possible
  • ensure cancellation of tasks

let records_map =
SegmentsSearcher::retrieve(self.segments(), &request.ids, with_payload, with_vector)?;
let timeout = timeout.unwrap_or(self.shared_storage_config.search_timeout);
let records_map = tokio::time::timeout(
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.

where the rubber meets the road

@agourlay agourlay marked this pull request as ready for review August 7, 2024 10:39
@agourlay agourlay requested a review from coszio August 7, 2024 10:40
@agourlay agourlay force-pushed the no-blocking-retrieve branch from 1da05ec to 3601a4c Compare August 7, 2024 15:14
Comment on lines +353 to +354
runtime_handle
.spawn_blocking({
Copy link
Copy Markdown
Member

@timvisee timvisee Aug 8, 2024

Choose a reason for hiding this comment

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

For reviewers, important part: here we now spawn blocking tasks for each segment segment holder.

Copy link
Copy Markdown
Member Author

@agourlay agourlay Aug 8, 2024

Choose a reason for hiding this comment

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

Having one task per segment is possible optimization (marked by a TODO below)

For now there is one task per segment_holder, so just one per request/shard :)

@agourlay agourlay merged commit 196f889 into dev Aug 8, 2024
@agourlay agourlay deleted the no-blocking-retrieve branch August 8, 2024 10:41
generall pushed a commit that referenced this pull request Aug 9, 2024
* Non blocking retrieve with timeout and cancellation support

* apply timeout for extra retrieve in rescoring
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