Prevents single split searches from different leaf_search from interleaving

[fulmicoton]

Before this PR, we just used a semaphore to acquire a permit and start a new tokio task to run our single split search.

In pseudo code, a leaf_search would look like:

for split in splits {
    let permit = semaphore.acquire().await;
    tokio::spawn(async move {
       single_split_search(split);
       drop(permit)
    })
}

The problem with this is that it allows interleaving split search from one search request with another one.

This interleaving strongly impacts search latency. For instance, one can imagine two queries A and B with 3 splits each. Executing as follows
| A | A | A | B | B | B |
offers a much short latency for A than
| A | B | B | A | B | A |

This PR also adds two metrics to monitor the number of queue single split search.

[github-actions]

On SSD:

Average search latency is 0.99x that of the reference (lower is better).
Ref run id: 3940, ref commit: 5e5b360
Link

On GCS:

Average search latency is 1.02x that of the reference (lower is better).
Ref run id: 3941, ref commit: 5e5b360
Link

[trinity-1686a]

I think the changes make sense, but some struct/params names make the code more confusing than it could be

[trinity-1686a]

i think inner is a very confusing name, the InnerSearchPermits is already "the inner thing", so an Arc<Mutex<_>> should outer, or a handle

[fulmicoton]

I don't quite get the confusion/comment.
The "outter" is SearchPermits.

I have not found a common practise on how to name the object of this pattern in Rust (in C++ pimpl is common, even if this is not the common usage of the pimpl pattern).

What would be a better name? would inner_arc help a bit?

[trinity-1686a]

this is a misnomer, it sounds like this is a akin to a tokio SemaphorePermit with num_permits > 1, but this is actually a semaphore, or a PermitProvider, or something of the like

[fulmicoton]

Yeah you are right.

[trinity-1686a]

I don't believe there is need for Weak here: InnerSearchPermits doesn't hold any SearchPermit (a oneshot sender doesn't hold the thing to transmit, the oneshot receiver do)

i find this type, Option<Weak<Mutex<InnerSearchPermits>>> rather confusing, in part because SearchPermits isn't the right name imo, in part because the Option<> very much looks like a way to store a bool alongside the rest of the structure to reduce the size of the struct by one usize. I'm not against using an Option that way, but if that's the reason, it ought to be written down, and if it isn't the goal, then i'm missing what the goal of this Option is, rather than a disable_drop: bool

[fulmicoton]

I prefer the Option. Setting it to None seems easier to "proofread". I agree that it makes the code less readable however, as the intent is way less clearer than with the boolean solution.

I'll add some comments.

[trinity-1686a]

you ended up changing it to a bool. If you think an Option is better, you can put it back, just add a 1-2 line comment on the purpose of None

[fulmicoton]

Yeah, I thought after packing everything as drop_without_recycling_permit it was fine like that.