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

bug: drains output channel when batching results #763

Merged
merged 7 commits into from
Sep 27, 2023

Conversation

jordanrfrazier
Copy link
Collaborator

@jordanrfrazier jordanrfrazier commented Sep 20, 2023

Drains the output channel. Previously was causing queries to block on send.

Puts the tokio runtime into an Arc to share it -- the handle() was not working like I wanted since it was being dropped (a blocking method) from an asynchronous context. Unsure why, but swapping to use Arc, as is also recommended, works fine.

https://docs.rs/tokio/latest/tokio/runtime/struct.Runtime.html#sharing

Closes #775

@cla-bot cla-bot bot added the cla-signed Set when all authors of a PR have signed our CLA label Sep 20, 2023
@github-actions github-actions bot added bug Something isn't working sparrow labels Sep 20, 2023
@jordanrfrazier
Copy link
Collaborator Author

Current status with runtime in session and using handles: the runtime is closing unexpectedly when a query is being made, resulting in an error. Need to investigate deeper into why this is happening.

crates/sparrow-session/src/execution.rs Outdated Show resolved Hide resolved
crates/sparrow-session/src/execution.rs Outdated Show resolved Hide resolved
crates/sparrow-session/src/execution.rs Outdated Show resolved Hide resolved
crates/sparrow-session/src/execution.rs Outdated Show resolved Hide resolved
crates/sparrow-session/src/execution.rs Outdated Show resolved Hide resolved
crates/sparrow-session/src/execution.rs Outdated Show resolved Hide resolved
crates/sparrow-session/src/execution.rs Outdated Show resolved Hide resolved
jordanrfrazier and others added 5 commits September 27, 2023 12:25
1. Creating (and dropping) a tokio runtime in `session` lead to problems
2. The logic in the `select` led to problems (progress always reported
   `None` once the stream was done, and being biased chose that). This
   is fixed by preferring `futures::select!` and `select_next_some`.
@jordanrfrazier jordanrfrazier added this pull request to the merge queue Sep 27, 2023
@github-merge-queue github-merge-queue bot removed this pull request from the merge queue due to failed status checks Sep 27, 2023
@jordanrfrazier jordanrfrazier added this pull request to the merge queue Sep 27, 2023
@jordanrfrazier jordanrfrazier removed this pull request from the merge queue due to a manual request Sep 27, 2023
@jordanrfrazier jordanrfrazier added this pull request to the merge queue Sep 27, 2023
Merged via the queue into main with commit e68a331 Sep 27, 2023
33 checks passed
@jordanrfrazier jordanrfrazier deleted the bug-testing-output-channel-blocks branch September 27, 2023 22:24
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working cla-signed Set when all authors of a PR have signed our CLA sparrow
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Preview blocks on excess batches (not rows)
2 participants