Skip to content

Conversation

@sylwiaszunejko
Copy link

@sylwiaszunejko sylwiaszunejko commented May 6, 2025

In order for Scylla to send the tablet info, the driver must tell
the database during connection handshake that it is able to
interpret it.

Fixes: #11

@sylwiaszunejko sylwiaszunejko self-assigned this May 6, 2025
@sylwiaszunejko sylwiaszunejko requested a review from dkropachev May 6, 2025 09:44
Comment on lines 79 to 84
if (options.TabletsRoutingV1)
{
startupOptions.Add(StartupOptionsFactory.TabletsRoutingV1Option, "true");
}

Copy link
Collaborator

Choose a reason for hiding this comment

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

What will happend if you set this option all the time ?
I think it should work just fine

Copy link
Author

Choose a reason for hiding this comment

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

this option is needed so the server know that it should send the tablet info, if the driver does not support using tablets the server won't send the tablets info, tablets metadata is quite expensive to generate, so an old driver (without support for tablets) will generate huge amounts of such notifications.

Copy link
Author

Choose a reason for hiding this comment

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

@sylwiaszunejko
Copy link
Author

@dkropachev do you think this need some tests?

@sylwiaszunejko sylwiaszunejko marked this pull request as ready for review May 8, 2025 10:00
In order for Scylla to send the tablet info, the driver must tell
the database during connection handshake that it is able to
interpret it.
@dkropachev
Copy link
Collaborator

@dkropachev do you think this need some tests?

Nope, let's have a task on that, my understanding that #20 needs to be addressed first and only then we can have a test that works properly on all the versions.

@sylwiaszunejko sylwiaszunejko merged commit 4abc93f into scylladb:master May 9, 2025
2 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.

Add parsing TABLETS_ROUTING_V1 extension

2 participants