Skip to content

SearchQueue: Permit::drop(self) sends two release signals (explicit + implicit Drop), causing double capacity return and potential parallelism violation #6578

Description

@gregormelhorn

Description

Permit::drop(self) sends one release signal, then Rust calls
Drop for Permit which sends a second signal. Every call to
permit.drop().await produces two signals. The scheduler grants
one permit per signal, so one logical release grants two permits.

How to reproduce

A deterministic example:

  1. Set parallelism = 2. Get two permits (all slots used).
  2. Put two waiters in the queue.
  3. Call permit.drop().await on one held permit.

The scheduler processes two signals:

  • Signal 1: searches_running goes from 2 to 1. Waiter 1 gets a permit.
  • Signal 2: searches_running goes from 1 to 0. Waiter 2 gets a permit.

After both signals, three searches are running. The configured limit
of 2 is exceeded. The admission gate at L154 sees 0 and lets another
request in.

Root cause

Permit::drop(self) at L52-55 sends the first signal and uses
up self:

pub async fn drop(self) {
    let _ = self.sender.send(()).await;
}

After this method returns, self goes out of scope. Rust's Drop
trait fires. Drop for Permit at L58-67 clones the sender and
spawns a task that sends the second signal:

impl Drop for Permit {
    fn drop(&mut self) {
        let sender = self.sender.clone();
        std::mem::drop(tokio::spawn(async move { sender.send(()).await }));
    }
}

The release handler at L131-138 runs once per signal: it decrements
searches_running and grants a permit to the next waiter.

Impact

The configured parallelism limit can be exceeded. One search finishing
grants permits to two waiters. The saturating_sub at L146 keeps
searches_running from going below zero, but does not stop the extra
grants.

Every route call site uses permit.drop().await:
search.rs:600,666,900,967, multi_search.rs:195,307,379,
facet_search.rs:331. The module doc recommends it.

Expected behavior

A permit release must return exactly one unit of capacity. Options:

  • The Drop impl checks a flag and skips the second signal when
    explicit drop already ran.
  • The explicit drop(self) method prevents the implicit Drop
    from sending a signal.
  • The implicit Drop is the only release path. The explicit method
    becomes a no-op wrapper or is removed.

Environment

  • Commit: fff2ef5a42658b16a937d922aabc3fb7f89f2018
  • File: crates/meilisearch/src/search_queue.rs

Activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions