diff --git a/compaction/compaction_strategy.cc b/compaction/compaction_strategy.cc index f6963e08fc6b..3793a866790b 100644 --- a/compaction/compaction_strategy.cc +++ b/compaction/compaction_strategy.cc @@ -87,17 +87,96 @@ std::optional compaction_strategy_impl::get_value(const std::mapsecond; } -compaction_strategy_impl::compaction_strategy_impl(const std::map& options) { - using namespace cql3::statements; +void compaction_strategy_impl::validate_min_max_threshold(const std::map& options, std::map& unchecked_options) { + auto min_threshold_key = "min_threshold", max_threshold_key = "max_threshold"; + + auto tmp_value = compaction_strategy_impl::get_value(options, min_threshold_key); + auto min_threshold = cql3::statements::property_definitions::to_long(min_threshold_key, tmp_value, DEFAULT_MIN_COMPACTION_THRESHOLD); + if (min_threshold < 2) { + throw exceptions::configuration_exception(fmt::format("{} value ({}) must be bigger or equal to 2", min_threshold_key, min_threshold)); + } + + tmp_value = compaction_strategy_impl::get_value(options, max_threshold_key); + auto max_threshold = cql3::statements::property_definitions::to_long(max_threshold_key, tmp_value, DEFAULT_MAX_COMPACTION_THRESHOLD); + if (max_threshold < 2) { + throw exceptions::configuration_exception(fmt::format("{} value ({}) must be bigger or equal to 2", max_threshold_key, max_threshold)); + } + + unchecked_options.erase(min_threshold_key); + unchecked_options.erase(max_threshold_key); +} + +static double validate_tombstone_threshold(const std::map& options) { + auto tmp_value = compaction_strategy_impl::get_value(options, compaction_strategy_impl::TOMBSTONE_THRESHOLD_OPTION); + auto tombstone_threshold = cql3::statements::property_definitions::to_double(compaction_strategy_impl::TOMBSTONE_THRESHOLD_OPTION, tmp_value, compaction_strategy_impl::DEFAULT_TOMBSTONE_THRESHOLD); + if (tombstone_threshold < 0.0 || tombstone_threshold > 1.0) { + throw exceptions::configuration_exception(fmt::format("{} value ({}) must be between 0.0 and 1.0", compaction_strategy_impl::TOMBSTONE_THRESHOLD_OPTION, tombstone_threshold)); + } + return tombstone_threshold; +} + +static double validate_tombstone_threshold(const std::map& options, std::map& unchecked_options) { + auto tombstone_threshold = validate_tombstone_threshold(options); + unchecked_options.erase(compaction_strategy_impl::TOMBSTONE_THRESHOLD_OPTION); + return tombstone_threshold; +} + +static db_clock::duration validate_tombstone_compaction_interval(const std::map& options) { + auto tmp_value = compaction_strategy_impl::get_value(options, compaction_strategy_impl::TOMBSTONE_COMPACTION_INTERVAL_OPTION); + auto interval = cql3::statements::property_definitions::to_long(compaction_strategy_impl::TOMBSTONE_COMPACTION_INTERVAL_OPTION, tmp_value, compaction_strategy_impl::DEFAULT_TOMBSTONE_COMPACTION_INTERVAL().count()); + auto tombstone_compaction_interval = db_clock::duration(std::chrono::seconds(interval)); + if (interval <= 0) { + throw exceptions::configuration_exception(fmt::format("{} value ({}) must be positive", compaction_strategy_impl::TOMBSTONE_COMPACTION_INTERVAL_OPTION, tombstone_compaction_interval)); + } + return tombstone_compaction_interval; +} + +static db_clock::duration validate_tombstone_compaction_interval(const std::map& options, std::map& unchecked_options) { + auto tombstone_compaction_interval = validate_tombstone_compaction_interval(options); + unchecked_options.erase(compaction_strategy_impl::TOMBSTONE_COMPACTION_INTERVAL_OPTION); + return tombstone_compaction_interval; +} + +void compaction_strategy_impl::validate_options_for_strategy_type(const std::map& options, sstables::compaction_strategy_type type) { + auto unchecked_options = options; + compaction_strategy_impl::validate_options(options, unchecked_options); + switch (type) { + case compaction_strategy_type::size_tiered: + size_tiered_compaction_strategy::validate_options(options, unchecked_options); + break; + case compaction_strategy_type::leveled: + leveled_compaction_strategy::validate_options(options, unchecked_options); + break; + case compaction_strategy_type::time_window: + time_window_compaction_strategy::validate_options(options, unchecked_options); + break; + default: + break; + } - auto tmp_value = get_value(options, TOMBSTONE_THRESHOLD_OPTION); - _tombstone_threshold = property_definitions::to_double(TOMBSTONE_THRESHOLD_OPTION, tmp_value, DEFAULT_TOMBSTONE_THRESHOLD); + unchecked_options.erase("class"); + if (!unchecked_options.empty()) { + throw exceptions::configuration_exception(fmt::format("Invalid compaction strategy options {} for chosen strategy type", unchecked_options)); + } +} - tmp_value = get_value(options, TOMBSTONE_COMPACTION_INTERVAL_OPTION); - auto interval = property_definitions::to_long(TOMBSTONE_COMPACTION_INTERVAL_OPTION, tmp_value, DEFAULT_TOMBSTONE_COMPACTION_INTERVAL().count()); - _tombstone_compaction_interval = db_clock::duration(std::chrono::seconds(interval)); +// options is a map of compaction strategy options and their values. +// unchecked_options is an analogical map from which already checked options are deleted. +// This helps making sure that only allowed options are being set. +void compaction_strategy_impl::validate_options(const std::map& options, std::map& unchecked_options) { + validate_tombstone_threshold(options, unchecked_options); + validate_tombstone_compaction_interval(options, unchecked_options); - // FIXME: validate options. + auto it = options.find("enabled"); + if (it != options.end() && it->second != "true" && it->second != "false") { + throw exceptions::configuration_exception(fmt::format("enabled value ({}) must be \"true\" or \"false\"", it->second)); + } + unchecked_options.erase("enabled"); +} + +compaction_strategy_impl::compaction_strategy_impl(const std::map& options) { + _tombstone_threshold = validate_tombstone_threshold(options); + _tombstone_compaction_interval = validate_tombstone_compaction_interval(options); } } // namespace sstables @@ -493,6 +572,20 @@ leveled_compaction_strategy::leveled_compaction_strategy(const std::map& options, std::map& unchecked_options) { + size_tiered_compaction_strategy_options::validate(options, unchecked_options); + + auto tmp_value = compaction_strategy_impl::get_value(options, SSTABLE_SIZE_OPTION); + auto min_sstables_size = cql3::statements::property_definitions::to_long(SSTABLE_SIZE_OPTION, tmp_value, DEFAULT_MAX_SSTABLE_SIZE_IN_MB); + if (min_sstables_size <= 0) { + throw exceptions::configuration_exception(fmt::format("{} value ({}) must be positive", SSTABLE_SIZE_OPTION, min_sstables_size)); + } + unchecked_options.erase(SSTABLE_SIZE_OPTION); +} + std::unique_ptr leveled_compaction_strategy::make_backlog_tracker() const { return std::make_unique(_max_sstable_size_in_mb, _stcs_options); } @@ -526,6 +619,14 @@ time_window_compaction_strategy::time_window_compaction_strategy(const std::map< _use_clustering_key_filter = true; } +// options is a map of compaction strategy options and their values. +// unchecked_options is an analogical map from which already checked options are deleted. +// This helps making sure that only allowed options are being set. +void time_window_compaction_strategy::validate_options(const std::map& options, std::map& unchecked_options) { + time_window_compaction_strategy_options::validate(options, unchecked_options); + size_tiered_compaction_strategy_options::validate(options, unchecked_options); +} + std::unique_ptr time_window_compaction_strategy::make_backlog_tracker() const { return std::make_unique(_options, _stcs_options); } @@ -543,6 +644,13 @@ size_tiered_compaction_strategy::size_tiered_compaction_strategy(const size_tier : _options(options) {} +// options is a map of compaction strategy options and their values. +// unchecked_options is an analogical map from which already checked options are deleted. +// This helps making sure that only allowed options are being set. +void size_tiered_compaction_strategy::validate_options(const std::map& options, std::map& unchecked_options) { + size_tiered_compaction_strategy_options::validate(options, unchecked_options); +} + std::unique_ptr size_tiered_compaction_strategy::make_backlog_tracker() const { return std::make_unique(_options); } diff --git a/compaction/compaction_strategy_impl.hh b/compaction/compaction_strategy_impl.hh index eed3173a34c2..24c21d5f8f78 100644 --- a/compaction/compaction_strategy_impl.hh +++ b/compaction/compaction_strategy_impl.hh @@ -21,20 +21,23 @@ class sstable_set_impl; class resharding_descriptor; class compaction_strategy_impl { +public: static constexpr float DEFAULT_TOMBSTONE_THRESHOLD = 0.2f; // minimum interval needed to perform tombstone removal compaction in seconds, default 86400 or 1 day. static constexpr std::chrono::seconds DEFAULT_TOMBSTONE_COMPACTION_INTERVAL() { return std::chrono::seconds(86400); } + static constexpr auto TOMBSTONE_THRESHOLD_OPTION = "tombstone_threshold"; + static constexpr auto TOMBSTONE_COMPACTION_INTERVAL_OPTION = "tombstone_compaction_interval"; protected: - const sstring TOMBSTONE_THRESHOLD_OPTION = "tombstone_threshold"; - const sstring TOMBSTONE_COMPACTION_INTERVAL_OPTION = "tombstone_compaction_interval"; - bool _use_clustering_key_filter = false; bool _disable_tombstone_compaction = false; float _tombstone_threshold = DEFAULT_TOMBSTONE_THRESHOLD; db_clock::duration _tombstone_compaction_interval = DEFAULT_TOMBSTONE_COMPACTION_INTERVAL(); public: static std::optional get_value(const std::map& options, const sstring& name); + static void validate_min_max_threshold(const std::map& options, std::map& unchecked_options); + static void validate_options_for_strategy_type(const std::map& options, sstables::compaction_strategy_type type); protected: + static void validate_options(const std::map& options, std::map& unchecked_options); compaction_strategy_impl() = default; explicit compaction_strategy_impl(const std::map& options); static compaction_descriptor make_major_compaction_job(std::vector candidates, diff --git a/compaction/leveled_compaction_strategy.hh b/compaction/leveled_compaction_strategy.hh index fea2ce297c49..2ca57e826cfa 100644 --- a/compaction/leveled_compaction_strategy.hh +++ b/compaction/leveled_compaction_strategy.hh @@ -36,7 +36,7 @@ struct leveled_compaction_strategy_state { class leveled_compaction_strategy : public compaction_strategy_impl { static constexpr int32_t DEFAULT_MAX_SSTABLE_SIZE_IN_MB = 160; - const sstring SSTABLE_SIZE_OPTION = "sstable_size_in_mb"; + static constexpr auto SSTABLE_SIZE_OPTION = "sstable_size_in_mb"; int32_t _max_sstable_size_in_mb = DEFAULT_MAX_SSTABLE_SIZE_IN_MB; size_tiered_compaction_strategy_options _stcs_options; @@ -46,6 +46,7 @@ private: leveled_compaction_strategy_state& get_state(table_state& table_s) const; public: static unsigned ideal_level_for_input(const std::vector& input, uint64_t max_sstable_size); + static void validate_options(const std::map& options, std::map& unchecked_options); leveled_compaction_strategy(const std::map& options); virtual compaction_descriptor get_sstables_for_compaction(table_state& table_s, strategy_control& control, std::vector candidates) override; diff --git a/compaction/size_tiered_compaction_strategy.cc b/compaction/size_tiered_compaction_strategy.cc index 3bce7e8dc0d6..d06563bf91e2 100644 --- a/compaction/size_tiered_compaction_strategy.cc +++ b/compaction/size_tiered_compaction_strategy.cc @@ -15,20 +15,73 @@ namespace sstables { -size_tiered_compaction_strategy_options::size_tiered_compaction_strategy_options(const std::map& options) { - using namespace cql3::statements; +static long validate_sstable_size(const std::map& options) { + auto tmp_value = compaction_strategy_impl::get_value(options, size_tiered_compaction_strategy_options::MIN_SSTABLE_SIZE_KEY); + auto min_sstables_size = cql3::statements::property_definitions::to_long(size_tiered_compaction_strategy_options::MIN_SSTABLE_SIZE_KEY, tmp_value, size_tiered_compaction_strategy_options::DEFAULT_MIN_SSTABLE_SIZE); + if (min_sstables_size < 0) { + throw exceptions::configuration_exception(fmt::format("{} value ({}) must be non negative", size_tiered_compaction_strategy_options::MIN_SSTABLE_SIZE_KEY, min_sstables_size)); + } + return min_sstables_size; +} + +static long validate_sstable_size(const std::map& options, std::map& unchecked_options) { + auto min_sstables_size = validate_sstable_size(options); + unchecked_options.erase(size_tiered_compaction_strategy_options::MIN_SSTABLE_SIZE_KEY); + return min_sstables_size; +} + +static double validate_bucket_low(const std::map& options) { + auto tmp_value = compaction_strategy_impl::get_value(options, size_tiered_compaction_strategy_options::BUCKET_LOW_KEY); + auto bucket_low = cql3::statements::property_definitions::to_double(size_tiered_compaction_strategy_options::BUCKET_LOW_KEY, tmp_value, size_tiered_compaction_strategy_options::DEFAULT_BUCKET_LOW); + if (bucket_low <= 0.0 || bucket_low >= 1.0) { + throw exceptions::configuration_exception(fmt::format("{} value ({}) must be between 0.0 and 1.0", size_tiered_compaction_strategy_options::BUCKET_LOW_KEY, bucket_low)); + } + return bucket_low; +} + +static double validate_bucket_low(const std::map& options, std::map& unchecked_options) { + auto bucket_low = validate_bucket_low(options); + unchecked_options.erase(size_tiered_compaction_strategy_options::BUCKET_LOW_KEY); + return bucket_low; +} + +static double validate_bucket_high(const std::map& options) { + auto tmp_value = compaction_strategy_impl::get_value(options, size_tiered_compaction_strategy_options::BUCKET_HIGH_KEY); + auto bucket_high = cql3::statements::property_definitions::to_double(size_tiered_compaction_strategy_options::BUCKET_HIGH_KEY, tmp_value, size_tiered_compaction_strategy_options::DEFAULT_BUCKET_HIGH); + if (bucket_high <= 1.0) { + throw exceptions::configuration_exception(fmt::format("{} value ({}) must be greater than 1.0", size_tiered_compaction_strategy_options::BUCKET_HIGH_KEY, bucket_high)); + } + return bucket_high; +} + +static double validate_bucket_high(const std::map& options, std::map& unchecked_options) { + auto bucket_high = validate_bucket_high(options); + unchecked_options.erase(size_tiered_compaction_strategy_options::BUCKET_HIGH_KEY); + return bucket_high; +} - auto tmp_value = compaction_strategy_impl::get_value(options, MIN_SSTABLE_SIZE_KEY); - min_sstable_size = property_definitions::to_long(MIN_SSTABLE_SIZE_KEY, tmp_value, DEFAULT_MIN_SSTABLE_SIZE); +static double validate_cold_reads_to_omit(const std::map& options) { + auto tmp_value = compaction_strategy_impl::get_value(options, size_tiered_compaction_strategy_options::COLD_READS_TO_OMIT_KEY); + auto cold_reads_to_omit = cql3::statements::property_definitions::to_double(size_tiered_compaction_strategy_options::COLD_READS_TO_OMIT_KEY, tmp_value, size_tiered_compaction_strategy_options::DEFAULT_COLD_READS_TO_OMIT); + if (cold_reads_to_omit < 0.0 || cold_reads_to_omit > 1.0) { + throw exceptions::configuration_exception(fmt::format("{} value ({}) must be between 0.0 and 1.0", size_tiered_compaction_strategy_options::COLD_READS_TO_OMIT_KEY, cold_reads_to_omit)); + } + return cold_reads_to_omit; +} - tmp_value = compaction_strategy_impl::get_value(options, BUCKET_LOW_KEY); - bucket_low = property_definitions::to_double(BUCKET_LOW_KEY, tmp_value, DEFAULT_BUCKET_LOW); +static double validate_cold_reads_to_omit(const std::map& options, std::map& unchecked_options) { + auto cold_reads_to_omit = validate_cold_reads_to_omit(options); + unchecked_options.erase(size_tiered_compaction_strategy_options::COLD_READS_TO_OMIT_KEY); + return cold_reads_to_omit; +} - tmp_value = compaction_strategy_impl::get_value(options, BUCKET_HIGH_KEY); - bucket_high = property_definitions::to_double(BUCKET_HIGH_KEY, tmp_value, DEFAULT_BUCKET_HIGH); +size_tiered_compaction_strategy_options::size_tiered_compaction_strategy_options(const std::map& options) { + using namespace cql3::statements; - tmp_value = compaction_strategy_impl::get_value(options, COLD_READS_TO_OMIT_KEY); - cold_reads_to_omit = property_definitions::to_double(COLD_READS_TO_OMIT_KEY, tmp_value, DEFAULT_COLD_READS_TO_OMIT); + min_sstable_size = validate_sstable_size(options); + bucket_low = validate_bucket_low(options); + bucket_high = validate_bucket_high(options); + cold_reads_to_omit = validate_cold_reads_to_omit(options); } size_tiered_compaction_strategy_options::size_tiered_compaction_strategy_options() { @@ -38,6 +91,20 @@ size_tiered_compaction_strategy_options::size_tiered_compaction_strategy_options cold_reads_to_omit = DEFAULT_COLD_READS_TO_OMIT; } +// options is a map of compaction strategy options and their values. +// unchecked_options is an analogical map from which already checked options are deleted. +// This helps making sure that only allowed options are being set. +void size_tiered_compaction_strategy_options::validate(const std::map& options, std::map& unchecked_options) { + validate_sstable_size(options, unchecked_options); + auto bucket_low = validate_bucket_low(options, unchecked_options); + auto bucket_high = validate_bucket_high(options, unchecked_options); + if (bucket_high <= bucket_low) { + throw exceptions::configuration_exception(fmt::format("{} value ({}) is less than or equal to the {} value ({})", BUCKET_HIGH_KEY, bucket_high, BUCKET_LOW_KEY, bucket_low)); + } + validate_cold_reads_to_omit(options, unchecked_options); + compaction_strategy_impl::validate_min_max_threshold(options, unchecked_options); +} + std::vector> size_tiered_compaction_strategy::create_sstable_and_length_pairs(const std::vector& sstables) { diff --git a/compaction/size_tiered_compaction_strategy.hh b/compaction/size_tiered_compaction_strategy.hh index dec492aea3b4..eb9ccef18903 100644 --- a/compaction/size_tiered_compaction_strategy.hh +++ b/compaction/size_tiered_compaction_strategy.hh @@ -18,15 +18,16 @@ class size_tiered_backlog_tracker; namespace sstables { class size_tiered_compaction_strategy_options { +public: static constexpr uint64_t DEFAULT_MIN_SSTABLE_SIZE = 50L * 1024L * 1024L; static constexpr double DEFAULT_BUCKET_LOW = 0.5; static constexpr double DEFAULT_BUCKET_HIGH = 1.5; static constexpr double DEFAULT_COLD_READS_TO_OMIT = 0.05; - const sstring MIN_SSTABLE_SIZE_KEY = "min_sstable_size"; - const sstring BUCKET_LOW_KEY = "bucket_low"; - const sstring BUCKET_HIGH_KEY = "bucket_high"; - const sstring COLD_READS_TO_OMIT_KEY = "cold_reads_to_omit"; - + static constexpr auto MIN_SSTABLE_SIZE_KEY = "min_sstable_size"; + static constexpr auto BUCKET_LOW_KEY = "bucket_low"; + static constexpr auto BUCKET_HIGH_KEY = "bucket_high"; + static constexpr auto COLD_READS_TO_OMIT_KEY = "cold_reads_to_omit"; +private: uint64_t min_sstable_size = DEFAULT_MIN_SSTABLE_SIZE; double bucket_low = DEFAULT_BUCKET_LOW; double bucket_high = DEFAULT_BUCKET_HIGH; @@ -35,48 +36,13 @@ public: size_tiered_compaction_strategy_options(const std::map& options); size_tiered_compaction_strategy_options(); + size_tiered_compaction_strategy_options(const size_tiered_compaction_strategy_options&) = default; + size_tiered_compaction_strategy_options(size_tiered_compaction_strategy_options&&) = default; + size_tiered_compaction_strategy_options& operator=(const size_tiered_compaction_strategy_options&) = default; + size_tiered_compaction_strategy_options& operator=(size_tiered_compaction_strategy_options&&) = default; + + static void validate(const std::map& options, std::map& unchecked_options); - // FIXME: convert java code below. -#if 0 - public static Map validateOptions(Map options, Map uncheckedOptions) throws ConfigurationException - { - String optionValue = options.get(MIN_SSTABLE_SIZE_KEY); - try - { - long minSSTableSize = optionValue == null ? DEFAULT_MIN_SSTABLE_SIZE : Long.parseLong(optionValue); - if (minSSTableSize < 0) - { - throw new ConfigurationException(String.format("%s must be non negative: %d", MIN_SSTABLE_SIZE_KEY, minSSTableSize)); - } - } - catch (NumberFormatException e) - { - throw new ConfigurationException(String.format("%s is not a parsable int (base10) for %s", optionValue, MIN_SSTABLE_SIZE_KEY), e); - } - - double bucketLow = parseDouble(options, BUCKET_LOW_KEY, DEFAULT_BUCKET_LOW); - double bucketHigh = parseDouble(options, BUCKET_HIGH_KEY, DEFAULT_BUCKET_HIGH); - if (bucketHigh <= bucketLow) - { - throw new ConfigurationException(String.format("%s value (%s) is less than or equal to the %s value (%s)", - BUCKET_HIGH_KEY, bucketHigh, BUCKET_LOW_KEY, bucketLow)); - } - - double maxColdReadsRatio = parseDouble(options, COLD_READS_TO_OMIT_KEY, DEFAULT_COLD_READS_TO_OMIT); - if (maxColdReadsRatio < 0.0 || maxColdReadsRatio > 1.0) - { - throw new ConfigurationException(String.format("%s value (%s) should be between between 0.0 and 1.0", - COLD_READS_TO_OMIT_KEY, optionValue)); - } - - uncheckedOptions.remove(MIN_SSTABLE_SIZE_KEY); - uncheckedOptions.remove(BUCKET_LOW_KEY); - uncheckedOptions.remove(BUCKET_HIGH_KEY); - uncheckedOptions.remove(COLD_READS_TO_OMIT_KEY); - - return uncheckedOptions; - } -#endif friend class size_tiered_compaction_strategy; }; @@ -109,6 +75,7 @@ public: size_tiered_compaction_strategy(const std::map& options); explicit size_tiered_compaction_strategy(const size_tiered_compaction_strategy_options& options); + static void validate_options(const std::map& options, std::map& unchecked_options); virtual compaction_descriptor get_sstables_for_compaction(table_state& table_s, strategy_control& control, std::vector candidates) override; diff --git a/compaction/time_window_compaction_strategy.cc b/compaction/time_window_compaction_strategy.cc index 1560f43a1f84..7fa35a5ec625 100644 --- a/compaction/time_window_compaction_strategy.cc +++ b/compaction/time_window_compaction_strategy.cc @@ -26,53 +26,104 @@ time_window_compaction_strategy_state& time_window_compaction_strategy::get_stat return table_s.get_compaction_strategy_state().get(); } -time_window_compaction_strategy_options::time_window_compaction_strategy_options(const std::map& options) { - std::chrono::seconds window_unit = DEFAULT_COMPACTION_WINDOW_UNIT; - int window_size = DEFAULT_COMPACTION_WINDOW_SIZE; - - auto it = options.find(COMPACTION_WINDOW_UNIT_KEY); - if (it != options.end()) { - auto valid_window_units_it = valid_window_units.find(it->second); - if (valid_window_units_it == valid_window_units.end()) { - throw exceptions::syntax_exception(sstring("Invalid window unit ") + it->second + " for " + COMPACTION_WINDOW_UNIT_KEY); +const std::unordered_map time_window_compaction_strategy_options::valid_window_units = { + { "MINUTES", 60s }, { "HOURS", 3600s }, { "DAYS", 86400s } +}; + +const std::unordered_map time_window_compaction_strategy_options::valid_timestamp_resolutions = { + { "MICROSECONDS", timestamp_resolutions::microsecond }, + { "MILLISECONDS", timestamp_resolutions::millisecond }, +}; + +static std::chrono::seconds validate_compaction_window_unit(const std::map& options) { + std::chrono::seconds window_unit = time_window_compaction_strategy_options::DEFAULT_COMPACTION_WINDOW_UNIT; + + auto tmp_value = compaction_strategy_impl::get_value(options, time_window_compaction_strategy_options::COMPACTION_WINDOW_UNIT_KEY); + if (tmp_value) { + auto valid_window_units_it = time_window_compaction_strategy_options::valid_window_units.find(tmp_value.value()); + if (valid_window_units_it == time_window_compaction_strategy_options::valid_window_units.end()) { + throw exceptions::configuration_exception(fmt::format("Invalid window unit {} for {}", tmp_value.value(), time_window_compaction_strategy_options::COMPACTION_WINDOW_UNIT_KEY)); } window_unit = valid_window_units_it->second; } - it = options.find(COMPACTION_WINDOW_SIZE_KEY); - if (it != options.end()) { - try { - window_size = std::stoi(it->second); - } catch (const std::exception& e) { - throw exceptions::syntax_exception(sstring("Invalid integer value ") + it->second + " for " + COMPACTION_WINDOW_SIZE_KEY); - } - } + return window_unit; +} + +static std::chrono::seconds validate_compaction_window_unit(const std::map& options, std::map& unchecked_options) { + auto window_unit = validate_compaction_window_unit(options); + unchecked_options.erase(time_window_compaction_strategy_options::COMPACTION_WINDOW_UNIT_KEY); + return window_unit; +} + +static int validate_compaction_window_size(const std::map& options) { + auto tmp_value = compaction_strategy_impl::get_value(options, time_window_compaction_strategy_options::COMPACTION_WINDOW_SIZE_KEY); + int window_size = cql3::statements::property_definitions::to_long(time_window_compaction_strategy_options::COMPACTION_WINDOW_SIZE_KEY, tmp_value, time_window_compaction_strategy_options::DEFAULT_COMPACTION_WINDOW_SIZE); if (window_size <= 0) { - throw exceptions::configuration_exception(fmt::format("{} must be greater than 1 for compaction_window_size", window_size)); + throw exceptions::configuration_exception(fmt::format("{} value ({}) must be greater than 1", time_window_compaction_strategy_options::COMPACTION_WINDOW_SIZE_KEY, window_size)); } - sstable_window_size = window_size * window_unit; + return window_size; +} + +static int validate_compaction_window_size(const std::map& options, std::map& unchecked_options) { + int window_size = validate_compaction_window_size(options); + unchecked_options.erase(time_window_compaction_strategy_options::COMPACTION_WINDOW_SIZE_KEY); + return window_size; +} + +static db_clock::duration validate_expired_sstable_check_frequency_seconds(const std::map& options) { + db_clock::duration expired_sstable_check_frequency = time_window_compaction_strategy_options::DEFAULT_EXPIRED_SSTABLE_CHECK_FREQUENCY_SECONDS(); - it = options.find(EXPIRED_SSTABLE_CHECK_FREQUENCY_SECONDS_KEY); - if (it != options.end()) { + auto tmp_value = compaction_strategy_impl::get_value(options, time_window_compaction_strategy_options::EXPIRED_SSTABLE_CHECK_FREQUENCY_SECONDS_KEY); + if (tmp_value) { try { - expired_sstable_check_frequency = std::chrono::seconds(std::stol(it->second)); + expired_sstable_check_frequency = std::chrono::seconds(std::stol(tmp_value.value())); } catch (const std::exception& e) { - throw exceptions::syntax_exception(sstring("Invalid long value ") + it->second + "for " + EXPIRED_SSTABLE_CHECK_FREQUENCY_SECONDS_KEY); + throw exceptions::syntax_exception(fmt::format("Invalid long value {} for {}", tmp_value.value(), time_window_compaction_strategy_options::EXPIRED_SSTABLE_CHECK_FREQUENCY_SECONDS_KEY)); } } - it = options.find(TIMESTAMP_RESOLUTION_KEY); - if (it != options.end()) { - if (!valid_timestamp_resolutions.contains(it->second)) { - throw exceptions::syntax_exception(sstring("Invalid timestamp resolution ") + it->second + "for " + TIMESTAMP_RESOLUTION_KEY); + return expired_sstable_check_frequency; +} + +static db_clock::duration validate_expired_sstable_check_frequency_seconds(const std::map& options, std::map& unchecked_options) { + db_clock::duration expired_sstable_check_frequency = validate_expired_sstable_check_frequency_seconds(options); + unchecked_options.erase(time_window_compaction_strategy_options::EXPIRED_SSTABLE_CHECK_FREQUENCY_SECONDS_KEY); + return expired_sstable_check_frequency; +} + +static time_window_compaction_strategy_options::timestamp_resolutions validate_timestamp_resolution(const std::map& options) { + time_window_compaction_strategy_options::timestamp_resolutions timestamp_resolution = time_window_compaction_strategy_options::timestamp_resolutions::microsecond; + + auto tmp_value = compaction_strategy_impl::get_value(options, time_window_compaction_strategy_options::TIMESTAMP_RESOLUTION_KEY); + if (tmp_value) { + if (!time_window_compaction_strategy_options::valid_timestamp_resolutions.contains(tmp_value.value())) { + throw exceptions::configuration_exception(fmt::format("Invalid timestamp resolution {} for {}", tmp_value.value(), time_window_compaction_strategy_options::TIMESTAMP_RESOLUTION_KEY)); } else { - timestamp_resolution = valid_timestamp_resolutions.at(it->second); + timestamp_resolution = time_window_compaction_strategy_options::valid_timestamp_resolutions.at(tmp_value.value()); } } - it = options.find("enable_optimized_twcs_queries"); + return timestamp_resolution; +} + +static time_window_compaction_strategy_options::timestamp_resolutions validate_timestamp_resolution(const std::map& options, std::map& unchecked_options) { + time_window_compaction_strategy_options::timestamp_resolutions timestamp_resolution = validate_timestamp_resolution(options); + unchecked_options.erase(time_window_compaction_strategy_options::TIMESTAMP_RESOLUTION_KEY); + return timestamp_resolution; +} + +time_window_compaction_strategy_options::time_window_compaction_strategy_options(const std::map& options) { + auto window_unit = validate_compaction_window_unit(options); + int window_size = validate_compaction_window_size(options); + + sstable_window_size = window_size * window_unit; + expired_sstable_check_frequency = validate_expired_sstable_check_frequency_seconds(options); + timestamp_resolution = validate_timestamp_resolution(options); + + auto it = options.find("enable_optimized_twcs_queries"); if (it != options.end() && it->second == "false") { enable_optimized_twcs_queries = false; } @@ -82,6 +133,29 @@ time_window_compaction_strategy_options::time_window_compaction_strategy_options time_window_compaction_strategy_options::time_window_compaction_strategy_options(const time_window_compaction_strategy_options&) = default; +// options is a map of compaction strategy options and their values. +// unchecked_options is an analogical map from which already checked options are deleted. +// This helps making sure that only allowed options are being set. +void time_window_compaction_strategy_options::validate(const std::map& options, std::map& unchecked_options) { + validate_compaction_window_unit(options, unchecked_options); + validate_compaction_window_size(options, unchecked_options); + validate_expired_sstable_check_frequency_seconds(options, unchecked_options); + validate_timestamp_resolution(options, unchecked_options); + compaction_strategy_impl::validate_min_max_threshold(options, unchecked_options); + + auto it = options.find("enable_optimized_twcs_queries"); + if (it != options.end() && it->second != "true" && it->second != "false") { + throw exceptions::configuration_exception(fmt::format("enable_optimized_twcs_queries value ({}) must be \"true\" or \"false\"", it->second)); + } + unchecked_options.erase("enable_optimized_twcs_queries"); + + it = unchecked_options.find("unsafe_aggressive_sstable_expiration"); + if (it != unchecked_options.end()) { + clogger.warn("unsafe_aggressive_sstable_expiration option is not supported for time window compaction strategy"); + unchecked_options.erase(it); + } +} + class classify_by_timestamp { time_window_compaction_strategy_options _options; std::vector _known_windows; diff --git a/compaction/time_window_compaction_strategy.hh b/compaction/time_window_compaction_strategy.hh index 7114d068df42..01d0ac446cc2 100644 --- a/compaction/time_window_compaction_strategy.hh +++ b/compaction/time_window_compaction_strategy.hh @@ -33,18 +33,15 @@ public: static constexpr auto COMPACTION_WINDOW_UNIT_KEY = "compaction_window_unit"; static constexpr auto COMPACTION_WINDOW_SIZE_KEY = "compaction_window_size"; static constexpr auto EXPIRED_SSTABLE_CHECK_FREQUENCY_SECONDS_KEY = "expired_sstable_check_frequency_seconds"; -private: - const std::unordered_map valid_window_units = { { "MINUTES", 60s }, { "HOURS", 3600s }, { "DAYS", 86400s } }; + + static const std::unordered_map valid_window_units; enum class timestamp_resolutions { microsecond, millisecond, }; - const std::unordered_map valid_timestamp_resolutions = { - { "MICROSECONDS", timestamp_resolutions::microsecond }, - { "MILLISECONDS", timestamp_resolutions::millisecond }, - }; - + static const std::unordered_map valid_timestamp_resolutions; +private: std::chrono::seconds sstable_window_size = DEFAULT_COMPACTION_WINDOW_UNIT * DEFAULT_COMPACTION_WINDOW_SIZE; db_clock::duration expired_sstable_check_frequency = DEFAULT_EXPIRED_SSTABLE_CHECK_FREQUENCY_SECONDS(); timestamp_resolutions timestamp_resolution = timestamp_resolutions::microsecond; @@ -54,6 +51,8 @@ public: time_window_compaction_strategy_options(time_window_compaction_strategy_options&&); time_window_compaction_strategy_options(const std::map& options); + static void validate(const std::map& options, std::map& unchecked_options); +public: std::chrono::seconds get_sstable_window_size() const { return sstable_window_size; } friend class time_window_compaction_strategy; @@ -87,6 +86,8 @@ public: virtual compaction_descriptor get_sstables_for_compaction(table_state& table_s, strategy_control& control, std::vector candidates) override; virtual std::vector get_cleanup_compaction_jobs(table_state& table_s, std::vector candidates) const override; + + static void validate_options(const std::map& options, std::map& unchecked_options); private: time_window_compaction_strategy_state& get_state(table_state& table_s) const; diff --git a/cql3/statements/create_table_statement.cc b/cql3/statements/create_table_statement.cc index 4cbb0c5420d2..f0e1525740d9 100644 --- a/cql3/statements/create_table_statement.cc +++ b/cql3/statements/create_table_statement.cc @@ -442,6 +442,10 @@ std::optional check_restricted_table_properties( current_ttl = (*schema)->default_time_to_live(); } + if (strategy) { + sstables::compaction_strategy_impl::validate_options_for_strategy_type(cfprops.get_compaction_type_options(), strategy.value()); + } + // Evaluate whether the strategy to evaluate was explicitly passed auto cs = (strategy) ? strategy : current_strategy; diff --git a/test/cql-pytest/test_compaction_strategy_validation.py b/test/cql-pytest/test_compaction_strategy_validation.py new file mode 100644 index 000000000000..06e6260594f4 --- /dev/null +++ b/test/cql-pytest/test_compaction_strategy_validation.py @@ -0,0 +1,43 @@ +# Copyright 2023-present ScyllaDB +# +# SPDX-License-Identifier: AGPL-3.0-or-later + +############################################################################# +# Tests for compaction strategy validation +############################################################################# + +from cassandra_tests.porting import create_keyspace, create_table, execute, assert_invalid_throw_message, ConfigurationException + +def assert_throws(cql, msg, cmd): + with create_keyspace(cql, "replication={ 'class' : 'SimpleStrategy', 'replication_factor' : 1 }") as ks: + with create_table(cql, ks, "(a int PRIMARY KEY, b int) WITH compaction = { 'class' : 'SizeTieredCompactionStrategy' }") as table: + assert_invalid_throw_message(cql, table, msg, ConfigurationException, cmd) + +def test_common_options(cql): + assert_throws(cql, "tombstone_threshold value (-0.4) must be between 0.0 and 1.0", "ALTER TABLE %s WITH compaction = { 'class' : 'SizeTieredCompactionStrategy', 'tombstone_threshold' : -0.4 }") + assert_throws(cql, "tombstone_threshold value (5.5) must be between 0.0 and 1.0", "ALTER TABLE %s WITH compaction = { 'class' : 'TimeWindowCompactionStrategy', 'tombstone_threshold' : 5.5 }") + assert_throws(cql, "tombstone_compaction_interval value (-7000ms) must be positive", "ALTER TABLE %s WITH compaction = { 'class' : 'LeveledCompactionStrategy', 'tombstone_compaction_interval' : -7 }") + +def test_size_tiered_compaction_strategy_options(cql): + assert_throws(cql, "min_sstable_size value (-1) must be non negative", "ALTER TABLE %s WITH compaction = { 'class' : 'SizeTieredCompactionStrategy', 'min_sstable_size' : -1 }") + assert_throws(cql, "bucket_low value (0) must be between 0.0 and 1.0", "ALTER TABLE %s WITH compaction = { 'class' : 'SizeTieredCompactionStrategy', 'bucket_low' : 0.0 }") + assert_throws(cql, "bucket_low value (1.3) must be between 0.0 and 1.0", "ALTER TABLE %s WITH compaction = { 'class' : 'SizeTieredCompactionStrategy', 'bucket_low' : 1.3 }") + assert_throws(cql, "bucket_high value (0.7) must be greater than 1.0", "ALTER TABLE %s WITH compaction = { 'class' : 'SizeTieredCompactionStrategy', 'bucket_high' : 0.7 }") + assert_throws(cql, "cold_reads_to_omit value (-8.1) must be between 0.0 and 1.0", "ALTER TABLE %s WITH compaction = { 'class' : 'SizeTieredCompactionStrategy', 'cold_reads_to_omit' : -8.1 }") + assert_throws(cql, "cold_reads_to_omit value (3.5) must be between 0.0 and 1.0", "ALTER TABLE %s WITH compaction = { 'class' : 'SizeTieredCompactionStrategy', 'cold_reads_to_omit' : 3.5 }") + assert_throws(cql, "min_threshold value (1) must be bigger or equal to 2", "ALTER TABLE %s WITH compaction = { 'class' : 'SizeTieredCompactionStrategy', 'min_threshold' : 1 }") + +def test_time_window_compaction_strategy_options(cql): + assert_throws(cql, "Invalid window unit SECONDS for compaction_window_unit", "ALTER TABLE %s WITH compaction = { 'class' : 'TimeWindowCompactionStrategy', 'compaction_window_unit' : 'SECONDS' }") + assert_throws(cql, "compaction_window_size value (-8) must be greater than 1", "ALTER TABLE %s WITH compaction = { 'class' : 'TimeWindowCompactionStrategy', 'compaction_window_size' : -8 }") + assert_throws(cql, "Invalid timestamp resolution SECONDS for timestamp_resolution", "ALTER TABLE %s WITH compaction = { 'class' : 'TimeWindowCompactionStrategy', 'timestamp_resolution' : 'SECONDS' }") + assert_throws(cql, "enable_optimized_twcs_queries value (no) must be \"true\" or \"false\"", "ALTER TABLE %s WITH compaction = { 'class' : 'TimeWindowCompactionStrategy', 'enable_optimized_twcs_queries' : 'no' }") + assert_throws(cql, "max_threshold value (1) must be bigger or equal to 2", "ALTER TABLE %s WITH compaction = { 'class' : 'TimeWindowCompactionStrategy', 'max_threshold' : 1 }") + +def test_leveled_compaction_strategy_options(cql): + assert_throws(cql, "sstable_size_in_mb value (-5) must be positive", "ALTER TABLE %s WITH compaction = { 'class' : 'LeveledCompactionStrategy', 'sstable_size_in_mb' : -5 }") + +def test_not_allowed_options(cql): + assert_throws(cql, "Invalid compaction strategy options {{abc, -54.54}} for chosen strategy type", "ALTER TABLE %s WITH compaction = { 'class' : 'SizeTieredCompactionStrategy', 'abc' : -54.54 }") + assert_throws(cql, "Invalid compaction strategy options {{dog, 3}} for chosen strategy type", "ALTER TABLE %s WITH compaction = { 'class' : 'TimeWindowCompactionStrategy', 'dog' : 3 }") + assert_throws(cql, "Invalid compaction strategy options {{compaction_window_size, 4}} for chosen strategy type", "ALTER TABLE %s WITH compaction = { 'class' : 'LeveledCompactionStrategy', 'compaction_window_size' : 4 }")