Skip to content

Commit ec08976

Browse files
transport: implement SCYLLA_USE_METADATA_ID support
Metadata id was introduced in CQLv5 to make metadata of prepared statement consistent between driver and database. This commit introduces a protocol extension that allows to use the same mechanism in CQLv4. This change: - Introduce SCYLLA_USE_METADATA_ID protocol extension for CQLv4 - Introduce METADATA_CHANGED flag in RESULT. The flag cames directly from CQLv5 binary protocol. In CQLv4, the bit was never used, so we assume it is safe to reuse it. - Implement handling of metadata_id and METADATA_CHANGED in RESULT rows - Implement returning metadata_id in RESULT prepared - Implement reading metadata_id from EXECUTE - Added description of SCYLLA_USE_METADATA_ID in documentation Metadata_id is wrapped in cql_metadata_id_wrapper because we need to distinguish the following situations: - Metadata_id is not supported by the protocol (e.g. CQLv4 without the extension is used) - Metadata_id is supported by the protocol but not set - e.g. PREPARE query is being handled: it doesn't contain metadata_id in the request but the reply (RESULT prepared) must contain metadata_id - Metadata_id is supported by the protocol and set, any number of bytes >= 0 is allowed, according to the CQLv5 protocol specification Fixes scylladb#20860
1 parent c60dcb3 commit ec08976

File tree

10 files changed

+155
-19
lines changed

10 files changed

+155
-19
lines changed

cql3/result_set.hh

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -27,12 +27,14 @@ public:
2727
GLOBAL_TABLES_SPEC = 0,
2828
HAS_MORE_PAGES = 1,
2929
NO_METADATA = 2,
30+
METADATA_CHANGED = 3,
3031
};
3132

3233
using flag_enum = super_enum<flag,
3334
flag::GLOBAL_TABLES_SPEC,
3435
flag::HAS_MORE_PAGES,
35-
flag::NO_METADATA>;
36+
flag::NO_METADATA,
37+
flag::METADATA_CHANGED>;
3638

3739
using flag_enum_set = enum_set<flag_enum>;
3840

cql3/statements/prepared_statement.hh

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -50,7 +50,10 @@ public:
5050
const std::vector<seastar::lw_shared_ptr<column_specification>> bound_names;
5151
const std::vector<uint16_t> partition_key_bind_indices;
5252
std::vector<sstring> warnings;
53+
private:
54+
mutable std::optional<cql_metadata_id_type> _cached_metadata_id;
5355

56+
public:
5457
prepared_statement(audit::audit_info_ptr&& audit_info, seastar::shared_ptr<cql_statement> statement_, std::vector<seastar::lw_shared_ptr<column_specification>> bound_names_,
5558
std::vector<uint16_t> partition_key_bind_indices, std::vector<sstring> warnings = {});
5659

@@ -64,6 +67,8 @@ public:
6467
checked_weak_ptr checked_weak_from_this() const {
6568
return checked_weak_ptr(this->weak_from_this());
6669
}
70+
71+
cql_metadata_id_type calculate_or_get_metadata_id() const;
6772
};
6873

6974
}

cql3/statements/raw/parsed_statement.cc

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,7 @@
1414
#include "cql3/column_specification.hh"
1515

1616
#include "cql3/cql_statement.hh"
17+
#include "cql3/result_set.hh"
1718

