From 91a5a41313b20bebbd4e62d80e30d2012a9db4ca Mon Sep 17 00:00:00 2001 From: Sylwia Szunejko Date: Tue, 2 Jan 2024 14:54:01 +0100 Subject: [PATCH] add a way to negotiate generation of the tablet info for drivers Tablets metadata is quite expensive to generate (each data_value is an allocation), so an old driver (without support for tablets) will generate huge amounts of such notifications. This commit adds a way to negotiate generation of the notification: a new driver will ask for them, and an old driver won't get them. It uses the OPTIONS/SUPPORTED/STARTUP protocol described in native_protocol_v4.spec. Closes scylladb/scylladb#16611 --- cql3/statements/modification_statement.cc | 4 ++-- cql3/statements/select_statement.cc | 2 +- docs/dev/protocol-extensions.md | 11 +++++++++++ test/lib/cql_test_env.cc | 3 +++ transport/cql_protocol_extension.cc | 5 ++++- transport/cql_protocol_extension.hh | 6 ++++-- 6 files changed, 25 insertions(+), 6 deletions(-) diff --git a/cql3/statements/modification_statement.cc b/cql3/statements/modification_statement.cc index 58bfd19cb3..f879c66c74 100644 --- a/cql3/statements/modification_statement.cc +++ b/cql3/statements/modification_statement.cc @@ -291,7 +291,7 @@ modification_statement::do_execute(query_processor& qp, service::query_state& qs auto result = seastar::make_shared(); if (keys_size_one) { auto&& table = s->table(); - if (_may_use_token_aware_routing && table.uses_tablets()) { + if (_may_use_token_aware_routing && table.uses_tablets() && qs.get_client_state().is_protocol_extension_set(cql_transport::cql_protocol_extension::TABLETS_ROUTING_V1)) { auto erm = table.get_effective_replication_map(); auto tablet_info = erm->check_locality(token); if (tablet_info.has_value()) { @@ -358,7 +358,7 @@ modification_statement::execute_with_condition(query_processor& qp, service::que std::optional tablet_info = locator::tablet_routing_info{locator::tablet_replica_set(), std::pair()}; auto&& table = s->table(); - if (_may_use_token_aware_routing && table.uses_tablets()) { + if (_may_use_token_aware_routing && table.uses_tablets() && qs.get_client_state().is_protocol_extension_set(cql_transport::cql_protocol_extension::TABLETS_ROUTING_V1)) { auto erm = table.get_effective_replication_map(); tablet_info = erm->check_locality(token); } diff --git a/cql3/statements/select_statement.cc b/cql3/statements/select_statement.cc index 9106e0f736..b513ea62e8 100644 --- a/cql3/statements/select_statement.cc +++ b/cql3/statements/select_statement.cc @@ -409,7 +409,7 @@ select_statement::do_execute(query_processor& qp, std::optional tablet_info = {}; auto&& table = _schema->table(); - if (_may_use_token_aware_routing && table.uses_tablets()) { + if (_may_use_token_aware_routing && table.uses_tablets() && state.get_client_state().is_protocol_extension_set(cql_transport::cql_protocol_extension::TABLETS_ROUTING_V1)) { if (key_ranges.size() == 1 && query::is_single_partition(key_ranges.front())) { token = key_ranges[0].start()->value().as_decorated_key().token(); diff --git a/docs/dev/protocol-extensions.md b/docs/dev/protocol-extensions.md index 64a19cb4a3..3a0807c2f4 100644 --- a/docs/dev/protocol-extensions.md +++ b/docs/dev/protocol-extensions.md @@ -213,3 +213,14 @@ When the driver receives information about the tablet, it has to check if any of the previously received tablets has an overlapping token range. The group of tablets that meets this criterion has to be deleted, and the new tablet should replace them. + +## Negotiate sending tablets info to the drivers + +This extension allows the driver to inform the database that it is aware of +tablets and is able to intepret the tablet information sent in `custom_payload`. + +Having a designated flag gives the ability to skip tablet metadata generation +(which is quite expensive) if driver is not aware of tablets. + +The feature is identified by the `TABLETS_ROUTING_V1` key, which is meant to be sent +in the SUPPORTED message. diff --git a/test/lib/cql_test_env.cc b/test/lib/cql_test_env.cc index 84e85da700..aebeff6d55 100644 --- a/test/lib/cql_test_env.cc +++ b/test/lib/cql_test_env.cc @@ -190,6 +190,9 @@ private: auto make_query_state() { if (_db.local().has_keyspace(ks_name)) { _core_local.local().client_state.set_keyspace(_db.local(), ks_name); + cql_transport::cql_protocol_extension_enum_set cql_proto_exts; + cql_proto_exts.set(cql_transport::cql_protocol_extension::TABLETS_ROUTING_V1); + _core_local.local().client_state.set_protocol_extensions(std::move(cql_proto_exts)); } return ::make_shared(_core_local.local().client_state, empty_service_permit()); } diff --git a/transport/cql_protocol_extension.cc b/transport/cql_protocol_extension.cc index 541fce8014..9f7a2c6c0b 100644 --- a/transport/cql_protocol_extension.cc +++ b/transport/cql_protocol_extension.cc @@ -17,7 +17,8 @@ namespace cql_transport { static const std::map EXTENSION_NAMES = { {cql_protocol_extension::LWT_ADD_METADATA_MARK, "SCYLLA_LWT_ADD_METADATA_MARK"}, - {cql_protocol_extension::RATE_LIMIT_ERROR, "SCYLLA_RATE_LIMIT_ERROR"} + {cql_protocol_extension::RATE_LIMIT_ERROR, "SCYLLA_RATE_LIMIT_ERROR"}, + {cql_protocol_extension::TABLETS_ROUTING_V1, "TABLETS_ROUTING_V1"} }; cql_protocol_extension_enum_set supported_cql_protocol_extensions() { @@ -34,6 +35,8 @@ std::vector additional_options_for_proto_ext(cql_protocol_exte return {format("LWT_OPTIMIZATION_META_BIT_MASK={:d}", cql3::prepared_metadata::LWT_FLAG_MASK)}; case cql_protocol_extension::RATE_LIMIT_ERROR: return {format("ERROR_CODE={}", exceptions::exception_code::RATE_LIMIT_ERROR)}; + case cql_protocol_extension::TABLETS_ROUTING_V1: + return {"TABLETS_ROUTING_V1"}; default: return {}; } diff --git a/transport/cql_protocol_extension.hh b/transport/cql_protocol_extension.hh index 0b917387ae..4e4a70ebd9 100644 --- a/transport/cql_protocol_extension.hh +++ b/transport/cql_protocol_extension.hh @@ -29,12 +29,14 @@ namespace cql_transport { */ enum class cql_protocol_extension { LWT_ADD_METADATA_MARK, - RATE_LIMIT_ERROR + RATE_LIMIT_ERROR, + TABLETS_ROUTING_V1 }; using cql_protocol_extension_enum = super_enum; + cql_protocol_extension::RATE_LIMIT_ERROR, + cql_protocol_extension::TABLETS_ROUTING_V1>; using cql_protocol_extension_enum_set = enum_set;