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

Move schema agreement to asyncio #248

Closed
wants to merge 1 commit into from

Conversation

Lorak-mmk
Copy link

@Lorak-mmk Lorak-mmk commented Jul 26, 2023

Fixes #168. See the issue description for more detailed description of the bug.

This commit introduces asyncio Event Loop to Cluster object. This loop is now responsible for executing most of wait_for_schema_agreement code, the only part delegated to executor is the one that actually sends request to a node. This way parts delegated to executor don't have to wait on any locks, preventing deadlocks.

In the future we can move more code to Cluster.loop and eventually retire Cluster.scheduler.

Fixes #168
Fixes #225

@Lorak-mmk Lorak-mmk force-pushed the fix-168-v2 branch 4 times, most recently from 53097b1 to c2c2862 Compare July 28, 2023 16:06
Fixes scylladb#168. See the issue description for more detailed description of the bug.

This commit introduces asyncio Event Loop to Cluster object.
This loop is now responsible for executing most of wait_for_schema_agreement code,
the only part delegated to executor is the one that actually sends request to a node.
This way tasks executing on executor doesn't have to wait on any locks, preventing deadlocks.

In the future we can move more code to Cluster.loop and eventually retire Cluster.scheduler.
@fruch
Copy link

fruch commented Aug 29, 2023

Looks interesting, wondering if it would play nicely when more loops are used, like in scylla unittests or when asyncio is used as backend (no sure it's working at all)

@Lorak-mmk
Copy link
Author

I don't think more loops should be a problem. We'll see for sure when I get this to work.
Regarding asyncio backend - it doesn't work, and I'd like to make it work because asyncore (which is the default backend when gevent / eventlet are not used and libev is not present) is getting removed in Python 3.12.

@fruch
Copy link

fruch commented Sep 5, 2023

@Lorak-mmk now that we are running more of dtest as gating, we started seeing this issue also in dtest

anything I can do to help with this one ?

@tchaikov
Copy link

tchaikov commented Jan 8, 2024

scylladb/scylladb#16676 might be relevant.

@Lorak-mmk
Copy link
Author

This PR was obsoleted by #256 , so closing.

@Lorak-mmk Lorak-mmk closed this Jan 11, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
3 participants