[PATCH v4 0/2] lwt: introduce "LWT" flag in prepared statement metadata and `cql_protocol_extensions` enum for features negotiation

[Pavel Solodovnikov]

This patch set adds a few new features in order to fix issue
#6259 (Extend the binary protocol resultMetadata with LWT flag).

The list of changes is briefly as follows:
- Add a new `LWT` flag to `cql3::prepared_metadata`,
which allows clients to clearly distinguish betwen lwt and
non-lwt statements without need to execute some custom parsing
logic (e.g. parsing the prepared query with regular expressions),
which is obviously quite fragile.
- Introduce the negotiation procedure for cql protocol extensions.
This is done via `cql_protocol_extension` enum and is expected
to have an appropriate mirroring implementation on the client
driver side in order to work properly.
- Implmenent a `LWT_ADD_METADATA_MARK` cql feature on top of the
aforementioned algorithm to make the feature negotiable and use
it conditionally (iff both server and client agrees with each
other on the set of cql extensions).

The feature is meant to be further utilized by client drivers
to use primary replicas consistently when dealing with conditional
statements.

NOTE: This patch set can be applied on its own without an implementation
on the client driver side, but it's best that the patch set is merged only
when a client-side impl is delivered and tested against these patches
(at first it would be our gocql driver but others should be adjusted as
well afterwards), which I intend to do after a short while.

Gocql-related work would be based on Henrik's PR:
https://github.com/scylladb/gocql/pull/41 (scylla: lwt optimization to
use primary replicas constistently). I'll send a notice shortly after
adjusted implementation with protocol negotiation is ready.

--
v3->v4 changes:
* Split the series into two commits to separate `cql_protocol_extensions`
and actual wiring of LWT metadata flag in the
`cql3::query_processor`.
* Address Gleb's review comments: revise docs and make features accept
multiple parameters under a single feature key in SUPPORTED message.

v2->v3 changes:
* Introduce `cql_protocol_extension` enum and a simple feature
negotiation infrastructure via SUPPORTED/STARTUP messages.
* Update the docs at `docs/protocol-extensions.md`.

v1->v2 changes:
* Updated commit message.

Pavel Solodovnikov (2):
transport: introduce `cql_protocol_extension` enum and cql protocol
extensions negotiation
lwt: introduce "LWT" flag in prepared statement metadata

configure.py | 1 +
cql3/cql_statement.hh | 4 ++
cql3/query_processor.hh | 5 +-
cql3/result_set.cc | 7 ++-
cql3/result_set.hh | 13 ++++-
cql3/statements/modification_statement.cc | 4 ++
cql3/statements/modification_statement.hh | 3 ++
docs/protocol-extensions.md | 61 +++++++++++++++++++--
service/client_state.hh | 17 ++++++
transport/cql_protocol_extension.cc | 51 ++++++++++++++++++
transport/cql_protocol_extension.hh | 66 +++++++++++++++++++++++
transport/messages/result_message.hh | 15 +++---
transport/server.cc | 22 +++++++-
13 files changed, 254 insertions(+), 15 deletions(-)
create mode 100644 transport/cql_protocol_extension.cc
create mode 100644 transport/cql_protocol_extension.hh

--
2.26.2

[Pavel Solodovnikov]

The patch introduces two new features to aid with negotiating
protocol extensions for the CQL protocol:
- `cql_protocol_extensions` enum, which holds all supported
extensions for the CQL protocol (currently contains only
`LWT_ADD_METADATA_MARK` extension, which will be mentioned
below).
- An additional mechainsm of negotiating cql protocol extensions
to be used in a client connection between a scylla server
and a client driver.

These extensions are propagated in SUPPORTED message sent from the
server side with "SCYLLA_" prefix and received back as a response
from the client driver in order to determine intersection between
the cql extensions that are both supported by the server and
acknowledged by a client driver.

This intersection of features is later determined to be a working
set of cql protocol extensions in use for the current `client_state`,
which is associated with a particular client connection.

This way we can easily settle on the used extensions set on
both sides of the connection.

Currently there is only one value: `LWT_ADD_METADATA_MARK`, which
regulates whether to set a designated bit in prepared statement
metadata indicating if the statement at hand is an lwt statement
or not (actual implementation for the feature will be in a later
patch).

Each extension can also propagate some custom parameters to the
corresponding key. CQL protocol specification allows to send
a list of values with each key in the SUPPORTED message, we use
that to pass parameters to extensions as `PARAM=VALUE` strings.

In case of `LWT_ADD_METADATA_MARK` it's
`SCYLLA_LWT_OPTIMIZATION_META_BIT_MASK` which designates the
bitmask for LWT flag in prepared statement metadata in order to be
used for lookup in a client library. The associated bits of code in
`cql3::prepared_metadata` are adjusted to accomodate the feature.

Also extend the underlying type of `flag` enum in
`cql3::prepared_metadata` to be `uint32_t` instead of `uint8_t`
because in either case flags mask is serialized as 32-bit integer.

In theory, shard-awareness extension support also should be
reworked in terms of provided minimal infrastructure, but for the
sake of simplicity, this is left to be done in a follow-up some
time later.

This solution eliminates the need to assume that all the client
drivers follow the CQL spec carefully because scylla-specific
features and protocol extensions could be enabled only in case both
server and client driver negotiate the supported feature set.

Tests: unit(dev, debug)

Signed-off-by: Pavel Solodovnikov <pa.solo...@scylladb.com>
---
configure.py | 1 +
cql3/result_set.hh | 10 ++++-
docs/protocol-extensions.md | 61 ++++++++++++++++++++++++--
service/client_state.hh | 17 ++++++++
transport/cql_protocol_extension.cc | 51 ++++++++++++++++++++++
transport/cql_protocol_extension.hh | 66 +++++++++++++++++++++++++++++
transport/server.cc | 22 +++++++++-
7 files changed, 223 insertions(+), 5 deletions(-)

create mode 100644 transport/cql_protocol_extension.cc
create mode 100644 transport/cql_protocol_extension.hh