1819
namespace cql3 {
1920

@@ -47,6 +48,7 @@ prepared_statement::prepared_statement(
4748
, bound_names(std::move(bound_names_))
4849
, partition_key_bind_indices(std::move(partition_key_bind_indices))
4950
, warnings(std::move(warnings))
51+
, _cached_metadata_id()
5052
{
5153
statement->set_audit_info(std::move(audit_info));
5254
}
@@ -66,6 +68,13 @@ prepared_statement::prepared_statement(audit::audit_info_ptr&& audit_info, ::sha
6668
: prepared_statement(std::move(audit_info), statement_, std::vector<lw_shared_ptr<column_specification>>(), std::vector<uint16_t>(), std::move(warnings))
6769
{ }
6870

71+
cql_metadata_id_type prepared_statement::calculate_or_get_metadata_id() const {
72+
if (!_cached_metadata_id.has_value()) {
73+
_cached_metadata_id = statement->get_result_metadata()->calculate_metadata_id();
74+
}
75+
return _cached_metadata_id.value();
76+
}
77+
6978
}
7079

7180
}

docs/dev/protocol-extensions.md

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -224,3 +224,15 @@ Having a designated flag gives the ability to skip tablet metadata generation
224224

225225
The feature is identified by the `TABLETS_ROUTING_V1` key, which is meant to be sent
226226
in the SUPPORTED message.
227+
228+
## Negotiate sending metadata id
229+
230+
This extension allows the driver to inform the database that it is aware of
231+
`metadata id` and is able to interpret the metadata id information.
232+
233+
Metadata id was originally introduced in CQLv5 to make metadata of prepared statement
234+
consistent between driver and database. This extension allows to use
235+
the same mechanism for other protocol versions, such as CQLv4.
236+
237+
The feature is identified by the `SCYLLA_USE_METADATA_ID` key, which is meant to be sent
238+
in the SUPPORTED message.

transport/cql_protocol_extension.cc

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -18,7 +18,8 @@ namespace cql_transport {
1818
static const std::map<cql_protocol_extension, seastar::sstring> EXTENSION_NAMES = {
1919
{cql_protocol_extension::LWT_ADD_METADATA_MARK, "SCYLLA_LWT_ADD_METADATA_MARK"},
2020
{cql_protocol_extension::RATE_LIMIT_ERROR, "SCYLLA_RATE_LIMIT_ERROR"},
21-
{cql_protocol_extension::TABLETS_ROUTING_V1, "TABLETS_ROUTING_V1"}
21+
{cql_protocol_extension::TABLETS_ROUTING_V1, "TABLETS_ROUTING_V1"},
22+
{cql_protocol_extension::USE_METADATA_ID, "SCYLLA_USE_METADATA_ID"}
2223
};
2324

2425
cql_protocol_extension_enum_set supported_cql_protocol_extensions() {

transport/cql_protocol_extension.hh

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -28,13 +28,15 @@ namespace cql_transport {
2828
enum class cql_protocol_extension {
2929
LWT_ADD_METADATA_MARK,
3030
RATE_LIMIT_ERROR,
31-
TABLETS_ROUTING_V1
31+
TABLETS_ROUTING_V1,
32+
USE_METADATA_ID
3233
};
3334

3435
using cql_protocol_extension_enum = super_enum<cql_protocol_extension,
3536
cql_protocol_extension::LWT_ADD_METADATA_MARK,
3637
cql_protocol_extension::RATE_LIMIT_ERROR,
37-
cql_protocol_extension::TABLETS_ROUTING_V1>;
38+
cql_protocol_extension::TABLETS_ROUTING_V1,
39+
cql_protocol_extension::USE_METADATA_ID>;
3840

3941
using cql_protocol_extension_enum_set = enum_set<cql_protocol_extension_enum>;
4042

transport/messages/result_message.hh

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -48,6 +48,10 @@ public:
4848
return _result_metadata;
4949
}
5050

51+
cql3::cql_metadata_id_type calculate_or_get_metadata_id() const {
52+
return _prepared->calculate_or_get_metadata_id();
53+
}
54+
5155
class cql;
5256
private:
5357
static ::shared_ptr<const cql3::metadata> extract_result_metadata(::shared_ptr<cql3::cql_statement> statement);

transport/response.hh

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -76,7 +76,7 @@ public:
7676
void write_string_bytes_map(const std::unordered_map<sstring, bytes>& map);
7777
void write_value(bytes_opt value);
7878
void write_value(std::optional<managed_bytes_view> value);
79-
void write(const cql3::metadata& m, bool skip = false);
79+
void write(const cql3::metadata& m, const cql_metadata_id_wrapper& request_metadata_id, bool no_metadata = false);
8080
void write(const cql3::prepared_metadata& m, uint8_t version);
8181

8282
// Make a non-owning scattered_message of the response. Remains valid as long

transport/server.cc

Lines changed: 76 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -174,6 +174,12 @@ sstring to_string(const event::schema_change::target_type t) {
174174
SCYLLA_ASSERT(false && "unreachable");
175175
}
176176

177+
bool is_metadata_id_supported(const service::client_state& client_state) {
178+
// TODO: metadata_id is mandatory in CQLv5, so extend the check below
179+
// when CQLv5 support is implemented
180+
return client_state.is_protocol_extension_set(cql_transport::cql_protocol_extension::USE_METADATA_ID);
181+
}
182+
177183
event::event_type parse_event_type(const sstring& value)
178184
{
179185
if (value == "TOPOLOGY_CHANGE") {
@@ -982,7 +988,7 @@ future<std::unique_ptr<cql_server::response>> cql_server::connection::process_op
982988

983989
std::unique_ptr<cql_server::response>
984990
make_result(int16_t stream, messages::result_message& msg, const tracing::trace_state_ptr& tr_state,
985-
cql_protocol_version_type version, bool skip_metadata = false);
991+
cql_protocol_version_type version, cql_metadata_id_wrapper&& metadata_id, bool skip_metadata = false);
986992

987993
template <typename Process>
988994
requires std::is_invocable_r_v<future<cql_server::process_fn_return_type>,
@@ -1078,7 +1084,9 @@ process_query_internal(service::client_state& client_state, distributed<cql3::qu
10781084
return cql_server::process_fn_return_type(convert_error_message_to_coordinator_result(msg.get()));
10791085
} else {
10801086
tracing::trace(q_state->query_state.get_trace_state(), "Done processing - preparing a result");
1081-
return cql_server::process_fn_return_type(make_foreign(make_result(stream, *msg, q_state->query_state.get_trace_state(), version, skip_metadata)));
1087+
cql_metadata_id_wrapper metadata_id{is_metadata_id_supported(q_state->query_state.get_client_state())};
1088+
1089+
return cql_server::process_fn_return_type(make_foreign(make_result(stream, *msg, q_state->query_state.get_trace_state(), version, std::move(metadata_id), skip_metadata)));
10821090
}
10831091
});
10841092
}
@@ -1101,11 +1109,14 @@ future<std::unique_ptr<cql_server::response>> cql_server::connection::process_pr
11011109
return qp.prepare(std::move(query), client_state, dialect).discard_result();
11021110
}).then([this, query, stream, &client_state, trace_state, dialect] () mutable {
11031111
tracing::trace(trace_state, "Done preparing on remote shards");
1104-
return _server._query_processor.local().prepare(std::move(query), client_state, dialect).then([this, stream, trace_state] (auto msg) {
1112+
return _server._query_processor.local().prepare(std::move(query), client_state, dialect).then([this, stream, &client_state, trace_state] (auto msg) {
11051113
tracing::trace(trace_state, "Done preparing on a local shard - preparing a result. ID is [{}]", seastar::value_of([&msg] {
11061114
return messages::result_message::prepared::cql::get_id(msg);
11071115
}));
1108-
return make_result(stream, *msg, trace_state, _version);
1116+
cql_metadata_id_wrapper metadata_id = is_metadata_id_supported(client_state)
1117+
? cql_metadata_id_wrapper(msg->calculate_or_get_metadata_id())
1118+
: cql_metadata_id_wrapper(false);
1119+
return make_result(stream, *msg, trace_state, _version, std::move(metadata_id));
11091120
});
11101121
});
11111122
}
@@ -1131,6 +1142,10 @@ process_execute_internal(service::client_state& client_state, distributed<cql3::
11311142
throw exceptions::prepared_query_not_found_exception(id);
11321143
}
11331144

1145+
cql_metadata_id_wrapper metadata_id = is_metadata_id_supported(client_state)
1146+
? cql_metadata_id_wrapper(cql3::cql_metadata_id_type(in.read_short_bytes()), prepared->calculate_or_get_metadata_id())
1147+
: cql_metadata_id_wrapper(false);
1148+
11341149
auto q_state = std::make_unique<cql_query_state>(client_state, trace_state, std::move(permit));
11351150
auto& query_state = q_state->query_state;
11361151
q_state->options = in.read_options(version, qp.local().get_cql_config());
@@ -1169,14 +1184,14 @@ process_execute_internal(service::client_state& client_state, distributed<cql3::
11691184

11701185
tracing::trace(trace_state, "Processing a statement");
11711186
return qp.local().execute_prepared_without_checking_exception_message(query_state, std::move(stmt), options, std::move(prepared), std::move(cache_key), needs_authorization)
1172-
.then([trace_state = query_state.get_trace_state(), skip_metadata, q_state = std::move(q_state), stream, version] (auto msg) {
1187+
.then([trace_state = query_state.get_trace_state(), skip_metadata, q_state = std::move(q_state), stream, version, metadata_id = std::move(metadata_id)] (auto msg) mutable {
11731188
if (msg->move_to_shard()) {
11741189
return cql_server::process_fn_return_type(make_foreign(dynamic_pointer_cast<messages::result_message::bounce_to_shard>(msg)));
11751190
} else if (msg->is_exception()) {
11761191
return cql_server::process_fn_return_type(convert_error_message_to_coordinator_result(msg.get()));
11771192
} else {
11781193
tracing::trace(q_state->query_state.get_trace_state(), "Done processing - preparing a result");
1179-
return cql_server::process_fn_return_type(make_foreign(make_result(stream, *msg, q_state->query_state.get_trace_state(), version, skip_metadata)));
1194+
return cql_server::process_fn_return_type(make_foreign(make_result(stream, *msg, q_state->query_state.get_trace_state(), version, std::move(metadata_id), skip_metadata)));
11801195
}
11811196
});
11821197
}
@@ -1296,7 +1311,9 @@ process_batch_internal(service::client_state& client_state, distributed<cql3::qu
12961311
return cql_server::process_fn_return_type(convert_error_message_to_coordinator_result(msg.get()));
12971312
} else {
12981313
tracing::trace(q_state->query_state.get_trace_state(), "Done processing - preparing a result");
1299-
return cql_server::process_fn_return_type(make_foreign(make_result(stream, *msg, trace_state, version)));
1314+
cql_metadata_id_wrapper metadata_id{is_metadata_id_supported(q_state->query_state.get_client_state())};
1315+
1316+
return cql_server::process_fn_return_type(make_foreign(make_result(stream, *msg, trace_state, version, std::move(metadata_id))));
13001317
}
13011318
});
13021319
}
@@ -1510,11 +1527,13 @@ class cql_server::fmt_visitor : public messages::result_message::visitor_base {
15101527
uint8_t _version;
15111528
cql_server::response& _response;
15121529
bool _skip_metadata;
1530+
cql_metadata_id_wrapper _metadata_id;
15131531
public:
1514-
fmt_visitor(uint8_t version, cql_server::response& response, bool skip_metadata)
1532+
fmt_visitor(uint8_t version, cql_server::response& response, bool skip_metadata, cql_metadata_id_wrapper&& metadata_id)
15151533
: _version{version}
15161534
, _response{response}
15171535
, _skip_metadata{skip_metadata}
1536+
, _metadata_id(std::move(metadata_id))
15181537
{ }
15191538

15201539
virtual void visit(const messages::result_message::void_message&) override {
@@ -1529,8 +1548,11 @@ class cql_server::fmt_visitor : public messages::result_message::visitor_base {
15291548
virtual void visit(const messages::result_message::prepared::cql& m) override {
15301549
_response.write_int(0x0004);
15311550
_response.write_short_bytes(m.get_id());
1551+
if (_metadata_id.is_supported_by_protocol()) {
1552+
_response.write_short_bytes(_metadata_id.get_response_metadata_id()._metadata_id);
1553+
}
15321554
_response.write(m.metadata(), _version);
1533-
_response.write(*m.result_metadata());
1555+
_response.write(*m.result_metadata(), _metadata_id);
15341556
}
15351557

15361558
virtual void visit(const messages::result_message::schema_change& m) override {
@@ -1550,7 +1572,7 @@ class cql_server::fmt_visitor : public messages::result_message::visitor_base {
15501572
virtual void visit(const messages::result_message::rows& m) override {
15511573
_response.write_int(0x0002);
15521574
auto& rs = m.rs();
1553-
_response.write(rs.get_metadata(), _skip_metadata);
1575+
_response.write(rs.get_metadata(), _metadata_id, _skip_metadata);
15541576
auto row_count_plhldr = _response.write_int_placeholder();
15551577

15561578
class visitor {
@@ -1578,7 +1600,7 @@ class cql_server::fmt_visitor : public messages::result_message::visitor_base {
15781600

15791601
std::unique_ptr<cql_server::response>
15801602
make_result(int16_t stream, messages::result_message& msg, const tracing::trace_state_ptr& tr_state,
1581-
cql_protocol_version_type version, bool skip_metadata) {
1603+
cql_protocol_version_type version, cql_metadata_id_wrapper&& metadata_id, bool skip_metadata) {
15821604
auto response = std::make_unique<cql_server::response>(stream, cql_binary_opcode::RESULT, tr_state);
15831605
if (__builtin_expect(!msg.warnings().empty() && version > 3, false)) {
15841606
response->set_frame_flag(cql_frame_flags::warning);
@@ -1588,7 +1610,7 @@ make_result(int16_t stream, messages::result_message& msg, const tracing::trace_
15881610
response->set_frame_flag(cql_frame_flags::custom_payload);
15891611
response->write_string_bytes_map(msg.custom_payload().value());
15901612
}
1591-
cql_server::fmt_visitor fmt{version, *response, skip_metadata};
1613+
cql_server::fmt_visitor fmt{version, *response, skip_metadata, std::move(metadata_id)};
15921614
msg.accept(fmt);
15931615
return response;
15941616
}
@@ -1996,7 +2018,7 @@ thread_local const type_codec::type_id_to_type_type type_codec::type_id_to_type
19962018
{ inet_addr_type, type_id::INET },
19972019
};
19982020

1999-
void cql_server::response::write(const cql3::metadata& m, bool no_metadata) {
2021+
void cql_server::response::write(const cql3::metadata& m, const cql_metadata_id_wrapper& metadata_id, bool no_metadata) {
20002022
auto flags = m.flags();
20012023
bool global_tables_spec = m.flags().contains<cql3::metadata::flag::GLOBAL_TABLES_SPEC>();
20022024
bool has_more_pages = m.flags().contains<cql3::metadata::flag::HAS_MORE_PAGES>();
@@ -2005,6 +2027,15 @@ void cql_server::response::write(const cql3::metadata& m, bool no_metadata) {
20052027
flags.set<cql3::metadata::flag::NO_METADATA>();
20062028
}
20072029

2030+
cql3::cql_metadata_id_type calculated_metadata_id{bytes{}};
2031+
if (metadata_id.is_supported_by_protocol() && metadata_id.has_request_metadata_id() && metadata_id.has_response_metadata_id()) {
2032+
if (metadata_id.get_request_metadata_id() != metadata_id.get_response_metadata_id()) {
2033+
flags.remove<cql3::metadata::flag::NO_METADATA>();
2034+
flags.set<cql3::metadata::flag::METADATA_CHANGED>();
2035+
no_metadata = false;
2036+
}
2037+
}
2038+
20082039
write_int(flags.mask());
20092040
write_int(m.column_count());
20102041

@@ -2016,6 +2047,10 @@ void cql_server::response::write(const cql3::metadata& m, bool no_metadata) {
20162047
return;
20172048
}
20182049

2050+
if (flags.contains<cql3::metadata::flag::METADATA_CHANGED>()) {
2051+
write_short_bytes(metadata_id.get_response_metadata_id()._metadata_id);
2052+
}
2053+
20192054
auto names_i = m.get_names().begin();
20202055

20212056
if (global_tables_spec) {
@@ -2068,6 +2103,34 @@ void cql_server::response::write(const cql3::prepared_metadata& m, uint8_t versi
20682103
}
20692104
}
20702105

2106+
bool cql_metadata_id_wrapper::has_request_metadata_id() const {
2107+
if (!_supported_by_protocol) {
2108+
on_internal_error(clogger, "metadata_id is unsupported");
2109+
}
2110+
return _request_metadata_id.has_value();
2111+
}
2112+
2113+
bool cql_metadata_id_wrapper::has_response_metadata_id() const {
2114+
if (!_supported_by_protocol) {
2115+
on_internal_error(clogger, "metadata_id is unsupported");
2116+
}
2117+
return _response_metadata_id.has_value();
2118+
}
2119+
2120+
const cql3::cql_metadata_id_type& cql_metadata_id_wrapper::get_request_metadata_id() const {
2121+
if (!has_request_metadata_id()) {
2122+
on_internal_error(clogger, "request metadata_id is empty");
2123+
}
2124+
return _request_metadata_id.value();
2125+
}
2126+
2127+
const cql3::cql_metadata_id_type& cql_metadata_id_wrapper::get_response_metadata_id() const {
2128+
if (!has_response_metadata_id()) {
2129+
on_internal_error(clogger, "response metadata_id is empty");
2130+
}
2131+
return _response_metadata_id.value();
2132+
}
2133+
20712134
future<utils::chunked_vector<client_data>> cql_server::get_client_data() {
20722135
utils::chunked_vector<client_data> ret;
20732136
co_await for_each_gently([&ret] (const generic_server::connection& c) {

0 commit comments

Comments
 (0)