From 762eec2bc687ce950c50c8bdcc8e7f5fb6316817 Mon Sep 17 00:00:00 2001 From: Tomasz Grabiec Date: Wed, 23 Oct 2019 18:23:26 +0200 Subject: [PATCH] Merge "Fix TTL serialization breakage" from Avi ommit 93270dd changed gc_clock to be 64-bit, to fix the Y2038 problem. While 64-bit tombstone::deletion_time is serialized in a compatible way, TTLs (gc_clock::duration) were not. This patchset reverts TTL serialization to the 32-bit serialization format, and also allows opting-in to the 64-bit format in case a cluster was installed with the broken code. Only Scylla 3.1.0 is vulnerable. Fixes #4855 Tests: unit (dev) (cherry picked from commit e621db591e41b9767468cb196ac4fc3ecbd4220c) --- db/config.hh | 1 + gc_clock.hh | 34 ++++++++++++++++++++++++++++++++++ main.cc | 6 ++++++ serializer.hh | 32 ++++++++++++++++++++++++++++++++ 4 files changed, 73 insertions(+) diff --git a/db/config.hh b/db/config.hh index 09e8f69c4d..d8d5fa2c45 100644 --- a/db/config.hh +++ b/db/config.hh @@ -757,6 +757,7 @@ public: " It is not enough to have ever since upgraded to newer versions of Cassandra. If you EVER used a version earlier than 2.1 in the cluster where these SSTables come from, DO NOT TURN ON THIS OPTION! You will corrupt your data. You have been warned.") \ val(enable_shard_aware_drivers, bool, true, Used, "Enable native transport drivers to use connection-per-shard for better performance") \ val(abort_on_internal_error, bool, false, Used, "Abort the server instead of throwing exception when internal invariants are violated.") \ + val(enable_3_1_0_compatibility_mode, bool, false, Used, "Set to true if the cluster was initially installed from 3.1.0. If it was upgraded from an earlier version, or installed from a later version, leave this set to false. This adjusts the communication protocol to work around a bug in Scylla 3.1.0") \ /* done! */ #define _make_value_member(name, type, deflt, status, desc, ...) \ diff --git a/gc_clock.hh b/gc_clock.hh index ac3cbeb958..46d01d226c 100644 --- a/gc_clock.hh +++ b/gc_clock.hh @@ -86,3 +86,37 @@ struct appending_hash { } } }; + + +namespace ser { + +// Forward-declaration - defined in serializer.hh, to avoid including it here. + +template +void serialize_gc_clock_duration_value(Output& out, int64_t value); + +template +int64_t deserialize_gc_clock_duration_value(Input& in); + +template +struct serializer; + +template <> +struct serializer { + template + static gc_clock::duration read(Input& in) { + return gc_clock::duration(deserialize_gc_clock_duration_value(in)); + } + + template + static void write(Output& out, gc_clock::duration d) { + serialize_gc_clock_duration_value(out, d.count()); + } + + template + static void skip(Input& in) { + read(in); + } +}; + +} diff --git a/main.cc b/main.cc index 8a6d4fdb7e..80fdb324b4 100644 --- a/main.cc +++ b/main.cc @@ -69,6 +69,7 @@ #include "sstables/sstables.hh" #include "gms/feature_service.hh" #include "distributed_loader.hh" +#include "serializer.hh" namespace fs = std::filesystem; @@ -408,6 +409,11 @@ int main(int ac, char** av) { read_config(opts, *cfg).get(); configurable::init_all(opts, *cfg, *ext).get(); + // We're writing to a non-atomic variable here. But bool writes are atomic + // in all supported architectures, and some broadcast or other below + // will apply the required memory barriers anyway. + ser::gc_clock_using_3_1_0_serialization = cfg->enable_3_1_0_compatibility_mode(); + logalloc::prime_segment_pool(memory::stats().total_memory(), memory::min_free_memory()).get(); logging::apply_settings(cfg->logging_settings(opts)); diff --git a/serializer.hh b/serializer.hh index e55b45e803..058048808d 100644 --- a/serializer.hh +++ b/serializer.hh @@ -282,6 +282,38 @@ struct normalize { template struct is_equivalent : std::is_same>>::type, typename normalize>>::type> { }; + +// gc_clock duration values were serialized as 32-bit prior to 3.1, and +// are serialized as 64-bit in 3.1.0. +// +// TTL values are capped to 20 years, which fits into 32 bits, so +// truncation is not a concern. + +inline bool gc_clock_using_3_1_0_serialization = false; + +template +void +serialize_gc_clock_duration_value(Output& out, int64_t v) { + if (!gc_clock_using_3_1_0_serialization) { + // This should have been caught by the CQL layer, so this is just + // for extra safety. + assert(int32_t(v) == v); + serializer::write(out, v); + } else { + serializer::write(out, v); + } +} + +template +int64_t +deserialize_gc_clock_duration_value(Input& in) { + if (!gc_clock_using_3_1_0_serialization) { + return serializer::read(in); + } else { + return serializer::read(in); + } +} + } /*