diff --git a/configure.py b/configure.py
index 5a75d00bf..2106e8bf4 100755
--- a/configure.py
+++ b/configure.py
@@ -547,6 +547,7 @@ scylla_core = (['database.cc',
'sstables/prepended_input_stream.cc',
'sstables/m_format_read_helpers.cc',
'sstables/sstable_directory.cc',
+ 'transport/cql_protocol_extension.cc',
'transport/event.cc',
'transport/event_notifier.cc',
'transport/server.cc',
diff --git a/cql3/result_set.hh b/cql3/result_set.hh
index 2e11b1ac5..42b2a5ff5 100644
--- a/cql3/result_set.hh
+++ b/cql3/result_set.hh
@@ -128,14 +128,22 @@ ::shared_ptr<const cql3::metadata> make_empty_metadata();

class prepared_metadata {
public:
- enum class flag : uint8_t {
+ enum class flag : uint32_t {
GLOBAL_TABLES_SPEC = 0,
+ // Denotes whether the prepared statement at hand is an LWT statement.
+ //
+ // Use the last available bit in the flags since we don't want to clash
+ // with C* in case they add some other flag in one the next versions of binary protocol.
+ LWT = 31
};

using flag_enum = super_enum<flag,
flag::GLOBAL_TABLES_SPEC>;

using flag_enum_set = enum_set<flag_enum>;
+
+ static constexpr flag_enum_set::mask_type LWT_FLAG_MASK = flag_enum_set::mask_for<flag::LWT>();
+
private:
flag_enum_set _flags;
std::vector<lw_shared_ptr<column_specification>> _names;
diff --git a/docs/protocol-extensions.md b/docs/protocol-extensions.md
index 56572c5b7..49d6f77c4 100644
--- a/docs/protocol-extensions.md
+++ b/docs/protocol-extensions.md
@@ -8,12 +8,38 @@ continue to interoperate with Cassandra and other compatible servers
with no configuration needed; the driver can discover the extensions
and enable them conditionally.

-An extension can be discovered by using the OPTIONS request; the
-returned SUPPORTED response will have zero or more options beginning
-with SCYLLA indicating extensions defined in this documented, in
+An extension can be discovered by the client driver by using the OPTIONS
+request; the returned SUPPORTED response will have zero or more options
+beginning with `SCYLLA` indicating extensions defined in this document, in
addition to options documented by Cassandra. How to use the extension
is further explained in this document.

+# Extending protocol extensions support
+
+As mentioned above, in order to use a protocol extension feature by both
+server and client, they need to negotiate the used feature set when establishing
+a connection.
+
+The negotiation procedure has the following steps:
+ - Client sends the OPTIONS request to the Scylla instance to get a list of
+ protocol extensions that the server understands.
+ - Server sends the SUPPORTED message in reply to the OPTIONS request. The
+ message body is a string multimap, in which keys describe different
+ extensions and possibly one or more additional values specific to a
+ particular extension (specified as distinct values under a feature key in
+ the following form: `ARG_NAME=VALUE`).
+ - The client determines the set of compatible extensions which it is going
+ to use in the current connection by intersecting known capabilities list
+ with what it has received in SUPPORTED response.
+ - Client driver sends the STARTUP request with additional payload consisting
+ of key-value pairs, each describing a negotiated extension.
+ - Server determines the set of compatible extensions by intersecting known
+ list of protocol extensions with what it has received in STARTUP request.
+
+Both client and server use the same string identifiers for the keys to determine
+negotiated extension set, judging by the presence of a particular key in the
+SUPPORTED/STARTUP messages.
+
# Intranode sharding

This extension allows the driver to discover how Scylla internally
@@ -80,3 +106,32 @@ is for Java):

It is recommended that drivers open connections until they have at
least one connection per shard, then close excess connections.
+
+# LWT prepared statements metadata mark
+
+This extension allows the driver to discover whether LWT statements have a
+special bit set in prepared statement metadata flags, which indicates that
+the driver currently deals with an LWT statement.
+
+Having a designated flag gives the ability to reliably detect LWT statements
+and remove the need to execute custom parsing logic for each query, which is not
+only costly but also error-prone (e.g. parsing the prepared query with regular
+expressions).
+
+The feature is meant to be further utilized by client drivers to use primary
+replicas consistently when dealing with conditional statements.
+
+Choosing primary replicas in a predefined order ensures that in case of multiple
+LWT queries that contend on a single key, these queries will queue up at the
+replica rather than compete: choose the primary replica first, then, if the
+primary is known to be down, the first secondary, then the second secondary, and
+so on.
+This will reduce contention over hot keys and thus increase LWT performance.
+
+The feature is identified by the `SCYLLA_LWT_ADD_METADATA_MARK` key that is
+meant to be sent in the SUPPORTED message along with the following additional
+parameters:
+ - `SCYLLA_LWT_OPTIMIZATION_META_BIT_MASK` is a 32-bit integer that represents
+ the bit mask that should be used by the client to test against when checking
+ prepared statement metadata flags to see if the current query is conditional
+ or not.
diff --git a/service/client_state.hh b/service/client_state.hh
index 7a0f0236b..eab04b9c3 100644
--- a/service/client_state.hh
+++ b/service/client_state.hh
@@ -53,6 +53,9 @@
#include "tracing/tracing.hh"
#include "tracing/trace_state.hh"

+#include "enum_set.hh"
+#include "transport/cql_protocol_extension.hh"
+
namespace auth {
class resource;
}
@@ -354,6 +357,20 @@ class client_state {
}
}
#endif
+
+private:
+
+ cql_transport::cql_protocol_extension_enum_set _enabled_protocol_extensions;
+
+public:
+
+ bool is_protocol_extension_set(cql_transport::cql_protocol_extension ext) const {
+ return _enabled_protocol_extensions.contains(ext);
+ }
+
+ void set_protocol_extensions(cql_transport::cql_protocol_extension_enum_set exts) {
+ _enabled_protocol_extensions = std::move(exts);
+ }
};

}
diff --git a/transport/cql_protocol_extension.cc b/transport/cql_protocol_extension.cc
new file mode 100644
index 000000000..d1634b569
--- /dev/null
+++ b/transport/cql_protocol_extension.cc
@@ -0,0 +1,51 @@
+/*
+ * Copyright 2020 ScyllaDB
+ */
+
+/*
+ * This file is part of Scylla.
+ *
+ * Scylla is free software: you can redistribute it and/or modify
+ * it under the terms of the GNU Affero General Public License as published by
+ * the Free Software Foundation, either version 3 of the License, or
+ * (at your option) any later version.
+ *
+ * Scylla is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU Affero General Public License
+ * along with Scylla. If not, see <http://www.gnu.org/licenses/>.
+ */
+
+#include <seastar/core/print.hh>
+#include "transport/cql_protocol_extension.hh"
+#include "cql3/result_set.hh"
+
+#include <map>
+
+namespace cql_transport {
+
+static const std::map<cql_protocol_extension, seastar::sstring> EXTENSION_NAMES = {
+ {cql_protocol_extension::LWT_ADD_METADATA_MARK, "SCYLLA_LWT_ADD_METADATA_MARK"}
+};
+
+cql_protocol_extension_enum_set supported_cql_protocol_extensions() {
+ return cql_protocol_extension_enum_set::full();
+}
+
+const seastar::sstring& protocol_extension_name(cql_protocol_extension ext) {
+ return EXTENSION_NAMES.at(ext);
+}
+
+std::vector<seastar::sstring> additional_options_for_proto_ext(cql_protocol_extension ext) {
+ switch (ext) {
+ case cql_protocol_extension::LWT_ADD_METADATA_MARK:
+ return {format("LWT_OPTIMIZATION_META_BIT_MASK={:d}", cql3::prepared_metadata::LWT_FLAG_MASK)};
+ default:
+ return {};
+ }
+}
+
+} // namespace cql_transport
diff --git a/transport/cql_protocol_extension.hh b/transport/cql_protocol_extension.hh
new file mode 100644
index 000000000..689b447a9
--- /dev/null
+++ b/transport/cql_protocol_extension.hh
@@ -0,0 +1,66 @@
+/*
+ * Copyright 2020 ScyllaDB
+ */
+
+/*
+ * This file is part of Scylla.
+ *
+ * Scylla is free software: you can redistribute it and/or modify
+ * it under the terms of the GNU Affero General Public License as published by
+ * the Free Software Foundation, either version 3 of the License, or
+ * (at your option) any later version.
+ *
+ * Scylla is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU Affero General Public License
+ * along with Scylla. If not, see <http://www.gnu.org/licenses/>.
+ */
+#pragma once
+
+#include <seastar/core/sstring.hh>
+#include "enum_set.hh"
+
+#include <map>
+
+namespace cql_transport {
+
+/**
+ * CQL Protocol extensions. They can be viewed as an opportunity to provide
+ * some vendor-specific extensions to the CQL protocol without changing
+ * the version of the protocol itself (i.e. when the changes introduced by
+ * extensions are binary-compatible with the current version of the protocol).
+ *
+ * Extensions are meant to be passed between client and server in terms of
+ * SUPPORTED/STARTUP messages in order to negotiate compatible set of features
+ * to be used in a connection.
+ *
+ * The negotiation procedure and extensions themselves are documented in the
+ * `docs/protocol-extensions.md`.
+ */
+enum class cql_protocol_extension {
+ LWT_ADD_METADATA_MARK
+};
+
+using cql_protocol_extension_enum = super_enum<cql_protocol_extension,
+ cql_protocol_extension::LWT_ADD_METADATA_MARK>;
+
+using cql_protocol_extension_enum_set = enum_set<cql_protocol_extension_enum>;
+
+cql_protocol_extension_enum_set supported_cql_protocol_extensions();
+
+/**
+ * Returns the name of extension to be used in SUPPORTED/STARTUP feature negotiation.
+ */
+const seastar::sstring& protocol_extension_name(cql_protocol_extension ext);
+
+/**
+ * Returns a list of additional key-value pairs (in the form of "ARG=VALUE" string)
+ * that belong to a particular extension and provide some additional capabilities
+ * to be used by the client driver in order to support this extension.
+ */
+std::vector<seastar::sstring> additional_options_for_proto_ext(cql_protocol_extension ext);
+
+} // namespace cql_transport
diff --git a/transport/server.cc b/transport/server.cc
index 87c0f0796..9f2e9900a 100644
--- a/transport/server.cc
+++ b/transport/server.cc
@@ -65,6 +65,8 @@

