From 8bfe3ca543dd00dfee853c9fde3d6b2e6d9688e4 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Botond=20D=C3=A9nes?= Date: Fri, 16 Jun 2023 05:14:37 -0400 Subject: [PATCH] query: move max_result_size to query-request.hh It is currently located in query_class_config.hh, which is named after a now defunct struct. This arrangement is unintuitive and there is no upside to it. The main user of max_result_size is query_comand, so colocate it next to the latter. Closes #14268 --- query-request.hh | 93 +++++++++++++++++++- query_class_config.hh | 107 ----------------------- reader_permit.hh | 7 +- test/lib/reader_concurrency_semaphore.hh | 1 - 4 files changed, 98 insertions(+), 110 deletions(-) delete mode 100644 query_class_config.hh diff --git a/query-request.hh b/query-request.hh index e8b510c8e3..5281746c25 100644 --- a/query-request.hh +++ b/query-request.hh @@ -21,7 +21,6 @@ #include "range.hh" #include "tracing/tracing.hh" #include "utils/small_vector.hh" -#include "query_class_config.hh" #include "db/per_partition_rate_limit_info.hh" #include "query_id.hh" #include "utils/UUID.hh" @@ -32,6 +31,13 @@ class position_in_partition_view; class position_in_partition; class partition_slice_builder; +namespace ser { + +template +class serializer; + +}; + namespace query { using column_id_vector = utils::small_vector; @@ -275,6 +281,91 @@ enum class tombstone_limit : uint64_t { max = max_tombstones }; using is_first_page = bool_class; +/* + * This struct is used in two incompatible ways. + * + * SEPARATE_PAGE_SIZE_AND_SAFETY_LIMIT cluster feature determines which way is + * used. + * + * 1. If SEPARATE_PAGE_SIZE_AND_SAFETY_LIMIT is not enabled on the cluster then + * `page_size` field is ignored. Depending on the query type the meaning of + * the remaining two fields is: + * + * a. For unpaged queries or for reverse queries: + * + * * `soft_limit` is used to warn about queries that result exceeds + * this limit. If the limit is exceeded, a warning will be written to + * the log. + * + * * `hard_limit` is used to terminate a query which result exceeds + * this limit. If the limit is exceeded, the operation will end with + * an exception. + * + * b. For all other queries, `soft_limit` == `hard_limit` and their value is + * really a page_size in bytes. If the page is not previously cut by the + * page row limit then reaching the size of `soft_limit`/`hard_limit` + * bytes will cause a page to be finished. + * + * 2. If SEPARATE_PAGE_SIZE_AND_SAFETY_LIMIT is enabled on the cluster then all + * three fields are always set. They are used in different places: + * + * a. `soft_limit` and `hard_limit` are used for unpaged queries and in a + * reversing reader used for reading KA/LA sstables. Their meaning is the + * same as in (1.a) above. + * + * b. all other queries use `page_size` field only and the meaning of the + * field is the same ase in (1.b) above. + * + * Two interpretations of the `max_result_size` struct are not compatible so we + * need to take care of handling a mixed clusters. + * + * As long as SEPARATE_PAGE_SIZE_AND_SAFETY_LIMIT cluster feature is not + * supported by all nodes in the clustser, new nodes will always use the + * interpretation described in the point (1). `soft_limit` and `hard_limit` + * fields will be set appropriately to the query type and `page_size` field + * will be set to 0. Old nodes will ignare `page_size` anyways and new nodes + * will know to ignore it as well when it's set to 0. Old nodes will never set + * `page_size` and that means new nodes will give it a default value of 0 and + * ignore it for messages that miss this field. + * + * Once SEPARATE_PAGE_SIZE_AND_SAFETY_LIMIT cluster feature becomes supported by + * the whole cluster, new nodes will start to set `page_size` to the right value + * according to the interpretation described in the point (2). + * + * For each request, only the coordinator looks at + * SEPARATE_PAGE_SIZE_AND_SAFETY_LIMIT and based on it decides for this request + * whether it will be handled with interpretation (1) or (2). Then all the + * replicas can check the decision based only on the message they receive. + * If page_size is set to 0 or not set at all then the request will be handled + * using the interpretation (1). Otherwise, interpretation (2) will be used. + */ +struct max_result_size { + uint64_t soft_limit; + uint64_t hard_limit; +private: + uint64_t page_size = 0; +public: + + max_result_size() = delete; + explicit max_result_size(uint64_t max_size) : soft_limit(max_size), hard_limit(max_size) { } + explicit max_result_size(uint64_t soft_limit, uint64_t hard_limit) : soft_limit(soft_limit), hard_limit(hard_limit) { } + max_result_size(uint64_t soft_limit, uint64_t hard_limit, uint64_t page_size) + : soft_limit(soft_limit) + , hard_limit(hard_limit) + , page_size(page_size) + { } + uint64_t get_page_size() const { + return page_size == 0 ? hard_limit : page_size; + } + friend bool operator==(const max_result_size&, const max_result_size&); + friend class ser::serializer; +}; + +inline bool operator==(const max_result_size& a, const max_result_size& b) { + return a.soft_limit == b.soft_limit && a.hard_limit == b.hard_limit && a.page_size == b.page_size; +} + + // Full specification of a query to the database. // Intended for passing across replicas. // Can be accessed across cores. diff --git a/query_class_config.hh b/query_class_config.hh deleted file mode 100644 index f59893d148..0000000000 --- a/query_class_config.hh +++ /dev/null @@ -1,107 +0,0 @@ -/* - * Copyright (C) 2020-present ScyllaDB - */ - -/* - * SPDX-License-Identifier: AGPL-3.0-or-later - */ - -#pragma once - -#include - -namespace ser { - -template -class serializer; - -}; - - -namespace query { - -/* - * This struct is used in two incompatible ways. - * - * SEPARATE_PAGE_SIZE_AND_SAFETY_LIMIT cluster feature determines which way is - * used. - * - * 1. If SEPARATE_PAGE_SIZE_AND_SAFETY_LIMIT is not enabled on the cluster then - * `page_size` field is ignored. Depending on the query type the meaning of - * the remaining two fields is: - * - * a. For unpaged queries or for reverse queries: - * - * * `soft_limit` is used to warn about queries that result exceeds - * this limit. If the limit is exceeded, a warning will be written to - * the log. - * - * * `hard_limit` is used to terminate a query which result exceeds - * this limit. If the limit is exceeded, the operation will end with - * an exception. - * - * b. For all other queries, `soft_limit` == `hard_limit` and their value is - * really a page_size in bytes. If the page is not previously cut by the - * page row limit then reaching the size of `soft_limit`/`hard_limit` - * bytes will cause a page to be finished. - * - * 2. If SEPARATE_PAGE_SIZE_AND_SAFETY_LIMIT is enabled on the cluster then all - * three fields are always set. They are used in different places: - * - * a. `soft_limit` and `hard_limit` are used for unpaged queries and in a - * reversing reader used for reading KA/LA sstables. Their meaning is the - * same as in (1.a) above. - * - * b. all other queries use `page_size` field only and the meaning of the - * field is the same ase in (1.b) above. - * - * Two interpretations of the `max_result_size` struct are not compatible so we - * need to take care of handling a mixed clusters. - * - * As long as SEPARATE_PAGE_SIZE_AND_SAFETY_LIMIT cluster feature is not - * supported by all nodes in the clustser, new nodes will always use the - * interpretation described in the point (1). `soft_limit` and `hard_limit` - * fields will be set appropriately to the query type and `page_size` field - * will be set to 0. Old nodes will ignare `page_size` anyways and new nodes - * will know to ignore it as well when it's set to 0. Old nodes will never set - * `page_size` and that means new nodes will give it a default value of 0 and - * ignore it for messages that miss this field. - * - * Once SEPARATE_PAGE_SIZE_AND_SAFETY_LIMIT cluster feature becomes supported by - * the whole cluster, new nodes will start to set `page_size` to the right value - * according to the interpretation described in the point (2). - * - * For each request, only the coordinator looks at - * SEPARATE_PAGE_SIZE_AND_SAFETY_LIMIT and based on it decides for this request - * whether it will be handled with interpretation (1) or (2). Then all the - * replicas can check the decision based only on the message they receive. - * If page_size is set to 0 or not set at all then the request will be handled - * using the interpretation (1). Otherwise, interpretation (2) will be used. - */ -struct max_result_size { - uint64_t soft_limit; - uint64_t hard_limit; -private: - uint64_t page_size = 0; -public: - - max_result_size() = delete; - explicit max_result_size(uint64_t max_size) : soft_limit(max_size), hard_limit(max_size) { } - explicit max_result_size(uint64_t soft_limit, uint64_t hard_limit) : soft_limit(soft_limit), hard_limit(hard_limit) { } - max_result_size(uint64_t soft_limit, uint64_t hard_limit, uint64_t page_size) - : soft_limit(soft_limit) - , hard_limit(hard_limit) - , page_size(page_size) - { } - uint64_t get_page_size() const { - return page_size == 0 ? hard_limit : page_size; - } - friend bool operator==(const max_result_size&, const max_result_size&); - friend class ser::serializer; -}; - -inline bool operator==(const max_result_size& a, const max_result_size& b) { - return a.soft_limit == b.soft_limit && a.hard_limit == b.hard_limit && a.page_size == b.page_size; -} - -} diff --git a/reader_permit.hh b/reader_permit.hh index 3f8e5d8e3b..0b633c746d 100644 --- a/reader_permit.hh +++ b/reader_permit.hh @@ -14,7 +14,12 @@ #include "db/timeout_clock.hh" #include "schema/schema_fwd.hh" #include "tracing/trace_state.hh" -#include "query_class_config.hh" + +namespace query { + +struct max_result_size; + +} namespace seastar { class file; diff --git a/test/lib/reader_concurrency_semaphore.hh b/test/lib/reader_concurrency_semaphore.hh index eb105fbf84..958da45069 100644 --- a/test/lib/reader_concurrency_semaphore.hh +++ b/test/lib/reader_concurrency_semaphore.hh @@ -9,7 +9,6 @@ #pragma once #include "../../reader_concurrency_semaphore.hh" -#include "query_class_config.hh" namespace tests {