quickwit-oss / quickwit-oss/quickwit

Test the opportunity to not abort timed out get_slice, but instead run a race with retried attempt

Open
#5,468 0 comments 0 reactions 0 assignees View on GitHub

Nobody has claimed this yet.

enhancement
Dominant language
Rust
Stars
11.7k
Forks
597
Avg merge
2d 22h
Merged PRs (30d)
37

Description

In #5466 we introduced aggressive timeout/retries to improve latency.
We could avoid abort attempts on timeout and instead run a race with the new attempts.

As suggested by @trinity-1686a

/// Downloads a slice of a file from the storage, and returns an in memory buffer
    async fn get_slice(&self, path: &Path, range: Range<usize>) -> StorageResult<OwnedBytes> {
        let mut futures_unordered = FuturesUnordered::new();
        let num_bytes = range.len();

        for (attempt_id, timeout_duration) in self
            .storage_timeout_policy
            .compute_timeout(num_bytes)
            .enumerate()
        {
            let get_slice_fut = self.underlying.get_slice(path, range.clone());
            futures_unordered.push(get_slice_fut);
            match tokio::time::timeout(timeout_duration, futures_unordered.next()).await {
                Ok(Some(result)) => {
                    crate::STORAGE_METRICS
                        .get_slice_timeout_successes
                        .get(attempt_id)
                        .or(crate::STORAGE_METRICS.get_slice_timeout_successes.last())
                        .unwrap()
                        .inc();
                    return result;
                }
                Ok(None) => {
                    // ..
                }
                Err(_elapsed) => {
                    rate_limited_info!(limit_per_min=60, num_bytes=num_bytes, path=%path.display(), timeout_secs=timeout_duration.as_secs_f32(), "get timeout elapsed");
                    continue;
                }
            }
        }
        rate_limited_warn!(limit_per_min=60, num_bytes=num_bytes, path=%path.display(), "all get_slice attempts timeouted");
        crate::STORAGE_METRICS.get_slice_timeout_all_timeouts.inc();
        return Err(
            StorageErrorKind::Timeout.with_error(anyhow::anyhow!("internal timeout on get_slice"))
        );
    }

Contributor guide

Open the contributing guide

First steps

  1. Read the whole issue, then the project's contributing guide.
  2. Comment on the issue to say you are picking it up — it saves two people doing the same work.
  3. Fork the repository and make your change on a branch.
  4. Open a pull request that references the issue number.

Research direction

Start by reviewing issue #5466 and the shown async get_slice implementation, including its timeout policy, retries, and timeout metrics. Investigate whether timed-out attempts can continue while new attempts race, and define completion as confirming the desired race behavior without breaking timeout handling or metrics.

Written by the indexing model from the issue text.

Assessment

Tech stack
rust
Domain
backend, cloud
Issue type
Feature
Difficulty
4/5
Estimated time
3-5 days
Activity status
Stale
Clarity
Mostly clear
Newbie friendliness
30/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.