#include "types/user.hh"

+#include "transport/cql_protocol_extension.hh"
+
namespace cql_transport {

static logging::logger clogger("cql_server");
@@ -728,6 +730,13 @@ future<std::unique_ptr<cql_server::response>> cql_server::connection::process_st
throw exceptions::protocol_exception(format("Unknown compression algorithm: {}", compression));
}
}
+ cql_protocol_extension_enum_set cql_proto_exts;
+ for (cql_protocol_extension ext : supported_cql_protocol_extensions()) {
+ if (options.find(protocol_extension_name(ext)) != options.cend()) {
+ cql_proto_exts.set(ext);
+ }
+ }
+ _client_state.set_protocol_extensions(std::move(cql_proto_exts));
auto& a = client_state.get_auth_service()->underlying_authenticator();
if (a.require_authentication()) {
return make_ready_future<std::unique_ptr<cql_server::response>>(make_autheticate(stream, a.qualified_java_name(), trace_state));
@@ -1213,8 +1222,19 @@ std::unique_ptr<cql_server::response> cql_server::connection::make_supported(int
opts.insert({"SCYLLA_SHARDING_IGNORE_MSB", format("{:d}", _server._config.sharding_ignore_msb)});
opts.insert({"SCYLLA_PARTITIONER", _server._config.partitioner_name});
}
+ for (cql_protocol_extension ext : supported_cql_protocol_extensions()) {
+ const sstring ext_key_name = protocol_extension_name(ext);
+ std::vector<sstring> params = additional_options_for_proto_ext(ext);
+ if (params.empty()) {
+ opts.emplace(ext_key_name, "");
+ } else {
+ for (sstring val : params) {
+ opts.emplace(ext_key_name, std::move(val));
+ }
+ }
+ }
auto response = std::make_unique<cql_server::response>(stream, cql_binary_opcode::SUPPORTED, tr_state);
- response->write_string_multimap(opts);
+ response->write_string_multimap(std::move(opts));
return response;
}

--
2.26.2

[Pavel Solodovnikov]

This patch adds a new `LWT` flag to `cql3::prepared_metadata`.

That allows clients to clearly distinguish betwen lwt and

non-lwt statements without need to execute some custom parsing
logic (e.g. parsing the prepared query with regular expressions),
which is obviously quite fragile.

The feature is meant to be further utilized by client drivers
to use primary replicas consistently when dealing with conditional
statements.

The value for the flag is chosen on purpose to be the last bit
in the flags bitset since we don't want to possibly clash with
C* implementation in case they add more possible flag values to
prepared metadata (though there is an issue regarding that:
https://issues.apache.org/jira/browse/CASSANDRA-15746).

If it's fixed in upstream Cassandra, then we could synchronize
the value for the flag with them.

Whether to use lwt optimization flag or not is handled by negotiation
procedure between scylla server and client library via SUPPORTED/STARTUP
messages (`LWT_ADD_METADATA_MARK` extension).

Tests: unit(dev, debug), manual testing with modified scylla/gocql driver

Fixes: #6259

Signed-off-by: Pavel Solodovnikov <pa.solo...@scylladb.com>
---

cql3/cql_statement.hh | 4 ++++
cql3/query_processor.hh | 5 +++--
cql3/result_set.cc | 7 ++++++-
cql3/result_set.hh | 3 ++-
cql3/statements/modification_statement.cc | 4 ++++
cql3/statements/modification_statement.hh | 3 +++
transport/messages/result_message.hh | 15 +++++++++------
7 files changed, 31 insertions(+), 10 deletions(-)

diff --git a/cql3/cql_statement.hh b/cql3/cql_statement.hh
index c3a6f7c70..b5feb02b8 100644
--- a/cql3/cql_statement.hh
+++ b/cql3/cql_statement.hh
@@ -108,6 +108,10 @@ class cql_statement {
virtual bool depends_on_column_family(const sstring& cf_name) const = 0;

virtual shared_ptr<const metadata> get_result_metadata() const = 0;
+
+ virtual bool is_conditional() const {
+ return false;
+ }
};

class cql_statement_no_metadata : public cql_statement {
diff --git a/cql3/query_processor.hh b/cql3/query_processor.hh
index 7cf18b397..b109d3adf 100644
--- a/cql3/query_processor.hh
+++ b/cql3/query_processor.hh
@@ -405,9 +405,10 @@ class query_processor {
}
assert(bound_terms == prepared->bound_names.size());
return make_ready_future<std::unique_ptr<statements::prepared_statement>>(std::move(prepared));
- }).then([&key, &id_getter] (auto prep_ptr) {
+ }).then([&key, &id_getter, &client_state] (auto prep_ptr) {
return make_ready_future<::shared_ptr<cql_transport::messages::result_message::prepared>>(
- ::make_shared<ResultMsgType>(id_getter(key), std::move(prep_ptr)));
+ ::make_shared<ResultMsgType>(id_getter(key), std::move(prep_ptr),
+ client_state.is_protocol_extension_set(cql_transport::cql_protocol_extension::LWT_ADD_METADATA_MARK)));
}).handle_exception_type([&query_string] (typename prepared_statements_cache::statement_is_too_big&) {
return make_exception_future<::shared_ptr<cql_transport::messages::result_message::prepared>>(
prepared_statement_is_too_big(query_string));
diff --git a/cql3/result_set.cc b/cql3/result_set.cc
index 4c9c55622..7dd836c3d 100644
--- a/cql3/result_set.cc
+++ b/cql3/result_set.cc
@@ -102,13 +102,18 @@ lw_shared_ptr<const service::pager::paging_state> metadata::paging_state() const
}

prepared_metadata::prepared_metadata(const std::vector<lw_shared_ptr<column_specification>>& names,
- const std::vector<uint16_t>& partition_key_bind_indices)
+ const std::vector<uint16_t>& partition_key_bind_indices,
+ bool is_conditional)
: _names{names}
, _partition_key_bind_indices{partition_key_bind_indices}
{
if (!names.empty() && column_specification::all_in_same_table(_names)) {
_flags.set<flag::GLOBAL_TABLES_SPEC>();
}
+
+ if (is_conditional) {
+ _flags.set<flag::LWT>();
+ }
}

