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

Connection to a ScyllaDB cluster is delayed as the driver tries to qu… #252

Merged
merged 1 commit into from
Sep 26, 2023

Conversation

mykaul
Copy link

@mykaul mykaul commented Aug 17, 2023

…ery system.peers_v2 table

Fixes: #245

Signed-off-by: Yaniv Kaul yaniv.kaul@scylladb.com

@Lorak-mmk
Copy link

I think we can merge this, as far as I can see by looking at the code this shouldn't break anything (but in the future I'd like to setup Cassandra CI to be sure).
Could you add a TODO comment to switch it back when (if?) Scylla adds peers_v2 table?

@mykaul
Copy link
Author

mykaul commented Aug 20, 2023

I think we can merge this, as far as I can see by looking at the code this shouldn't break anything (but in the future I'd like to setup Cassandra CI to be sure).
Could you add a TODO comment to switch it back when (if?) Scylla adds peers_v2 table?

Do we have such plans? I see it was added to Cassandra as part of https://issues.apache.org/jira/browse/CASSANDRA-7544 - what are we missing that we'd like to add to ScyllaDB?

@Lorak-mmk
Copy link

I have no idea if we need it or have any plans to implement it.
But now I look at the code that added peers_v2 support in this driver: datastax@40fe726
and it looks like it is a feature used by C*.
I'd like to avoid breaking C* compatibility, so what do you think about only disabling PEERS_V2 when connecting to Scylla?
The patch would probably look like this (I didn't test it, it's just the idea): Lorak-mmk@ed97974

@fruch
Copy link

fruch commented Aug 29, 2023

I have no idea if we need it or have any plans to implement it. But now I look at the code that added peers_v2 support in this driver: datastax@40fe726 and it looks like it is a feature used by C*. I'd like to avoid breaking C* compatibility, so what do you think about only disabling PEERS_V2 when connecting to Scylla? The patch would probably look like this (I didn't test it, it's just the idea): Lorak-mmk@ed97974

I agree it's a better direction, assuming it works.

@mykaul mykaul force-pushed the issue_245 branch 2 times, most recently from 7c0af68 to 0955b0a Compare September 21, 2023 09:55
…ery system.peers_v2 table

The logic is now that if there is sharding information available, it's a Scylla cluster and then do NOT
try to use that table.

Fixes: scylladb#245

Signed-off-by: Yaniv Kaul <yaniv.kaul@scylladb.com>
@mykaul
Copy link
Author

mykaul commented Sep 21, 2023

I have no idea if we need it or have any plans to implement it. But now I look at the code that added peers_v2 support in this driver: datastax@40fe726 and it looks like it is a feature used by C*. I'd like to avoid breaking C* compatibility, so what do you think about only disabling PEERS_V2 when connecting to Scylla? The patch would probably look like this (I didn't test it, it's just the idea): Lorak-mmk@ed97974

Done.

@Lorak-mmk Lorak-mmk merged commit d12d2c1 into scylladb:master Sep 26, 2023
13 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Connection to a ScyllaDB cluster is delayed as the driver tries to query system.peers_v2 table
3 participants