interleave docstore fetch

[trinity-1686a]

Description

While analyzing traces, I found many situations where the same block of the docstore was decompressed multiple times. This happens when two or more documents stored in the same block are fetched at the same time. Both get a cache miss, load the block, decompress it, and write back to cache.
This PR attempt to limit this behavior by interleaving access to the docstore so two requests for the same block are more likely to happen one after the other rather than concurrently, while making sure they are not too far apart to prevent the cache from evicting the entry.

How was this PR tested?

Some unit test was added to verify interleave_sequence_generator does what it's supposed to do.
Additionally, some performance tests where done on a small index: 2681920 docs, 6 splits (a day of gh-archive), for the query org.login:kubernetes AND repo.name:kubernetes, fetching 20, 100 and 1000 documents. Result is an average of 100 runs, in microseconds

max_hits	before	after	1-after/before
20	8469	8431	0.4%
100	21665	18384	15.1%
1000	201438	164475	18.3%

the impact is negligible for a small max_hits, but becomes significant when more documents are requested.

The impact would be significantly smaller on a dataset where there is less locality on data ingestion.

[fulmicoton]

Storage are wrapped in a DebounceStorage that catch colliding in flight fetch request and debounces them.
Is this not working or is your measure too high level to catch that?
If it is really not working, can you investigate why?

The commit message is good though.

[fulmicoton]

@trinity-1686a If you need to discuss the debouncer, I think it was implemented by @PSeitz

[trinity-1686a]

Both get a cache miss, load the block, decompress it, and write back to cache.

on local disk, actually loading the block is very fast, but decompressing it takes a few hundred µs. I did not actually check if the block was getting loaded twice from storage, what I observed however is this line in tantivy being reached twice, and taking both time the few hundred µs it takes, for the same value of checkpoint.byte_range.

I'll add a few logs to make sure DebouncedStorage does work as intended, but one way or another, it is not responsible for the thing that caught my eye

[fulmicoton]

Ah understood. I had misread. So the decompression is really the problem here.
Can we debounce decompression explicitly or implicitly rather than using this obscure trick?

Also can you move the decompression from the tokio loop?

[trinity-1686a]

Can we debounce decompression explicitly or implicitly rather than using this obscure trick?

I thought about it, and had a hard time coming up with something that would work okay in both sync and async, and wouldn't be too complex for the benefits.
Additionally, as we limit the number of concurrent doc fetches to NUM_CONCURRENT_REQUESTS, debouncing would save CPU time, but not end-to-end request duration. Assuming a worst case scenario where we load 100 docs, and each group of 10 consecutive docs is stored in the same block. The 1st fetch loads from disk and decompress, the 9 after each take a spot, but mostly wait for doc1 to finish. Then 11th loads from disk and decompress, 12-20 take a spot and wait, and so one... reducing duplicate work, but replacing it by some jobs waiting on others.

Also can you move the decompression from the tokio loop?

I plan on doing that at some point, but it won't change the time a single request take, only how a request impact the rest of the system.

[trinity-1686a]

We discussed a bit with @fulmicoton . This gives good results, but is a bit too "magical". It at least had the benefit of proving a possible gain. Instead we will add some async fn fetch_docs(&self, doc_ids: &[DocId]) -> crate::Result<Vec<Document>> in tantivy.