Skip to content

Fix use the update runtime for update operations#7569

Merged
agourlay merged 2 commits intodevfrom
fix-update-runtime-for-update-ops
Nov 20, 2025
Merged

Fix use the update runtime for update operations#7569
agourlay merged 2 commits intodevfrom
fix-update-runtime-for-update-ops

Conversation

@agourlay
Copy link
Member

@agourlay agourlay commented Nov 20, 2025

The following update operations are running on the calling Tokio runtime:

  • update_all_local
  • update_from_peer
  • update_from_client

For the REST API that would be the actix-rt and for gRPC that would be the general runtime.

We actually have a dedicated runtime for update operations that we should be using instead.

https://github.com/qdrant/qdrant/blob/master/src/common/helpers.rs#L26-L45

This PR changes those 3 call sites to use it which should result in better stability in the other runtimes.

Sorry for the reformating noise.

Future work

There are more tokio::task::spawn usages to audit to check their correctness.

@agourlay agourlay marked this pull request as ready for review November 20, 2025 10:37
Copy link
Member

@timvisee timvisee left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Lets take extra care with tokio::task::spawn in reviews. And maybe even ban (lint) it in the future.

FAILED tests/consensus_tests/test_collection_recovery_limits.py::test_collection_recovery_reach_limit - Exception: HTTP request to http://127.0.0.1:33891/collections/test_collection/points?wait=true&ordering=weak failed with status code 500 after 5.20299s with response body:
{'status': {'error': 'Service internal error: task 240 panicked with message "A Tokio 1.x context was found, but IO is disabled. Call `enable_io` on the runtime builder to enable IO."'}, 'time': 3.28820663, 'usage': {'hardware': {'cpu': 0, 'payload_io_read': 0, 'payload_io_write': 70315, 'payload_index_io_read': 0, 'payload_index_io_write': 0, 'vector_io_read': 0, 'vector_io_write': 1649102}, 'inference': None}}
@agourlay
Copy link
Member Author

@timvisee I had to enable the IO component on the update runtime to pass the consensus tests.
6470b8e

@qdrant qdrant deleted a comment from coderabbitai bot Nov 20, 2025
@agourlay agourlay merged commit 50cf48b into dev Nov 20, 2025
16 checks passed
@agourlay agourlay deleted the fix-update-runtime-for-update-ops branch November 20, 2025 11:33
Comment on lines 32 to +33
.enable_time()
.enable_io()
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Let's just do enable_all() on all three runtimes. Wdyt?

timvisee pushed a commit that referenced this pull request Nov 25, 2025
* Fix use the update runtime for update operations

* Enable IO on update runtime

FAILED tests/consensus_tests/test_collection_recovery_limits.py::test_collection_recovery_reach_limit - Exception: HTTP request to http://127.0.0.1:33891/collections/test_collection/points?wait=true&ordering=weak failed with status code 500 after 5.20299s with response body:
{'status': {'error': 'Service internal error: task 240 panicked with message "A Tokio 1.x context was found, but IO is disabled. Call `enable_io` on the runtime builder to enable IO."'}, 'time': 3.28820663, 'usage': {'hardware': {'cpu': 0, 'payload_io_read': 0, 'payload_io_write': 70315, 'payload_index_io_read': 0, 'payload_index_io_write': 0, 'vector_io_read': 0, 'vector_io_write': 1649102}, 'inference': None}}
@timvisee timvisee mentioned this pull request Nov 25, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants