Commit Graph

1644 Commits

Author SHA1 Message Date
Botond Dénes
3286a6fa14 Merge 'Reload reclaimed bloom filters when memory is available' from Lakshmi Narayanan Sreethar
PR #17771 introduced a threshold for the total memory used by all bloom filters across SSTables. When the total usage surpasses the threshold, the largest bloom filter will be removed from memory, bringing the total usage back under the threshold. This PR adds support for reloading such reclaimed bloom filters back into memory when memory becomes available (i.e., within the 10% of available memory earmarked for the reclaimable components).

The SSTables manager now maintains a list of all SSTables whose bloom filter was removed from memory and attempts to reload them when an SSTable, whose bloom filter is still in memory, gets deleted. The manager reloads from the smallest to the largest bloom filter to maximize the number of filters being reloaded into memory.

Closes scylladb/scylladb#18186

* github.com:scylladb/scylladb:
  sstable_datafile_test: add testcase to test reclaim during reload
  sstable_datafile_test: add test to verify auto reload of reclaimed components
  sstables_manager: reload previously reclaimed components when memory is available
  sstables_manager: start a fiber to reload components
  sstable_directory_test: fix generation in sstable_directory_test_table_scan_incomplete_sstables
  sstable_datafile_test: add test to verify reclaimed components reload
  sstables: support reloading reclaimed components
  sstables_manager: add new intrusive set to track the reclaimed sstables
  sstable: add link and comparator class to support new instrusive set
  sstable: renamed intrusive list link type
  sstable: track memory reclaimed from components per sstable
  sstable: rename local variable in sstable::total_reclaimable_memory_size
2024-05-10 13:01:01 +03:00
Lakshmi Narayanan Sreethar
4d22c4b68b sstable_datafile_test: add testcase to test reclaim during reload
Signed-off-by: Lakshmi Narayanan Sreethar <lakshmi.sreethar@scylladb.com>
2024-05-09 19:57:40 +05:30
Pavel Emelyanov
5497bb5a3d loading_shared_values: Replace static-assert with concept
The templatized get_or_load() accepts Loader template parameter and
static-asserts on its signature. Concept is more suitable here.

Signed-off-by: Pavel Emelyanov <xemul@scylladb.com>

Closes scylladb/scylladb#18582
2024-05-09 16:29:49 +03:00
Benny Halevy
ebff5f5d70 everywhere: include seastar headers using angle brackets
seastar is an external library therefore it should
use the system-include syntax.

Closes scylladb/scylladb#18513
2024-05-06 10:00:31 +03:00
Benny Halevy
e5ca65f78b test/perf: report also log_allocations/op
Currently perf-simple-query --write ignores
log allocations that happen on the memtable
apply path.

This change adds tracking and accounting
of the number of log allocation,
and reporting of thereof.

For reference, here's the output of
build/release/scylla perf-simple-query --write --default-log-level=error --random-seed=1 -c 1
```
random-seed=1
enable-cache=1
Running test with config: {partitions=10000, concurrency=100, mode=write, frontend=cql, query_single_key=no, counters=no}
Disabling auto compaction
78073.55 tps ( 59.4 allocs/op,  16.3 logallocs/op,  14.3 tasks/op,   52991 insns/op,        0 errors)
77263.59 tps ( 59.3 allocs/op,  16.0 logallocs/op,  14.3 tasks/op,   53282 insns/op,        0 errors)
79913.07 tps ( 59.3 allocs/op,  16.0 logallocs/op,  14.3 tasks/op,   53295 insns/op,        0 errors)
79554.32 tps ( 59.3 allocs/op,  16.0 logallocs/op,  14.3 tasks/op,   53284 insns/op,        0 errors)
79151.53 tps ( 59.3 allocs/op,  16.0 logallocs/op,  14.3 tasks/op,   53289 insns/op,        0 errors)

median 79151.53 tps ( 59.3 allocs/op,  16.0 logallocs/op,  14.3 tasks/op,   53289 insns/op,        0 errors)
median absolute deviation: 761.54
maximum: 79913.07
minimum: 77263.59
```

Signed-off-by: Benny Halevy <bhalevy@scylladb.com>
2024-05-02 18:42:41 +03:00
Kefu Chai
08d1362f80 utils/chunked_vector: fix some typos in comment
Signed-off-by: Kefu Chai <kefu.chai@scylladb.com>

Closes scylladb/scylladb#18486
2024-05-01 16:38:43 +03:00
Avi Kivity
329b135b5e Merge 'chunked_vector: fix use after free in emplace back' from Benny Halevy
Currently, push_back or emplace_back reallocate the last chunk
before constructing the new element.

If the arg passed to push_back/emplace_back is a reference to an
existing element in the vector, reallocating the last chunk will
invalidate the arg reference before it is used.

This patch changes the order when reallocating
the last chunk in reserve_for_emplace_back:
First, a new chunk_ptr is allocated.
Then, the back_element is emplaced in the
newly allocated array.
And only then, existing elements in the current
last chunk are migrated to the new chunk.
Eventually, the new chunk replaces the existing chunk.

If no reservation is requried, the back element
is emplaced "in place" in the current last chunk.

Fixes scylladb/scylladb#18072

Closes scylladb/scylladb#18073

* github.com:scylladb/scylladb:
  test: chunked_managed_vector_test: add test_push_back_using_existing_element
  utils: chunked_vector: reserve_for_emplace_back: emplace before migrating existing elements
  utils: chunked_vector: push_back: call emplace_back
  utils: chunked_vector: define min_chunk_capacity
  utils: chunked*vector: use std::clamp
2024-04-30 18:09:04 +03:00
Kefu Chai
3a1ceb96d7 utils: UUID_gen: include <atomic>
in UUID_gen.cc, we are using `std::atomic<int64_t>` in
`make_thread_local_node()`, but this template is not defined by
any of the included headers. but  we should include used headers
to be self-contained.

when compiling on ubuntu:jammy with libstdc++-13, we have following
error:
```
/usr/local/bin/cmake -E __run_co_compile --tidy="clang-tidy-18;--checks=-*,bugprone-use-after-move;--extra-arg-before=--driver-mode=g++" --source=/home/runner/work/scylladb/scylladb/utils/UUID_gen.cc -- /usr/bin/clang++-18 -DBOOST_ALL_NO_LIB -DBOOST_NO_CXX98_FUNCTION_BASE -DBOOST_REGEX_DYN_LINK -DSCYLLA_BUILD_MODE=release -DSEASTAR_API_LEVEL=7 -DSEASTAR_LOGGER_COMPILE_TIME_FMT -DSEASTAR_LOGGER_TYPE_STDOUT -DSEASTAR_SCHEDULING_GROUPS_COUNT=16 -DSEASTAR_SSTRING -DXXH_PRIVATE_API -I/home/runner/work/scylladb/scylladb -I/home/runner/work/scylladb/scylladb/seastar/include -I/home/runner/work/scylladb/scylladb/build/seastar/gen/include -I/home/runner/work/scylladb/scylladb/build/seastar/gen/src -isystem /home/runner/work/scylladb/scylladb/cooking/include -ffunction-sections -fdata-sections -O3 -g -gz -std=gnu++20 -fvisibility=hidden -Wall -Werror -Wextra -Wno-error=deprecated-declarations -Wimplicit-fallthrough -Wno-c++11-narrowing -Wno-deprecated-copy -Wno-mismatched-tags -Wno-missing-field-initializers -Wno-overl
Error: /home/runner/work/scylladb/scylladb/utils/UUID_gen.cc:29:33: error: implicit instantiation of undefined template 'std::atomic<long>' [clang-diagnostic-error]
   29 |     static std::atomic<int64_t> thread_id_counter;
      |                                 ^
/usr/lib/gcc/x86_64-linux-gnu/13/../../../../include/c++/13/bits/shared_ptr_atomic.h:361:11: note: template is declared here
  361 |     class atomic;
      |           ^
```
so, in this change, we include `<atomic>` to address this
build failure.

Signed-off-by: Kefu Chai <kefu.chai@scylladb.com>

Closes scylladb/scylladb#18387
2024-04-30 09:07:22 +03:00
Kefu Chai
48048c2f94 utils/to_string: include fmt/std.h if fmt >= v10
in to_string.hh, we define the specialization of
`fmt::formatter<std::optional<T>>`, which is available in {fmt} v10
and up. to avoid conditionally including `utils/to_string.hh` and
`fmt/std.h` in all source files formatting `std::optional<T>` using
{fmt}, let's include `fmt/std.h` if {fmt}'s verison is greater or equal
to 10. in future, we should drop the specialization and use `fmt/std.h`
directly.

Refs #13245

Signed-off-by: Kefu Chai <kefu.chai@scylladb.com>

Closes scylladb/scylladb#18325
2024-04-23 12:09:05 +03:00
Kefu Chai
372a4d1b79 treewide: do not define FMT_DEPRECATED_OSTREAM
since we do not rely on FMT_DEPRECATED_OSTREAM to define the
fmt::formatter for us anymore, let's stop defining `FMT_DEPRECATED_OSTREAM`.

in this change,

* utils: drop the range formatters in to_string.hh and to_string.c, as
  we don't use them anymore. and the tests for them in
  test/boost/string_format_test.cc are removed accordingly.
* utils: use fmt to print chunk_vector and small_vector. as
  we are not able to print the elements using operator<< anymore
  after switching to {fmt} formatters.
* test/boost: specialize fmt::details::is_std_string_like<bytes>
  due to a bug in {fmt} v9, {fmt} fails to format a range whose
  element type is `basic_sstring<uint8_t>`, as it considers it
  as a string-like type, but `basic_sstring<uint8_t>`'s char type
  is signed char, not char. this issue does not exist in {fmt} v10,
  so, in this change, we add a workaround to explicitly specialize
  the type trait to assure that {fmt} format this type using its
  `fmt::formatter` specialization instead of trying to format it
  as a string. also, {fmt}'s generic ranges formatter calls the
  pair formatter's `set_brackets()` and `set_separator()` methods
  when printing the range, but operator<< based formatter does not
  provide these method, we have to include this change in the change
  switching to {fmt}, otherwise the change specializing
  `fmt::details::is_std_string_like<bytes>` won't compile.
* test/boost: in tests, we use `BOOST_REQUIRE_EQUAL()` and its friends
  for comparing values. but without the operator<< based formatters,
  Boost.Test would not be able to print them. after removing
  the homebrew formatters, we need to use the generic
  `boost_test_print_type()` helper to do this job. so we are
  including `test_utils.hh` in tests so that we can print
  the formattable types.
* treewide: add "#include "utils/to_string.hh" where
  `fmt::formatter<optional<>>` is used.
* configure.py: do not define FMT_DEPRECATED_OSTREAM
* cmake: do not define FMT_DEPRECATED_OSTREAM

Refs #13245

Signed-off-by: Kefu Chai <kefu.chai@scylladb.com>
2024-04-19 22:57:36 +08:00
Kefu Chai
a439ebcfce treewide: include fmt/ranges.h and/or fmt/std.h
before this change, we rely on the default-generated fmt::formatter
created from operator<<, but fmt v10 dropped the default-generated
formatter.

in this change, we include `fmt/ranges.h` and/or `fmt/std.h`
for formatting the container types, like vector, map
optional and variant using {fmt} instead of the homebrew
formatter based on operator<<.
with this change, the changes adding fmt::formatter and
the changes using ostream formatter explicitly, we are
allowed to drop `FMT_DEPRECATED_OSTREAM` macro.

Refs scylladb#13245

Signed-off-by: Kefu Chai <kefu.chai@scylladb.com>
2024-04-19 22:56:16 +08:00
Kefu Chai
01f13850cb utils/managed_bytes: add support for fmt::to_string() to bytes and friends
in 3835ebfcdc, `fmt::formatter` were added to `bytes` and friend, but
their `format()` methods were intentionally implemented as plain
methods, which only acccept `fmt::format_context`. it was a decision
decision. the intention was to reduce the usage of template, to speed
up the compilation at the expense of dropping the support of other
appenders, notably the one used by `fmt::to_string()`, where the type
of "format_context" is not a `fmt::format_context`, but a string
appender. but it turns out we still have users in tests using
`fmt::to_string()`, to convert, for instance, `bytes` to `std::string`,

so, to make their life easier, we add the templated `format()` to
these types. an alternative is to change the callers to use something
like `fmt::format("{}", v)`, which is less convenient though.

Refs #13245

Signed-off-by: Kefu Chai <kefu.chai@scylladb.com>
2024-04-19 22:56:13 +08:00
Kefu Chai
168ade72f8 treewide: replace formatter<std::string_view> with formatter<string_view>
in in {fmt} before v10, it provides the specialization of `fmt::formatter<..>`
for `std::string_view` as well as the specialization of `fmt::formatter<..>`
for `fmt::string_view` which is an implementation builtin in {fmt} for
compatibility of pre-C++17. and this type is used even if the code is
compiled with C++ stadandard greater or equal to C++17. also, before v10,
the `fmt::formatter<std::string_view>::format()` is defined so it accepts
`std::string_view`. after v10, `fmt::formatter<std::string_view>` still
exists, but it is now defined using `format_as()` machinery, so it's
`format()` method does not actually accept `std::string_view`, it
accepts `fmt::string_view`, as the former can be converted to
`fmt::string_view`.

this is why we can inherit from `fmt::formatter<std::string_view>` and
use `formatter<std::string_view>::format(foo, ctx);` to implement the
`format()` method with {fmt} v9, but we cannot do this with {fmt} v10,
and we would have following compilation failure:

```
FAILED: service/CMakeFiles/service.dir/RelWithDebInfo/topology_state_machine.cc.o
/home/kefu/.local/bin/clang++ -DFMT_DEPRECATED_OSTREAM -DFMT_SHARED -DSCYLLA_BUILD_MODE=release -DSEASTAR_API_LEVEL=7 -DSEASTAR_LOGGER_COMPILE_TIME_FMT -DSEASTAR_LOGGER_TYPE_STDOUT -DSEASTAR_SCHEDULING_GROUPS_COUNT=16 -DSEASTAR_SSTRING -DXXH_PRIVATE_API -DCMAKE_INTDIR=\"RelWithDebInfo\" -I/home/kefu/dev/scylladb -I/home/kefu/dev/scylladb/build/gen -I/home/kefu/dev/scylladb/seastar/include -I/home/kefu/dev/scylladb/build/seastar/gen/include -I/home/kefu/dev/scylladb/build/seastar/gen/src -ffunction-sections -fdata-sections -O3 -g -gz -std=gnu++20 -fvisibility=hidden -Wall -Werror -Wextra -Wno-error=deprecated-declarations -Wimplicit-fallthrough -Wno-c++11-narrowing -Wno-deprecated-copy -Wno-mismatched-tags -Wno-missing-field-initializers -Wno-overloaded-virtual -Wno-unsupported-friend -Wno-enum-constexpr-conversion -Wno-unused-parameter -ffile-prefix-map=/home/kefu/dev/scylladb=. -march=westmere -mllvm -inline-threshold=2500 -fno-slp-vectorize -U_FORTIFY_SOURCE -Werror=unused-result -MD -MT service/CMakeFiles/service.dir/RelWithDebInfo/topology_state_machine.cc.o -MF service/CMakeFiles/service.dir/RelWithDebInfo/topology_state_machine.cc.o.d -o service/CMakeFiles/service.dir/RelWithDebInfo/topology_state_machine.cc.o -c /home/kefu/dev/scylladb/service/topology_state_machine.cc
/home/kefu/dev/scylladb/service/topology_state_machine.cc:254:41: error: no matching member function for call to 'format'
  254 |     return formatter<std::string_view>::format(it->second, ctx);
      |            ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~^~~~~~
/usr/include/fmt/core.h:2759:22: note: candidate function template not viable: no known conversion from 'seastar::basic_sstring<char, unsigned int, 15>' to 'const fmt::basic_string_view<char>' for 1st argument
 2759 |   FMT_CONSTEXPR auto format(const T& val, FormatContext& ctx) const
      |                      ^      ~~~~~~~~~~~~
```

because the inherited `format()` method actually comes from
`fmt::formatter<fmt::string_view>`. to reduce the confusion, in this
change, we just inherit from `fmt::format<string_view>`, where
`string_view` is actually `fmt::string_view`. this follows
the document at
https://fmt.dev/latest/api.html#formatting-user-defined-types,
and since there is less indirection under the hood -- we do not
use the specialization created by `FMT_FORMAT_AS` which inherit
from `formatter<fmt::string_view>`, hopefully this can improve
the compilation speed a little bit. also, this change addresses
the build failure with {fmt} v10.

Signed-off-by: Kefu Chai <kefu.chai@scylladb.com>

Closes scylladb/scylladb#18299
2024-04-19 07:44:07 +03:00
Kefu Chai
2bb8e7c3c3 utils: include "seastarx.hh" in composite_abort_source.hh
there is chance that `utils/small_vector.hh` does not include
`using namespace seastar`, and even if it does, we should not rely
on it. but if it does not, checkhh would fail. so let's include
"seastarx.hh" in this header, so it is self-contained.

Signed-off-by: Kefu Chai <kefu.chai@scylladb.com>

Closes scylladb/scylladb#18265
2024-04-17 07:11:01 +03:00
Benny Halevy
2afc584f08 utils: chunked_vector: reserve_for_emplace_back: emplace before migrating existing elements
Currently, push_back or emplace_back reallocate the last chunk
before constructing the new element.

If the arg passed to push_back/emplace_back is a reference to an
existing element in the vector, reallocating the last chunk will
invalidate the arg reference before it is used.

This patch changes the order when reallocating
the last chunk in reserve_for_emplace_back:
First, a new chunk_ptr is allocated.
Then, the back_element is emplaced in the
newly allocated array.
And only then, existing elements in the current
last chunk are migrated to the new chunk.
Eventually, the new chunk replaces the existing chunk.

If no reservation is requried, the back element
is emplaced "in place" in the current last chunk.

Fixes scylladb/scylladb#18072

Signed-off-by: Benny Halevy <bhalevy@scylladb.com>
2024-04-11 14:34:48 +03:00
Benny Halevy
2c0e40a21f utils: chunked_vector: push_back: call emplace_back
When pushing an element with a value referencing
an exisiting element in the vector, we currently
risking use-after-free when that element gets moved
to a reallocated chunk, if capacity needs to be reserved,
by that, invaliding the refernce to the existing element
before it is used.

This patch prepares for fixing that in the emplace path
by converging to a single code path.

Signed-off-by: Benny Halevy <bhalevy@scylladb.com>
2024-04-11 14:33:43 +03:00
Benny Halevy
882bb21903 utils: chunked_vector: define min_chunk_capacity
Expose the number of items in the first allocated chunk.
This will be used by a unit test in the next patch.

Signed-off-by: Benny Halevy <bhalevy@scylladb.com>
2024-04-11 14:33:43 +03:00
Benny Halevy
e066f81cb3 utils: chunked*vector: use std::clamp
It is available in the std library since C++17.

Signed-off-by: Benny Halevy <bhalevy@scylladb.com>
2024-04-11 14:33:43 +03:00
Kefu Chai
0be61e51d3 treewide: include <fmt/ostream.h>
this header was previously brought in by seastar's sstring.hh. but
since sstring.hh does not include <fmt/ostream.h> anymore,
`gms/application_state.cc` does not have access to this header.
also, `gms/application_state.cc` should `#include` the used header
by itself.

so, in this change, let's include  <fmt/ostream.h> in `gms/application_state.cc`.
this change addresses the FTBFS with the latest seastar.

the same applies to other places changed in this commit.

Signed-off-by: Kefu Chai <kefu.chai@scylladb.com>

Closes scylladb/scylladb#18193
2024-04-11 11:59:41 +03:00
Kefu Chai
fcf7ca5675 utils/logalloc: do not allocate memory in reclaim_timer::report()
before this change, `reclaim_timer::report()` calls

```c++
fmt::format(", at {}", current_backtrace())
```

which allocates a `std::string` on heap, so it can fail and throw. in
that case, `std::terminate()` is called. but at that moment, the reason
why `reclaim_timer::report()` gets called is that we fail to reclaim
memory for the caller. so we are more likely to run into this issue. anyway,
we should not allocate memory in this path.

in this change, a dedicated printer is created so that we don't format
to a temporary `std::string`, and instead write directly to the buffer
of logger. this avoids the memory allocation.

Fixes #18099
Signed-off-by: Kefu Chai <kefu.chai@scylladb.com>

Closes scylladb/scylladb#18100
2024-04-01 11:01:52 +03:00
Botond Dénes
885cb2af07 utils/rjson: include tasklocal backtrace in rapidjson assert error message
Currently, the error message on a failed RAPIDJSON_ASSERT() is this:

    rjson::error (JSON error: condition not met: false)

This is printed e.g. when the code processing a json expects an object
but the JSON has a different type. Or if a JSON object is missing an
expected member. This message however is completely inadequate for
determinig what went wrong. Change this to include a task-local
backtrace, like a real assert failure would. The new error looks like
this:

    rjson::error (JSON assertion failed on condition '{}' at: libseastar.so+0x56dede 0x2bde95e 0x2cc18f3 0x2cf092d 0x2d2316b libseastar.so+0x46b623)

Closes scylladb/scylladb#18101
2024-03-29 18:41:54 +01:00
Kefu Chai
a047178fe7 utils: UUID: drop UUID::to_sstring()
this function is not used anymore, and it relies on
`FMT_DEPRECATED_OSTREAM` to generated `fmt::formatter` for
`UUID`, and this feature is deprecated in {fmt} v9, and
dropped in {fmt} v10.

in this change, let's drop this member function.

Signed-off-by: Kefu Chai <kefu.chai@scylladb.com>
2024-03-26 13:38:37 +08:00
Kefu Chai
1b859e484f treewide: use fmt::to_string() to transform a UUID to std::string
without `FMT_DEPRECATED_OSTREAM` macro, `UUID::to_sstring()` is
implemented using its `fmt::formatter`, which is not available
at the end of this header file where `UUID` is defined. at this moment,
we still use `FMT_DEPRECATED_OSTREAM` and {fmt} v9, so we can
still use `UUID::to_sstring()`, but in {fmt} v10, we cannot.

so, in this change, we change all callers of `UUID::to_sstring()`
to `fmt::to_string()`, so that we don't depend on
`FMT_DEPRECATED_OSTREAM` and {fmt} v9 anymore.

Signed-off-by: Kefu Chai <kefu.chai@scylladb.com>
2024-03-26 13:38:37 +08:00
Kamil Braun
9979adb670 Merge 'topology_coordinator: do not clear unpublished CDC generation's data' from Patryk Jędrzejczak
In this PR, we ensure unpublished CDC generation's data is
never removed, which was theoretically possible. If it happened,
it could cause problems. CDC generation publisher would then try
to publish the generation with its data removed. In particular, the
precondition of calling `_sys_ks.read_cdc_generation` wouldn't be
satisfied.

We also add a test that passes only after the fix. However, this test
needs to block execution of the CDC generation publisher's loop
twice. Currently, error injections with handlers do not allow it
because handlers always share received messages. Apart from the
first created handler, all handlers would be instantly unblocked by
a message from the past that has already unblocked the first
handler. This seems like a general limitation that could cause
problems in the future, so in this PR, we extend injections with
handlers to solve it once and for all. We add the `share_messages`
parameter to the `inject` (with handler) function. Depending on its
value, handlers will share messages (as before) or not.

Fixes scylladb/scylladb#17497

Closes scylladb/scylladb#17934

* github.com:scylladb/scylladb:
  topology_coordinator: clean_obsolete_cdc_generations: fix log
  topology_coordinator: do not clear unpublished CDC generation's data
  topology_coordinator: cdc_generation_publisher_fiber injection: make handlers share messages
  error_injection: allow injection handlers to not share messages
2024-03-22 11:20:26 +01:00
Kamil Braun
4359a1b460 Merge 'raft timeouts: better handling of lost quorum' from Petr Gusev
In this PR we add timeouts support to raft groups registry. We introduce
the `raft_server_with_timeouts` class, which wraps the `raft::server`
add exposes its interface with additional `raft_timeout` parameter. If
it's set, the wrapper cancels the `abort_source` after certain amount of
time. The value of the timeout can be specified either in the
`raft_timeout` parameter, or the default value can be set in `the
raft_server_with_timeouts` class constructor.

The `raft_group_registry` interface is extended with
`group0_with_timeouts()` method. It returns an instance of
`raft_server_with_timeouts` for group0 raft server. The timeout value
for it is configured in `create_server_for_group0`. It's one minute by
default and can be overridden for tests with
`group0-raft-op-timeout-in-ms` parameter.

The new api allows the client to decide whether to use timeouts or not.
In this PR we are reviewing all the group0 call sites and add
`raft_timeout` if that makes sense. The general principle is that if the
code is handling a client request and the client expects a potential
error, we use timeouts. We don't use timeouts for background fibers
(such as topology coordinator), since they wouldn't add much value. The
only thing the background fiber can do with a timeout is to retry, and
this will have the same end effect as not having a timeout at all.

Fixes scylladb/scylladb#16604

Closes scylladb/scylladb#17590

* github.com:scylladb/scylladb:
  migration_manager: use raft_timeout{}
  storage_service::join_node_response_handler: use raft_timeout{}
  storage_service::start_upgrade_to_raft_topology: use raft_timeout{}
  storage_service::set_tablet_balancing_enabled: use raft_timeout{}
  storage_service::move_tablet: use raft_timeout{}
  raft_check_and_repair_cdc_streams: use raft_timeout{}
  raft_timeout: test that node operations fail properly
  raft_rebuild: use raft_timeout{}
  do_cluster_cleanup: use raft_timeout{}
  raft_initialize_discovery_leader: use raft_timeout{}
  update_topology_with_local_metadata: use with_timeout{}
  raft_decommission: use raft_timeout{}
  raft_removenode: use raft_timeout{}
  join_node_request_handler: add raft_timeout to make_nonvoters and add_entry
  raft_group0: make_raft_config_nonvoter: add raft_timeout parameter
  raft_group0: make_raft_config_nonvoter: add abort_source parameter
  manager_client: server_add with start=false shouldn't call driver_connect
  scylla_cluster: add seeds parameter to the add_server and servers_add
  raft_server_with_timeouts: report the lost quorum
  join_node_request_handler: add raft_timeout{} for start_operation
  skip_mode: add platform_key
  auth: use raft_timeout{}
  raft_group0_client: add raft_timeout parameter
  raft_group_registry: add group0_with_timeouts
  utils: add composite_abort_source.hh
  error_injection: move api registration to set_server_init
  error_injection: add inject_parameter method
  error_injection: move injection_name string into injection_shared_data
  error_injection: pass injection parameters at startup
2024-03-22 10:45:33 +01:00
Patryk Jędrzejczak
c5c4cc7d00 error_injection: allow injection handlers to not share messages
For a single injection, all created injection handlers share all
received messages. In particular, it means that one received message
unblocks all handlers waiting for the first message. This behavior
is often desired, for example, if multiple fibers execute the
injected code and we want to unblock them all with a single message.
However, there is a problem if we want to block every execution
of the injected code. Apart from the first created handler, all
handlers will be instantly unblocked by messages from the past that
have already unblocked the first handler.

In one of the following commits, we add a test that needs to block
the CDC generation publisher's loop twice. Since it looks like there
are no good workarounds for this arguably general problem, we extend
injections with handlers in a way that solves it. We introduce the
new `share_messages` parameter. Depending on its value, handlers
will share messages or not. The details are described in the new
comments in `error_injection.hh`.

We also add some basic unit tests for the new funcionality.
2024-03-21 14:35:38 +01:00
Petr Gusev
532a720c3d utils: add composite_abort_source.hh 2024-03-21 16:12:51 +04:00
Kefu Chai
a58be49abf utils: add fmt::formatter for utils::bad_exception_container_access
before this change, we rely on the default-generated fmt::formatter
created from operator<<, but fmt v10 dropped the default-generated
formatter.

in this change, `fmt::formatter<utils::bad_exception_container_access>` is
added for backward compatibility with {fmt} < 10.

Refs #13245

Signed-off-by: Kefu Chai <kefu.chai@scylladb.com>
2024-03-21 12:48:19 +08:00
Petr Gusev
e4318e139d error_injection: add inject_parameter method
In this commit we extend the error_injector
with a new method inject_parameter. It allows
to pass parameters from tests to scylla, e.g. to
lower timeouts or limits. A typical use cases is
described in scylladb/scylladb#15571.

It's logically the same as inject_with_handler,
whose lambda reads the parameter named 'value'.
The only difference is that the inject_parameter
doesn't return future, it just read the
parameter from  the injection shared_data.
2024-03-19 20:18:23 +04:00
Petr Gusev
460567c4fd error_injection: move injection_name string into injection_shared_data
In subsequent commit we'll need the injection_name from inside
injection_shared_data, so in this commit we move it there.
Additionally, we fix the todo about switching the injections dictionary
from map to unordered_set, now unordered_map contains
string_views, pointing to injection_name inside
injection_shared_data.
2024-03-19 20:17:02 +04:00
Petr Gusev
49a4220fea error_injection: pass injection parameters at startup
Injection parameters can be used in the lambda passed to
inject_with_handler method to take some values from
the test. However, there was no way to set values to these
parameters on node startup, only through
the error injection REST api. Therefore, we couldn't rely
on this when inject_with_handler is used during
node startup, it could trigger before we call the api
from the test.

