Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

QueryHandle::next_row_async never schedules a waking task #7999

Open
teh-cmc opened this issue Nov 5, 2024 · 0 comments
Open

QueryHandle::next_row_async never schedules a waking task #7999

teh-cmc opened this issue Nov 5, 2024 · 0 comments
Labels
🪳 bug Something isn't working 🔍 re_query affects re_query itself

Comments

@teh-cmc
Copy link
Member

teh-cmc commented Nov 5, 2024

See:

    pub fn next_row_async(
        &self,
    ) -> impl std::future::Future<Output = Option<Vec<Box<dyn ArrowArray>>>> {
        let res: Option<Option<_>> = self
            .engine
            .try_with(|store, cache| self._next_row(store, cache));

        std::future::poll_fn(move |_cx| match &res {
            Some(row) => std::task::Poll::Ready(row.clone()),
            None => std::task::Poll::Pending,
        })
    }

The pending case just casually goes to sleep without ever scheduling a waking task. This will invariably lead to the async tasks deadlocking.

This needs to queue some task e.g. on the rayon threadpool in order to waker.wake() when the synchronous lock becomes available once again.
No need for anything particularly fancy, QueryHandle is the least contended thing in the world.

@teh-cmc teh-cmc added 🪳 bug Something isn't working 🔍 re_query affects re_query itself labels Nov 5, 2024
@teh-cmc teh-cmc added this to the 0.20 - Maps, H.264, Undo milestone Nov 5, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
🪳 bug Something isn't working 🔍 re_query affects re_query itself
Projects
None yet
Development

No branches or pull requests

1 participant