prepared_metadata::flag_enum_set prepared_metadata::flags() const {
diff --git a/cql3/result_set.hh b/cql3/result_set.hh
index 42b2a5ff5..0b6a31a0d 100644
--- a/cql3/result_set.hh
+++ b/cql3/result_set.hh
@@ -150,7 +150,8 @@ class prepared_metadata {
std::vector<uint16_t> _partition_key_bind_indices;
public:
prepared_metadata(const std::vector<lw_shared_ptr<column_specification>>& names,
- const std::vector<uint16_t>& partition_key_bind_indices);
+ const std::vector<uint16_t>& partition_key_bind_indices,
+ bool is_conditional);

flag_enum_set flags() const;
const std::vector<lw_shared_ptr<column_specification>>& names() const;
diff --git a/cql3/statements/modification_statement.cc b/cql3/statements/modification_statement.cc
index 74e7bf138..f241a4f6e 100644
--- a/cql3/statements/modification_statement.cc
+++ b/cql3/statements/modification_statement.cc
@@ -646,6 +646,10 @@ void modification_statement::inc_cql_stats(bool is_internal) const {
++_stats.query_cnt(src_sel, _ks_sel, cond_sel, type);
}

+bool modification_statement::is_conditional() const {
+ return _if_exists || _if_not_exists || _has_regular_column_conditions || _has_static_column_conditions;
+}
+
void modification_statement::add_condition(lw_shared_ptr<column_condition> cond) {
if (cond->column.is_static()) {
_has_static_column_conditions = true;
diff --git a/cql3/statements/modification_statement.hh b/cql3/statements/modification_statement.hh
index 8f547ab0d..de5dfeaa6 100644
--- a/cql3/statements/modification_statement.hh
+++ b/cql3/statements/modification_statement.hh
@@ -184,6 +184,9 @@ class modification_statement : public cql_statement_opt_metadata {
const restrictions::statement_restrictions& restrictions() const {
return *_restrictions;
}
+
+ bool is_conditional() const override;
+
public:
void add_condition(lw_shared_ptr<column_condition> cond);

diff --git a/transport/messages/result_message.hh b/transport/messages/result_message.hh
index 89b0d41d0..45fa31a9d 100644
--- a/transport/messages/result_message.hh
+++ b/transport/messages/result_message.hh
@@ -41,9 +41,12 @@ class result_message::prepared : public result_message {
cql3::prepared_metadata _metadata;
::shared_ptr<const cql3::metadata> _result_metadata;
protected:
- prepared(cql3::statements::prepared_statement::checked_weak_ptr prepared)
+ prepared(cql3::statements::prepared_statement::checked_weak_ptr prepared, bool support_lwt_opt)
: _prepared(std::move(prepared))
- , _metadata(_prepared->bound_names, _prepared->partition_key_bind_indices)
+ , _metadata(
+ _prepared->bound_names,
+ _prepared->partition_key_bind_indices,
+ support_lwt_opt ? _prepared->statement->is_conditional() : false)
, _result_metadata{extract_result_metadata(_prepared->statement)}
{ }
public:
@@ -138,8 +141,8 @@ std::ostream& operator<<(std::ostream& os, const result_message::set_keyspace& m
class result_message::prepared::cql : public result_message::prepared {
bytes _id;
public:
- cql(const bytes& id, cql3::statements::prepared_statement::checked_weak_ptr p)
- : result_message::prepared(std::move(p))
+ cql(const bytes& id, cql3::statements::prepared_statement::checked_weak_ptr p, bool support_lwt_opt)
+ : result_message::prepared(std::move(p), support_lwt_opt)
, _id{id}
{ }

@@ -165,8 +168,8 @@ std::ostream& operator<<(std::ostream& os, const result_message::prepared::cql&
class result_message::prepared::thrift : public result_message::prepared {
int32_t _id;
public:
- thrift(int32_t id, cql3::statements::prepared_statement::checked_weak_ptr prepared)
- : result_message::prepared(std::move(prepared))
+ thrift(int32_t id, cql3::statements::prepared_statement::checked_weak_ptr prepared, bool support_lwt_opt)
+ : result_message::prepared(std::move(prepared), support_lwt_opt)
, _id{id}
{ }

--
2.26.2

[Gleb Natapov]

We should copy the state in constructor that is used to copy
client_state between shards.

> --
> You received this message because you are subscribed to the Google Groups "ScyllaDB development" group.
> To unsubscribe from this group and stop receiving emails from it, send an email to scylladb-dev...@googlegroups.com.
> To view this discussion on the web visit https://groups.google.com/d/msgid/scylladb-dev/20200615133548.231613-2-pa.solodovnikov%40scylladb.com.

--
Gleb.

[Gleb Natapov]

On Mon, Jun 15, 2020 at 04:35:48PM +0300, Pavel Solodovnikov wrote:
> This patch adds a new `LWT` flag to `cql3::prepared_metadata`.
>
> That allows clients to clearly distinguish betwen lwt and
> non-lwt statements without need to execute some custom parsing
> logic (e.g. parsing the prepared query with regular expressions),
> which is obviously quite fragile.
>
> The feature is meant to be further utilized by client drivers
> to use primary replicas consistently when dealing with conditional
> statements.
>
> The value for the flag is chosen on purpose to be the last bit
> in the flags bitset since we don't want to possibly clash with
> C* implementation in case they add more possible flag values to
> prepared metadata (though there is an issue regarding that:
> https://issues.apache.org/jira/browse/CASSANDRA-15746).
>

The value is chosen by the previous patch.

> --
> You received this message because you are subscribed to the Google Groups "ScyllaDB development" group.
> To unsubscribe from this group and stop receiving emails from it, send an email to scylladb-dev...@googlegroups.com.

> To view this discussion on the web visit https://groups.google.com/d/msgid/scylladb-dev/20200615133548.231613-3-pa.solodovnikov%40scylladb.com.

--
Gleb.

[Pavel Solodovnikov]

Thanks for noticing. I'll adjust.

[Pavel Solodovnikov]

On Tue, Jun 16, 2020 at 9:44 AM Gleb Natapov <gl...@scylladb.com> wrote:

On Mon, Jun 15, 2020 at 04:35:48PM +0300, Pavel Solodovnikov wrote:
> This patch adds a new `LWT` flag to `cql3::prepared_metadata`.
>
> That allows clients to clearly distinguish betwen lwt and
> non-lwt statements without need to execute some custom parsing
> logic (e.g. parsing the prepared query with regular expressions),
> which is obviously quite fragile.
>
> The feature is meant to be further utilized by client drivers
> to use primary replicas consistently when dealing with conditional
> statements.
>
> The value for the flag is chosen on purpose to be the last bit
> in the flags bitset since we don't want to possibly clash with
> C* implementation in case they add more possible flag values to
> prepared metadata (though there is an issue regarding that:
> https://issues.apache.org/jira/browse/CASSANDRA-15746).
>
The value is chosen by the previous patch.

Thanks, I'll move the description to the previous patch.

[Pavel Solodovnikov]

This patch set adds a few new features in order to fix issue
#6259 (Extend the binary protocol resultMetadata with LWT flag).

The list of changes is briefly as follows:

- Add a new `LWT` flag to `cql3::prepared_metadata`,
which allows clients to clearly distinguish betwen lwt and

non-lwt statements without need to execute some custom parsing
logic (e.g. parsing the prepared query with regular expressions),
which is obviously quite fragile.

- Introduce the negotiation procedure for cql protocol extensions.
This is done via `cql_protocol_extension` enum and is expected
to have an appropriate mirroring implementation on the client
driver side in order to work properly.
- Implmenent a `LWT_ADD_METADATA_MARK` cql feature on top of the
aforementioned algorithm to make the feature negotiable and use
it conditionally (iff both server and client agrees with each
other on the set of cql extensions).

The feature is meant to be further utilized by client drivers
to use primary replicas consistently when dealing with conditional
statements.

NOTE: This patch set can be applied on its own without an implementation
on the client driver side, but it's best that the patch set is merged only
when a client-side impl is delivered and tested against these patches
(at first it would be our gocql driver but others should be adjusted as
well afterwards), which I intend to do after a short while.

Gocql-related work would be based on Henrik's PR:
https://github.com/scylladb/gocql/pull/41 (scylla: lwt optimization to
use primary replicas constistently). I'll send a notice shortly after
adjusted implementation with protocol negotiation is ready.

--

v4->v5 changes:
* Copy `_enabled_protocol_extensions` in `client_state` ctor used when
moving a client_state to another shard.
* Add missing entry for `flag::LWT` to `flag_enum` in
`cql3::prepared_metadata`.
* Adjusted commit messages wrt. Gleb's request for changes.

v3->v4 changes:
* Split the series into two commits to separate `cql_protocol_extensions`
and actual wiring of LWT metadata flag in the
`cql3::query_processor`.
* Address Gleb's review comments: revise docs and make features accept
multiple parameters under a single feature key in SUPPORTED message.

v2->v3 changes:
* Introduce `cql_protocol_extension` enum and a simple feature
negotiation infrastructure via SUPPORTED/STARTUP messages.
* Update the docs at `docs/protocol-extensions.md`.

v1->v2 changes:
* Updated commit message.


Pavel Solodovnikov (2):
transport: introduce `cql_protocol_extension` enum and cql protocol
extensions negotiation
lwt: introduce "LWT" flag in prepared statement metadata

configure.py | 1 +
cql3/cql_statement.hh | 4 ++
cql3/query_processor.hh | 5 +-
cql3/result_set.cc | 7 ++-

cql3/result_set.hh | 16 ++++--

cql3/statements/modification_statement.cc | 4 ++
cql3/statements/modification_statement.hh | 3 ++
docs/protocol-extensions.md | 61 +++++++++++++++++++--

service/client_state.hh | 20 ++++++-

transport/cql_protocol_extension.cc | 51 ++++++++++++++++++
transport/cql_protocol_extension.hh | 66 +++++++++++++++++++++++
transport/messages/result_message.hh | 15 +++---
transport/server.cc | 22 +++++++-

13 files changed, 258 insertions(+), 17 deletions(-)

create mode 100644 transport/cql_protocol_extension.cc
create mode 100644 transport/cql_protocol_extension.hh

--
2.26.2

[Pavel Solodovnikov]

The value for the flag is chosen on purpose to be the last bit
in the flags bitset since we don't want to possibly clash with
C* implementation in case they add more possible flag values to
prepared metadata (though there is an issue regarding that:
https://issues.apache.org/jira/browse/CASSANDRA-15746).

If it's fixed in upstream Cassandra, then we could synchronize
the value for the flag with them.

Also extend the underlying type of `flag` enum in
`cql3::prepared_metadata` to be `uint32_t` instead of `uint8_t`
because in either case flags mask is serialized as 32-bit integer.

In theory, shard-awareness extension support also should be
reworked in terms of provided minimal infrastructure, but for the
sake of simplicity, this is left to be done in a follow-up some
time later.

This solution eliminates the need to assume that all the client
drivers follow the CQL spec carefully because scylla-specific
features and protocol extensions could be enabled only in case both
server and client driver negotiate the supported feature set.

Tests: unit(dev, debug)

Signed-off-by: Pavel Solodovnikov <pa.solo...@scylladb.com>
---

configure.py | 1 +
cql3/result_set.hh | 13 +++++-
docs/protocol-extensions.md | 61 ++++++++++++++++++++++++--
service/client_state.hh | 20 ++++++++-

transport/cql_protocol_extension.cc | 51 ++++++++++++++++++++++
transport/cql_protocol_extension.hh | 66 +++++++++++++++++++++++++++++
transport/server.cc | 22 +++++++++-

7 files changed, 227 insertions(+), 7 deletions(-)

create mode 100644 transport/cql_protocol_extension.cc
create mode 100644 transport/cql_protocol_extension.hh

diff --git a/configure.py b/configure.py
index 5a75d00bf..2106e8bf4 100755
--- a/configure.py
+++ b/configure.py
@@ -547,6 +547,7 @@ scylla_core = (['database.cc',
'sstables/prepended_input_stream.cc',
'sstables/m_format_read_helpers.cc',
'sstables/sstable_directory.cc',
+ 'transport/cql_protocol_extension.cc',
'transport/event.cc',
'transport/event_notifier.cc',
'transport/server.cc',

diff --git a/cql3/result_set.hh b/cql3/result_set.hh
index 2e11b1ac5..86f666852 100644
--- a/cql3/result_set.hh
+++ b/cql3/result_set.hh
@@ -128,14 +128,23 @@ ::shared_ptr<const cql3::metadata> make_empty_metadata();

class prepared_metadata {
public:
- enum class flag : uint8_t {
+ enum class flag : uint32_t {
GLOBAL_TABLES_SPEC = 0,
+ // Denotes whether the prepared statement at hand is an LWT statement.
+ //

+ // Use the last available bit in the flags since we don't want to clash

+ // with C* in case they add some other flag in one the next versions of binary protocol.
+ LWT = 31
};

using flag_enum = super_enum<flag,

- flag::GLOBAL_TABLES_SPEC>;
+ flag::GLOBAL_TABLES_SPEC,
+ flag::LWT>;

+only costly but also error-prone (e.g. parsing the prepared query with regular
+expressions).
+
+The feature is meant to be further utilized by client drivers to use primary
+replicas consistently when dealing with conditional statements.

+
+Choosing primary replicas in a predefined order ensures that in case of multiple
+LWT queries that contend on a single key, these queries will queue up at the
+replica rather than compete: choose the primary replica first, then, if the
+primary is known to be down, the first secondary, then the second secondary, and
+so on.
+This will reduce contention over hot keys and thus increase LWT performance.
+
+The feature is identified by the `SCYLLA_LWT_ADD_METADATA_MARK` key that is
+meant to be sent in the SUPPORTED message along with the following additional
+parameters:
+ - `SCYLLA_LWT_OPTIMIZATION_META_BIT_MASK` is a 32-bit integer that represents
+ the bit mask that should be used by the client to test against when checking
+ prepared statement metadata flags to see if the current query is conditional
+ or not.
diff --git a/service/client_state.hh b/service/client_state.hh

index 7a0f0236b..457273745 100644

--- a/service/client_state.hh
+++ b/service/client_state.hh
@@ -53,6 +53,9 @@
#include "tracing/tracing.hh"
#include "tracing/trace_state.hh"

+#include "enum_set.hh"
+#include "transport/cql_protocol_extension.hh"
+
namespace auth {
class resource;
}

@@ -87,7 +90,8 @@ class client_state {
client_state(const client_state* cs, seastar::sharded<auth::service>* auth_service)
: _keyspace(cs->_keyspace), _user(cs->_user), _auth_state(cs->_auth_state),
_is_internal(cs->_is_internal), _is_thrift(cs->_is_thrift), _remote_address(cs->_remote_address),
- _auth_service(auth_service ? &auth_service->local() : nullptr) {}
+ _auth_service(auth_service ? &auth_service->local() : nullptr),
+ _enabled_protocol_extensions(cs->_enabled_protocol_extensions) {}
friend client_state_for_another_shard;
private:
sstring _keyspace;
@@ -354,6 +358,20 @@ class client_state {

}
}
#endif
+
+private:
+
+ cql_transport::cql_protocol_extension_enum_set _enabled_protocol_extensions;

[Pavel Solodovnikov]

This patch adds a new `LWT` flag to `cql3::prepared_metadata`.

That allows clients to clearly distinguish betwen lwt and

non-lwt statements without need to execute some custom parsing
logic (e.g. parsing the prepared query with regular expressions),
which is obviously quite fragile.

The feature is meant to be further utilized by client drivers
to use primary replicas consistently when dealing with conditional
statements.

Whether to use lwt optimization flag or not is handled by negotiation
procedure between scylla server and client library via SUPPORTED/STARTUP
messages (`LWT_ADD_METADATA_MARK` extension).

Tests: unit(dev, debug), manual testing with modified scylla/gocql driver

Fixes: #6259

Signed-off-by: Pavel Solodovnikov <pa.solo...@scylladb.com>
---

diff --git a/cql3/result_set.hh b/cql3/result_set.hh
index 86f666852..898c64628 100644
--- a/cql3/result_set.hh
+++ b/cql3/result_set.hh
@@ -151,7 +151,8 @@ class prepared_metadata {

[Pavel Solodovnikov]

This patch set adds a few new features in order to fix issue
#6259 (Extend the binary protocol resultMetadata with LWT flag).

The list of changes is briefly as follows:

- Add a new `LWT` flag to `cql3::prepared_metadata`,
which allows clients to clearly distinguish betwen lwt and

non-lwt statements without need to execute some custom parsing
logic (e.g. parsing the prepared query with regular expressions),
which is obviously quite fragile.

- Introduce the negotiation procedure for cql protocol extensions.
This is done via `cql_protocol_extension` enum and is expected
to have an appropriate mirroring implementation on the client
driver side in order to work properly.
- Implmenent a `LWT_ADD_METADATA_MARK` cql feature on top of the
aforementioned algorithm to make the feature negotiable and use
it conditionally (iff both server and client agrees with each
other on the set of cql extensions).

The feature is meant to be further utilized by client drivers
to use primary replicas consistently when dealing with conditional
statements.

13 files changed, 258 insertions(+), 17 deletions(-)

create mode 100644 transport/cql_protocol_extension.cc
create mode 100644 transport/cql_protocol_extension.hh

--
2.26.2

[Pavel Solodovnikov]

Sorry, I forgot to include the `[PATCH]` tag in the cover letter topic. Will resend with a proper title.

[Gleb Natapov]

LGTM

> --
> You received this message because you are subscribed to the Google Groups "ScyllaDB development" group.
> To unsubscribe from this group and stop receiving emails from it, send an email to scylladb-dev...@googlegroups.com.

> To view this discussion on the web visit https://groups.google.com/d/msgid/scylladb-dev/20200616084217.332712-1-pa.solodovnikov%40scylladb.com.

--
Gleb.

[Tomasz Grabiec]

GIT URL please.

--
You received this message because you are subscribed to the Google Groups "ScyllaDB development" group.
To unsubscribe from this group and stop receiving emails from it, send an email to scylladb-dev...@googlegroups.com.

To view this discussion on the web visit https://groups.google.com/d/msgid/scylladb-dev/20200616084411.333344-1-pa.solodovnikov%40scylladb.com.

[Pavel Solodovnikov]

Sorry, here it is: g...@github.com:ManManson/scylla feature/lwt_prepared_meta_flag_2

GitHub view: https://github.com/ManManson/scylla/tree/feature/lwt_prepared_meta_flag_2

[Tomasz Grabiec]

modification_statement::do_execute() uses has_conditions() to decide whether to go with the LWT path. Shouldn't we use the same check? 

 

--
You received this message because you are subscribed to the Google Groups "ScyllaDB development" group.
To unsubscribe from this group and stop receiving emails from it, send an email to scylladb-dev...@googlegroups.com.

To view this discussion on the web visit https://groups.google.com/d/msgid/scylladb-dev/20200616084217.332712-3-pa.solodovnikov%40scylladb.com.

[Pavel Solodovnikov]

Yes, you are right. I'll adjust.

What confused me at first was the fact that `has_conditions()` does not check for

`_if_exists` or `_if_not_exists` and it's not evident that these indeed participate in

setting _has_{static|regular}_column_conditions until you look into the source code where it's actually used.

[Pavel Solodovnikov]

This patch set adds a few new features in order to fix issue
#6259 (Extend the binary protocol resultMetadata with LWT flag).

The list of changes is briefly as follows:

- Add a new `LWT` flag to `cql3::prepared_metadata`,
which allows clients to clearly distinguish betwen lwt and

non-lwt statements without need to execute some custom parsing
logic (e.g. parsing the prepared query with regular expressions),
which is obviously quite fragile.

- Introduce the negotiation procedure for cql protocol extensions.
This is done via `cql_protocol_extension` enum and is expected
to have an appropriate mirroring implementation on the client
driver side in order to work properly.
- Implmenent a `LWT_ADD_METADATA_MARK` cql feature on top of the
aforementioned algorithm to make the feature negotiable and use
it conditionally (iff both server and client agrees with each
other on the set of cql extensions).

The feature is meant to be further utilized by client drivers
to use primary replicas consistently when dealing with conditional
statements.

NOTE: This patch set can be applied on its own without an implementation
on the client driver side, but it's best that the patch set is merged only
when a client-side impl is delivered and tested against these patches
(at first it would be our gocql driver but others should be adjusted as
well afterwards), which I intend to do after a short while.

Gocql-related work would be based on Henrik's PR:
https://github.com/scylladb/gocql/pull/41 (scylla: lwt optimization to
use primary replicas constistently). I'll send a notice shortly after
adjusted implementation with protocol negotiation is ready.

--

v5->v6 changes:
* modification_statement::is_conditional() now uses has_conditions()

[Pavel Solodovnikov]

Signed-off-by: Pavel Solodovnikov <pa.solo...@scylladb.com>
---

configure.py | 1 +
cql3/result_set.hh | 13 +++++-
docs/protocol-extensions.md | 61 ++++++++++++++++++++++++--
service/client_state.hh | 20 ++++++++-
transport/cql_protocol_extension.cc | 51 ++++++++++++++++++++++
transport/cql_protocol_extension.hh | 66 +++++++++++++++++++++++++++++
transport/server.cc | 22 +++++++++-

7 files changed, 227 insertions(+), 7 deletions(-)

create mode 100644 transport/cql_protocol_extension.cc
create mode 100644 transport/cql_protocol_extension.hh

diff --git a/configure.py b/configure.py
index 5a75d00bf..2106e8bf4 100755
--- a/configure.py
+++ b/configure.py
@@ -547,6 +547,7 @@ scylla_core = (['database.cc',
'sstables/prepended_input_stream.cc',
'sstables/m_format_read_helpers.cc',
'sstables/sstable_directory.cc',
+ 'transport/cql_protocol_extension.cc',
'transport/event.cc',
'transport/event_notifier.cc',
'transport/server.cc',

diff --git a/cql3/result_set.hh b/cql3/result_set.hh
index 2e11b1ac5..86f666852 100644
--- a/cql3/result_set.hh
+++ b/cql3/result_set.hh

[Pavel Solodovnikov]

This patch adds a new `LWT` flag to `cql3::prepared_metadata`.

That allows clients to clearly distinguish betwen lwt and

non-lwt statements without need to execute some custom parsing
logic (e.g. parsing the prepared query with regular expressions),
which is obviously quite fragile.

The feature is meant to be further utilized by client drivers
to use primary replicas consistently when dealing with conditional
statements.

Whether to use lwt optimization flag or not is handled by negotiation
procedure between scylla server and client library via SUPPORTED/STARTUP
messages (`LWT_ADD_METADATA_MARK` extension).

Tests: unit(dev, debug), manual testing with modified scylla/gocql driver

Fixes: #6259

Signed-off-by: Pavel Solodovnikov <pa.solo...@scylladb.com>
---

diff --git a/cql3/result_set.hh b/cql3/result_set.hh
index 86f666852..898c64628 100644
--- a/cql3/result_set.hh
+++ b/cql3/result_set.hh

@@ -151,7 +151,8 @@ class prepared_metadata {
std::vector<uint16_t> _partition_key_bind_indices;
public:
prepared_metadata(const std::vector<lw_shared_ptr<column_specification>>& names,
- const std::vector<uint16_t>& partition_key_bind_indices);
+ const std::vector<uint16_t>& partition_key_bind_indices,
+ bool is_conditional);

flag_enum_set flags() const;
const std::vector<lw_shared_ptr<column_specification>>& names() const;
diff --git a/cql3/statements/modification_statement.cc b/cql3/statements/modification_statement.cc

index 74e7bf138..2285719f8 100644

--- a/cql3/statements/modification_statement.cc
+++ b/cql3/statements/modification_statement.cc
@@ -646,6 +646,10 @@ void modification_statement::inc_cql_stats(bool is_internal) const {
++_stats.query_cnt(src_sel, _ks_sel, cond_sel, type);
}

+bool modification_statement::is_conditional() const {

+ return has_conditions();
+}

[Tomasz Grabiec]

Git URL please.

--
You received this message because you are subscribed to the Google Groups "ScyllaDB development" group.
To unsubscribe from this group and stop receiving emails from it, send an email to scylladb-dev...@googlegroups.com.

To view this discussion on the web visit https://groups.google.com/d/msgid/scylladb-dev/20200629093423.55502-1-pa.solodovnikov%40scylladb.com.

[Pavel Solodovnikov]

g...@github.com:ManManson/scylla feature/lwt_prepared_meta_flag_2

[Commit Bot]

From: Pavel Solodovnikov <pa.solo...@scylladb.com>
Committer: Pavel Solodovnikov <pa.solo...@scylladb.com>
Branch: next

transport: introduce `cql_protocol_extension` enum and cql protocol extensions negotiation

Signed-off-by: Pavel Solodovnikov <pa.solo...@scylladb.com>

---

diff --git a/configure.py b/configure.py
--- a/configure.py
+++ b/configure.py
@@ -547,6 +547,7 @@ def find_headers(repodir, excluded_dirs):

'sstables/prepended_input_stream.cc',
'sstables/m_format_read_helpers.cc',
'sstables/sstable_directory.cc',
+ 'transport/cql_protocol_extension.cc',
'transport/event.cc',
'transport/event_notifier.cc',
'transport/server.cc',
diff --git a/cql3/result_set.hh b/cql3/result_set.hh

--- a/cql3/result_set.hh
+++ b/cql3/result_set.hh
@@ -128,14 +128,23 @@ public:

class prepared_metadata {
public:
- enum class flag : uint8_t {
+ enum class flag : uint32_t {
GLOBAL_TABLES_SPEC = 0,
+ // Denotes whether the prepared statement at hand is an LWT statement.
+ //
+ // Use the last available bit in the flags since we don't want to clash
+ // with C* in case they add some other flag in one the next versions of binary protocol.
+ LWT = 31
};

using flag_enum = super_enum<flag,
- flag::GLOBAL_TABLES_SPEC>;
+ flag::GLOBAL_TABLES_SPEC,
+ flag::LWT>;

using flag_enum_set = enum_set<flag_enum>;
+
+ static constexpr flag_enum_set::mask_type LWT_FLAG_MASK = flag_enum_set::mask_for<flag::LWT>();
+
private:
flag_enum_set _flags;
std::vector<lw_shared_ptr<column_specification>> _names;
diff --git a/docs/protocol-extensions.md b/docs/protocol-extensions.md

--- a/service/client_state.hh
+++ b/service/client_state.hh
@@ -53,6 +53,9 @@
#include "tracing/tracing.hh"
#include "tracing/trace_state.hh"

+#include "enum_set.hh"
+#include "transport/cql_protocol_extension.hh"
+
namespace auth {
class resource;
}

@@ -87,7 +90,8 @@ private:

client_state(const client_state* cs, seastar::sharded<auth::service>* auth_service)
: _keyspace(cs->_keyspace), _user(cs->_user), _auth_state(cs->_auth_state),
_is_internal(cs->_is_internal), _is_thrift(cs->_is_thrift), _remote_address(cs->_remote_address),
- _auth_service(auth_service ? &auth_service->local() : nullptr) {}
+ _auth_service(auth_service ? &auth_service->local() : nullptr),
+ _enabled_protocol_extensions(cs->_enabled_protocol_extensions) {}
friend client_state_for_another_shard;
private:
sstring _keyspace;

@@ -354,6 +358,20 @@ public:

}
}
#endif
+
+private:
+
+ cql_transport::cql_protocol_extension_enum_set _enabled_protocol_extensions;
+
+public:
+
+ bool is_protocol_extension_set(cql_transport::cql_protocol_extension ext) const {
+ return _enabled_protocol_extensions.contains(ext);
+ }
+
+ void set_protocol_extensions(cql_transport::cql_protocol_extension_enum_set exts) {
+ _enabled_protocol_extensions = std::move(exts);
+ }
};

}
diff --git a/transport/cql_protocol_extension.cc b/transport/cql_protocol_extension.cc

--- a/transport/cql_protocol_extension.cc

--- a/transport/cql_protocol_extension.hh

[Commit Bot]

From: Pavel Solodovnikov <pa.solo...@scylladb.com>
Committer: Pavel Solodovnikov <pa.solo...@scylladb.com>
Branch: next

lwt: introduce "LWT" flag in prepared statement metadata

This patch adds a new `LWT` flag to `cql3::prepared_metadata`.

That allows clients to clearly distinguish betwen lwt and

non-lwt statements without need to execute some custom parsing
logic (e.g. parsing the prepared query with regular expressions),
which is obviously quite fragile.

The feature is meant to be further utilized by client drivers

to use primary replicas consistently when dealing with conditional
statements.

Whether to use lwt optimization flag or not is handled by negotiation
procedure between scylla server and client library via SUPPORTED/STARTUP
messages (`LWT_ADD_METADATA_MARK` extension).

Tests: unit(dev, debug), manual testing with modified scylla/gocql driver

Signed-off-by: Pavel Solodovnikov <pa.solo...@scylladb.com>

---

diff --git a/cql3/cql_statement.hh b/cql3/cql_statement.hh
--- a/cql3/cql_statement.hh
+++ b/cql3/cql_statement.hh
@@ -108,6 +108,10 @@ public:

virtual bool depends_on_column_family(const sstring& cf_name) const = 0;

virtual shared_ptr<const metadata> get_result_metadata() const = 0;
+
+ virtual bool is_conditional() const {
+ return false;
+ }
};

class cql_statement_no_metadata : public cql_statement {
diff --git a/cql3/query_processor.hh b/cql3/query_processor.hh

--- a/cql3/query_processor.hh
+++ b/cql3/query_processor.hh
@@ -405,9 +405,10 @@ private:

}
assert(bound_terms == prepared->bound_names.size());
return make_ready_future<std::unique_ptr<statements::prepared_statement>>(std::move(prepared));
- }).then([&key, &id_getter] (auto prep_ptr) {
+ }).then([&key, &id_getter, &client_state] (auto prep_ptr) {
return make_ready_future<::shared_ptr<cql_transport::messages::result_message::prepared>>(
- ::make_shared<ResultMsgType>(id_getter(key), std::move(prep_ptr)));
+ ::make_shared<ResultMsgType>(id_getter(key), std::move(prep_ptr),
+ client_state.is_protocol_extension_set(cql_transport::cql_protocol_extension::LWT_ADD_METADATA_MARK)));
}).handle_exception_type([&query_string] (typename prepared_statements_cache::statement_is_too_big&) {
return make_exception_future<::shared_ptr<cql_transport::messages::result_message::prepared>>(
prepared_statement_is_too_big(query_string));
diff --git a/cql3/result_set.cc b/cql3/result_set.cc

--- a/cql3/result_set.cc
+++ b/cql3/result_set.cc
@@ -102,13 +102,18 @@ lw_shared_ptr<const service::pager::paging_state> metadata::paging_state() const
}

prepared_metadata::prepared_metadata(const std::vector<lw_shared_ptr<column_specification>>& names,
- const std::vector<uint16_t>& partition_key_bind_indices)
+ const std::vector<uint16_t>& partition_key_bind_indices,
+ bool is_conditional)
: _names{names}
, _partition_key_bind_indices{partition_key_bind_indices}
{
if (!names.empty() && column_specification::all_in_same_table(_names)) {
_flags.set<flag::GLOBAL_TABLES_SPEC>();
}
+
+ if (is_conditional) {
+ _flags.set<flag::LWT>();
+ }
}

prepared_metadata::flag_enum_set prepared_metadata::flags() const {

diff --git a/cql3/result_set.hh b/cql3/result_set.hh
--- a/cql3/result_set.hh
+++ b/cql3/result_set.hh

@@ -151,7 +151,8 @@ private:

std::vector<uint16_t> _partition_key_bind_indices;
public:
prepared_metadata(const std::vector<lw_shared_ptr<column_specification>>& names,
- const std::vector<uint16_t>& partition_key_bind_indices);
+ const std::vector<uint16_t>& partition_key_bind_indices,
+ bool is_conditional);

flag_enum_set flags() const;
const std::vector<lw_shared_ptr<column_specification>>& names() const;
diff --git a/cql3/statements/modification_statement.cc b/cql3/statements/modification_statement.cc

--- a/cql3/statements/modification_statement.cc
+++ b/cql3/statements/modification_statement.cc
@@ -646,6 +646,10 @@ void modification_statement::inc_cql_stats(bool is_internal) const {
++_stats.query_cnt(src_sel, _ks_sel, cond_sel, type);
}

+bool modification_statement::is_conditional() const {
+ return has_conditions();
+}
+
void modification_statement::add_condition(lw_shared_ptr<column_condition> cond) {
if (cond->column.is_static()) {
_has_static_column_conditions = true;
diff --git a/cql3/statements/modification_statement.hh b/cql3/statements/modification_statement.hh

--- a/cql3/statements/modification_statement.hh
+++ b/cql3/statements/modification_statement.hh
@@ -184,6 +184,9 @@ public:

const restrictions::statement_restrictions& restrictions() const {
return *_restrictions;
}
+
+ bool is_conditional() const override;
+
public:
void add_condition(lw_shared_ptr<column_condition> cond);

diff --git a/transport/messages/result_message.hh b/transport/messages/result_message.hh

--- a/transport/messages/result_message.hh
+++ b/transport/messages/result_message.hh
@@ -41,9 +41,12 @@ private:

[Commit Bot]

From: Pavel Solodovnikov <pa.solo...@scylladb.com>
Committer: Pavel Solodovnikov <pa.solo...@scylladb.com>

Branch: master

Signed-off-by: Pavel Solodovnikov <pa.solo...@scylladb.com>

---

diff --git a/configure.py b/configure.py
--- a/configure.py
+++ b/configure.py
@@ -547,6 +547,7 @@ def find_headers(repodir, excluded_dirs):
'sstables/prepended_input_stream.cc',
'sstables/m_format_read_helpers.cc',
'sstables/sstable_directory.cc',
+ 'transport/cql_protocol_extension.cc',
'transport/event.cc',
'transport/event_notifier.cc',
'transport/server.cc',

diff --git a/cql3/result_set.hh b/cql3/result_set.hh
--- a/cql3/result_set.hh
+++ b/cql3/result_set.hh

[Commit Bot]

From: Pavel Solodovnikov <pa.solo...@scylladb.com>
Committer: Pavel Solodovnikov <pa.solo...@scylladb.com>
Branch: master

lwt: introduce "LWT" flag in prepared statement metadata

This patch adds a new `LWT` flag to `cql3::prepared_metadata`.

That allows clients to clearly distinguish betwen lwt and
non-lwt statements without need to execute some custom parsing
logic (e.g. parsing the prepared query with regular expressions),
which is obviously quite fragile.

The feature is meant to be further utilized by client drivers

to use primary replicas consistently when dealing with conditional
statements.

Whether to use lwt optimization flag or not is handled by negotiation
procedure between scylla server and client library via SUPPORTED/STARTUP
messages (`LWT_ADD_METADATA_MARK` extension).

Tests: unit(dev, debug), manual testing with modified scylla/gocql driver

Signed-off-by: Pavel Solodovnikov <pa.solo...@scylladb.com>

---

diff --git a/cql3/result_set.hh b/cql3/result_set.hh
--- a/cql3/result_set.hh
+++ b/cql3/result_set.hh