In this commit with solve this problem by allowing these
parameters to be assigned through scylla.yaml config.

The defer.hh header was added to error_injection.hh to fix
compilation after adding error_injection.hh to config.hh,
defer function is used in error_injection.hh.
2024-03-19 20:17:02 +04:00
Avi Kivity
dd76e1c834 Merge 'Simplify error_injection::inject_with_handler()' from Pavel Emelyanov
The method in question can have a shorter name that matches all other injections in this class, and can be non-template

Closes scylladb/scylladb#17734

* github.com:scylladb/scylladb:
  error_injection: De-template inject() with handler
  error_injection: Overload inject() instead of inject_with_handler()
2024-03-14 13:37:54 +02:00
Avi Kivity
4db4b2279c Merge 'tools/scylla-nodetool: implement the last batch of commands' from Botond Dénes
This PR implements the following new nodetool commands:
* netstats
* tablehistograms/cfhistograms
* proxyhistograms

All commands come with tests and all tests pass with both the new and the current nodetool implementations.

Refs: https://github.com/scylladb/scylladb/issues/15588

Closes scylladb/scylladb#17651

* github.com:scylladb/scylladb:
  tools/scylla-nodetool: implement the proxyhistograms command
  tools/scylla-nodetool: implement the tableshistograms command
  tools/scylla-nodetool: introduce buffer_samples
  utils/estimated_histogram: estimated_histogram: add constructor taking buckets
  tools/scylla-nodetool: implement the netstats command
  tools/scylla-nodetool: add correct units to file_size_printer
2024-03-13 12:46:11 +02:00
Pavel Emelyanov
88a40b0dfa uuid: UUID_gen::get_UUID src argument is const pointer
Signed-off-by: Pavel Emelyanov <xemul@scylladb.com>

Closes scylladb/scylladb#17762
2024-03-13 10:21:25 +02:00
Botond Dénes
47ac7d70e4 utils/estimated_histogram: estimated_histogram: add constructor taking buckets
And bucket offsets. Allows constructing the histogram back from a json
format.
2024-03-13 02:06:30 -04:00
Kefu Chai
35fc065458 utils/exception_container: add fmt::formatter for exception_container
before this change, we rely on the default-generated fmt::formatter
created from operator<<, but fmt v10 dropped the default-generated
formatter.

in this change, we define formatters for `exception_container<..>`
and drop its operator<<.

Refs #13245

Signed-off-by: Kefu Chai <kefu.chai@scylladb.com>
2024-03-12 14:53:55 +08:00
Kefu Chai
9300d7b80b utils/human_readable: add fmt::formatter for human_readable_value
before this change, we rely on the default-generated fmt::formatter created
from operator<<, but fmt v10 dropped the default-generated formatter.

in this change, we define formatters for `utils::human_readable_value`,
and drop its operator<<

Refs #13245

Signed-off-by: Kefu Chai <kefu.chai@scylladb.com>
2024-03-12 14:53:55 +08:00
Kefu Chai
007d7f1355 utils: add fmt::formatter for std::strong_ordering and friends
before this change, we rely on the default-generated fmt::formatter created
from operator<<, but fmt v10 dropped the default-generated formatter.

in this change, we define formatters for

* std::strong_ordering
* std::weak_ordering
* std::partial_ordering

and their operator<<:s are moved to test/lib/test_utils.{hh,cc}, as they
are only used by Boost.test.

Refs #13245

Signed-off-by: Kefu Chai <kefu.chai@scylladb.com>
2024-03-12 14:53:55 +08:00
Pavel Emelyanov
0d5c25aef5 error_injection: De-template inject() with handler
The recently renamed inject_with_handler() was a template, but it can be
symmetrical to its peer that accepts void function as a callback, and
use std::function as its argument.

Signed-off-by: Pavel Emelyanov <xemul@scylladb.com>
2024-03-11 19:32:21 +03:00
Pavel Emelyanov
1f44a374b8 error_injection: Overload inject() instead of inject_with_handler()
The inject_with_handler() method accepts a coroutine that can be called
wiht injection_handler. With such function as an argument, there's no
need in distinctive inject_with_handler() name for a method, it can be
overload of all the existing inject()-s

Signed-off-by: Pavel Emelyanov <xemul@scylladb.com>
2024-03-11 19:30:19 +03:00
Kefu Chai
3835ebfcdc utils/managed_bytes: add fmt::formatters for managed_bytes and friends
before this change, we rely on the default-generated fmt::formatter
created from operator<<, but fmt v10 dropped the default-generated
formatter.

in this change, we define formatters for

* managed_bytes
* managed_bytes_view
* managed_bytes_opt

Refs #13245

Signed-off-by: Kefu Chai <kefu.chai@scylladb.com>
2024-02-23 11:32:41 +08:00
Kefu Chai
3d9054991b utils/logalloc: add fmt::formatter for occupancy_stats
before this change, we rely on the default-generated fmt::formatter
created from operator<<, but fmt v10 dropped the default-generated
formatter.

in this change, we define formatters for `occupancy_stats`, and
drop its operator<<.

Refs #13245

Signed-off-by: Kefu Chai <kefu.chai@scylladb.com>
2024-02-23 11:32:41 +08:00
Avi Kivity
51df8b9173 interval: rename nonwrapping_interval to interval
Our interval template started life as `range`, and was supported
wrapping to follow Cassandra's convention of wrapping around the
maximum token.

We later recognized that an interval type should usually be non-wrapping
and split it into wrapping_range and nonwrapping_range, with `range`
aliasing wrapping_range to preserve compatibility.

Even later, we realized the name was already taken by C++ ranges and
so renamed it to `interval`. Given that intervals are usually non-wrapping,
the default `interval` type is non-wrapping.

We can now simplify it further, recognizing that everyone assumes
that an interval is non-wrapping and so doesn't need the
nonwrapping_interval_designation. We just rename nonwrapping_interval
to `interval` and remove the type alias.
2024-02-21 19:43:17 +02:00
Avi Kivity
605bf6e221 range.hh: retire
range.hh was deprecated in bd794629f9 (2020) since its names
conflict with the C++ library concept of an iterator range. The name
::range also mapped to the dangerous wrapping_interval rather than
nonwrapping_interval.

Complete the deprecation by removing range.hh and replacing all the
aliases by the names they point to from the interval library. Note
this now exposes uses of wrapping intervals as they are now explicit.

The unit tests are renamed and range.hh is deleted.

Closes scylladb/scylladb#17428
2024-02-21 00:24:25 +02:00
Kefu Chai
a7a2cf64cc utils/rjson: add templated streaming_writer::Write()
so we can use it in a templated context.

Signed-off-by: Kefu Chai <kefu.chai@scylladb.com>
2024-02-20 18:12:35 +08:00
Kefu Chai
4da9a62472 utils: managed_bytes: fix typo in comment
s/assigments/assignments/

this misspelling was identified by codespell.

Signed-off-by: Kefu Chai <kefu.chai@scylladb.com>

Closes scylladb/scylladb#17333
2024-02-15 10:37:25 +02:00
Botond Dénes
120442231f Merge 'row_cache: test cache consistency during multi-partition cache updates' from Michał Chojnowski
Adds a test reproducing https://github.com/scylladb/scylladb/issues/16759, and the instrumentation needed for it.

Closes scylladb/scylladb#17208

* github.com:scylladb/scylladb:
  row_cache_test: test cache consistency during memtable-to-cache merge
  row_cache: use preemption_source in update()
  utils: preempt: add preemption_source
2024-02-13 17:37:06 +02:00
Michał Chojnowski
5a3e4a1cc0 utils: managed_bytes: optimize memory usage for small buffers
managed_bytes is implemented as chain of blob_storage objects.
Each blob_storage contains 24 bytes of metadata. But in the most
common case -- when there is only a single element in the chain --
16 bytes of this metadata is trivial/unused.

This is regrettable waste because managed_bytes is used for every
database cell in the memtables and cache. It means that every value
of size >= 7 bytes (smaller ones fit in the inline storage of
managed_bytes) receives 16 bytes of useless overhead.

To correct that, this patch adds to managed_bytes an alternative storage
layout -- used for buffers small enough to fit in one contiguous
fragment -- which only stores the necessary minimum of metadata.
(That is: a pointer to the parent, to facilitate moving the storage during
memory defragmentation).
2024-02-09 20:56:20 +01:00
Michał Chojnowski
277a31f0ae utils: managed_bytes: rewrite managed_bytes methods in terms of managed_bytes_view
Some methods of managed_bytes contain the logic needed to read/write the
contents of managed_bytes, even though this logic is already present in
managed_bytes_{,mutable}_view.

Reimplementing those methods by using the views as intermediates allows us to
remove some code and makes the responsibilities cleaner -- after the change,
managed_bytes contains the logic of allocating and freeing the storage,
while views provide read/write access to the storage.

This change will simplify the next patch which changes the internals of
managed_bytes.
2024-02-09 17:00:33 +01:00
Michał Chojnowski
fabab2f46f utils: preempt: add preemption_source
While `preemption_check` can be passed to functions to control
their preemption points, there is no way to inspect the
state of the system after the preemption results in a yield.

`preemption_source` is a superset of `preemption_check`,
which also allows for customizing the yield, not just the preemption
check. An implementation passed by a test can hook the yield to
put the tested function to sleep, run some code, and then wake the
function up.

We use the preprocessor to minimize the impact on release builds.
Only dev-mode preemption_source is hookable. When it's used in other
modes, it should compile to direct reactor calls, as if it wasn't used.
2024-02-07 18:31:28 +01:00