Commit Graph

42306 Commits

Author SHA1 Message Date
Kefu Chai
e5bcea6718 docs: drop documents related to {,dclocal_}read_repair_chance
since "read_repair_chance" and "dclocal_read_repair_chance" are
removed, and not supported anymore. let's stop documenting them.

Refs #3502

Signed-off-by: Kefu Chai <kefu.chai@scylladb.com>
2024-04-25 17:15:27 +08:00
Kefu Chai
c323c93fa4 treewide: remove {dclocal_,}read_repair_chance options
dclocal_read_repair_chance and read_repair_chance have been removed
in Cassandra 3.11 and 4.x, see
https://issues.apache.org/jira/browse/CASSANDRA-13910.
if we expose the properties via DDL, Cassandra would fails to consume
the CQL statement to creating the table when performing migration
from Scylla to Cassandra 4.x, as the latter does not understand
these properties anymore.

currently the default values of `dc_local_read_repair_chance` and
`read_repair_chance` are both "0". so this is practically disabled,
unless user deliberately set them to a value greater than 0.

also, as a side effect, Cassandra 4.x has better support of
Python3. the cqlsh shipped along with Cassandra 3.11.16 only
supports python2.7, see
https://github.com/apache/cassandra/blob/cassandra-3.11.16/bin/cqlsh.py
it errors out if the system only provides python3 with the error
of

```
No appropriate python interpreter found.
```
but modern linux systems do not provide python2 anymore.

so, in this change, we deprecate these two options.

Fixes #3502
Signed-off-by: Kefu Chai <kefu.chai@scylladb.com>
2024-04-25 17:15:27 +08:00
Kamil Braun
3363f6e1e8 Merge 'Fix write failures during node replace with same IP with topology over raft' from Gleb
Currently a new node is marked as alive too late, after it is already
reported as a pending node. The patch series changes replace procedure
to be the same as what node_ops do: first stop reporting the IP of the
node that is being replaced as a natural replica for writes, then mark
the IP is alive, and only after that report the IP as a pending endpoint.

Fixes: scylladb/scylladb#17421

* 'gleb/17421-fix-v2' of github.com:scylladb/scylla-dev:
  test_replace_reuse_ip: add data plane load
  sync_raft_topology_nodes: make replace procedure similar to nodeops one
  storage_service: topology_coordinator: fix indentation after previous patch
  storage_service: topology coordinator: drop ring check in node_state::replacing state
2024-04-24 17:09:01 +02:00
Petr Gusev
bc98774f83 test_replace_reuse_ip: add data plane load
In this commit we enhance test_replace_reuse_ip
to reproduce #17421. We create a test table and run
insert queries on it while the first node is
being replaced. In this form the test fails
without the fix from the previous commit. Some
insert requests fail with [Unavailable exception]
"Cannot achieve consistency level for cl QUORUM...".
2024-04-24 16:59:24 +03:00
Gleb Natapov
4614fedd22 sync_raft_topology_nodes: make replace procedure similar to nodeops one
In replace-with-same-ip a new node calls gossiper.start_gossiping
from join_token_ring with the 'advertise' parameter set to false.
This means that this node will fail echo RPC-s from other nodes,
making it appear as not alive to them. The node changes this only
in storage_service::join_node_response_handler, when the topology
coordinator notifies it that it's actually allowed to join the
cluster. The node calls _gossiper.advertise_to_nodes({}), and
only from this moment other nodes can see it as alive.

The problem is that topology coordinator sends this notification
in topology::transition_state::join_group0 state. In this state
nodes of the cluster already see the new node as pending,
they react with calling tmpr->add_replacing_endpoint and
update_topology_change_info when they process the corresponding
raft notification in sync_raft_topology_nodes. When the new
token_metadata is published, assure_sufficient_live_nodes
sees the new node in pending_endpoints. All of this happen
before the new node handled successful join notification,
so it's not alive yet. Suppose we had a cluster with three
nodes and we're replacing on them with a fourth node.
For cl=qurum assure_sufficient_live_nodes throws if
live < need + pending, which in our case becomes 2 < 2 + 1.
The end effect is that during replace-with-same-ip
data plane requests can fail with unavailable_exception,
breaking availability.

The patch makes boot procedure more similar to node ops one.
It splits the marking of a node as "being replaced" and adding it to
pending set in to different steps and marks it as alive in the middle.
So when the node is in topology::transition_state::join_group0 state
it marked as "being replaced" which means it will no longer be used for
reads and writes. Then, in the next state, new node is marked as alive and
is added to pending list.

fixes scylladb/scylladb#17421
2024-04-24 16:59:22 +03:00
Kamil Braun
1297b9a322 mutation: mutation_by_size_splitter: skip last mutation if it's empty
Currently, the last mutation emitted by split_mutation could be empty.
It can happen as follows:
- consume range tombstone change at pos `1` with some timestamp
- consume clustering row at pos `2`
- flush: this will create mutation with range tombstone (1, 2) and
  clustering row at 2
- consume range tombstone change at pos `2` with no timestamp (i.e.
  closing rtc)
- end of partition

since the closing rtc has the same position as the clustering row, no
additional range tombstone will be emitted -- the only necessary range
tombstone was already emitted in the previous mutation.

On the other hand, `test_split_mutations` expects all emitted mutations
to be non-empty, which is a sane expectation for this function.

The test catched a case like this with random-seed=629157129.

Fix this by skipping the last mutation if it turns out to be empty.

Fixes: scylladb/scylladb#18042

Closes scylladb/scylladb#18375
2024-04-24 16:25:31 +03:00
Raphael S. Carvalho
71682aebdd storage_service: Fix use-after-move in storage_service::node_ops_cmd_handler
```
service/storage_service.cc:4288:62: warning: 'req' used after it was moved [bugprone-use-after-move]
            node_ops_insert(ops_uuid, coordinator, std::move(req.ignore_nodes), [this, coordinator, req = std::move(req)] () mutable {
                                                             ^
service/storage_service.cc:4288:107: note: move occurred here
            node_ops_insert(ops_uuid, coordinator, std::move(req.ignore_nodes), [this, coordinator, req = std::move(req)] () mutable {
                                                                                                          ^
service/storage_service.cc:4288:62: note: the use and move are unsequenced, i.e. there is no guarantee about the order in which they are evaluated
            node_ops_insert(ops_uuid, coordinator, std::move(req.ignore_nodes), [this, coordinator, req = std::move(req)] () mutable {
                                                             ^

```

if evaluation order is right-to-left (GCC), req is moved first, and req.ignore_nodes will be empty,
so nodes that should be ignored will still be considered, potentially resulting in a failure during
replace.

https://godbolt.org/z/jPcM6GEx1

courtesy of clang-tidy.

Fixes #18324.

Signed-off-by: Raphael S. Carvalho <raphaelsc@scylladb.com>

Closes scylladb/scylladb#18366
2024-04-24 15:36:28 +03:00
Kamil Braun
8876b9b0ef test/pylib: random_tables: use IF NOT EXISTS when creating keyspace
Due to Python driver's unexpected behavior, "CREATE KEYSPACE" statement
may sometimes get executed twice (scylladb/python-driver#317), leading
to "Keyspace ... already exists" error in our tests
(scylladb/scylladb#17654). Work around this by using "IF NOT EXISTS".

Fixes: scylladb/scylladb#17654

Closes scylladb/scylladb#18368
2024-04-24 10:09:26 +03:00
Anna Stuchlik
c0e4f3e646 doc: include OSS-specific info as separate files
This commit excludes OSS-specific links and content
added in https://github.com/scylladb/scylladb/pull/17624
to separate files and adds the include directive `.. scylladb_include_flag::`
to include these files in the doc source files.

Reason: Adding the link to the Open Source upgrade guide
(/upgrade/upgrade-opensource/upgrade-guide-from-5.4-to-6.0/enable-consistent-topology)
breaks the Enterprise documentation because the Enterprise docs don't
contain that upgrade guide.  We must add separate files for OSS and
Enterprise to prevent failing the Enterprise build and breaking the
links.

Closes scylladb/scylladb#18372
2024-04-23 16:59:05 +02:00
Raphael S. Carvalho
fa2dc5aefa sstables: Fix use-after-move in an error path of FS-based sstable writer
```
sstables/storage.cc:152:21: warning: 'file_path' used after it was moved [bugprone-use-after-move]
        remove_file(file_path).get();
                    ^
sstables/storage.cc:145:64: note: move occurred here
    auto w = file_writer(output_stream<char>(std::move(sink)), std::move(file_path));

```

It's a regression when TOC is found for a new sstable, and we try to delete temporary TOC.

courtesy of clang-tidy.

Fixes #18323.

Signed-off-by: Raphael S. Carvalho <raphaelsc@scylladb.com>

Closes scylladb/scylladb#18367
2024-04-23 17:19:55 +03:00
Pavel Emelyanov
f5f57dc817 table: No need to open directory in snapshot_exists()
In order to check if a snapshot of a certain name exists the checking
method opens directory. It can be made with more lightweight call.

Also, though not critical, is that it fogets to close it.

Coroutinuze the method while at it.

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

Closes scylladb/scylladb#18365
2024-04-23 17:19:24 +03:00
Botond Dénes
572003c469 Merge 'Cleanup the way snapshot details are propagated via API' from Pavel Emelyanov
There's a database::get_snapshot_details() method that returns collection of all snapshots for all ks.cf out there and there are several *snapshot_details* aux structures around it. This PR keeps only one "details" and cleans up the way it propagates from database up to the respective API calls.

Closes scylladb/scylladb#18317

* github.com:scylladb/scylladb:
  snapshot_ctl: Brush up true_snapshots_size() internals
  snapshot_ctl: Remove unused details struct
  snapshot_ctl: No double recoding of details
  database,snapshots: Move database::snapshot_details into snapshot_ctl
  database,snapshots: Make database::get_snapshot_details() return map, not vector
  table,snapshots: Move table::snapshot_details into snapshot_ctl
2024-04-23 16:28:25 +03:00
Kefu Chai
9e8805bb49 repair, transport: s/get0()/get()/
`future::get0()` was deprecated in favor of `future::get()`. so
let's use the latter instead. this change silences a `-Wdeprecated`
warning.

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

Closes scylladb/scylladb#18357
2024-04-23 15:48:54 +03:00
Kefu Chai
4fd9b2a791 reader: silence false-positive use-after-move warning
when compiling with clang-tidy, it warngs:
```
[6/9] Building CXX object readers/CMakeFiles/readers.dir/multishard.cc.o
/home/kefu/dev/scylladb/readers/multishard.cc:84:53: warning: 'fut_and_result' used after it was moved [bugprone-use-after-move]
   84 |                 auto result = std::get<1>(std::move(fut_and_result));
      |                                                     ^
/home/kefu/dev/scylladb/readers/multishard.cc:79:34: note: move occurred here
   79 |             _read_ahead_future = std::get<0>(std::move(fut_and_result));
      |                                  ^
```

but this warning is but a false alarm, as we are not really moving away
the *whole* tuple, we are just move away an element from it. but
clang-tidy cannot tell which element we are actually moving. so, silence
both places of `std::move()`.

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

Closes scylladb/scylladb#18363
2024-04-23 15:47:50 +03:00
Botond Dénes
5a1e3b25d0 Merge 'Sanitize sstables::directory_semaphore usage' from Pavel Emelyanov
The semaphore in question is used to limit parallelism of manipulations with table's sstables. It's currently used in two places -- sstable_directory (mainly on boot) and by table::take_snapshot() to take snapshot. For the latter, there's also a database -> sharded<directory_semaphore> reference.

This PR sanitizes the semaphore usage. The results are
- directory_semaphore no longer needs to friend several classes that mess with its internals
- database no longer references directory_semaphore

Closes scylladb/scylladb#18281

* github.com:scylladb/scylladb:
  database: Keep local directory_semaphore to initialize sstables managers
  database: Don't reference directory_semaphore
  table: Use directory semaphore from sstables manager
  table: Indentation fix after previous patch
  table: Use directory_semaphore for rate-limited snapshot taking
  sstables: Move directory_semaphore::parallel_for_each() to header
  sstables: Move parallel_for_each_restricted to directory_semaphore
  table: Use smp::all_cpus() to iterate over all CPUs locally
2024-04-23 13:54:52 +03:00
Kefu Chai
ab4de1f470 auth: move fmt::formatter<auth::resource_kind> up
before this change, `fmt::formatter<auth::resource_kind>` is located at
line 250 in this file, but it is used at line 130. so, {fmt} is not able
to find it:

```
/usr/include/fmt/core.h:2593:45: error: implicit instantiation of undefined template 'fmt::detail::type_is_unformattable_for<auth::resource_kind, char>'
 2593 |     type_is_unformattable_for<T, char_type> _;
      |                                             ^
/usr/include/fmt/core.h:2656:23: note: in instantiation of function template specialization 'fmt::detail::parse_format_specs<auth::resource_kind, fmt::detail::compile_parse_context<char>>' requested here
 2656 |         parse_funcs_{&parse_format_specs<Args, parse_context_type>...} {}
      |                       ^
/usr/include/fmt/core.h:2787:47: note: in instantiation of member function 'fmt::detail::format_string_checker<char, auth::resource_kind, auth::resource_kind>::format_string_checker' requested here
 2787 |       detail::parse_format_string<true>(str_, checker(s));
      |                                               ^
/home/kefu/dev/scylladb/auth/resource.hh:130:29: note: in instantiation of function template specialization 'fmt::basic_format_string<char, auth::resource_kind &, auth::resource_kind &>::basic_format_string<char[65], 0>' requested here
  130 |             seastar::format("This resource has kind '{}', but was expected to have kind '{}'.", actual, expected)) {
      |                             ^
/usr/include/fmt/core.h:1578:45: note: template is declared here
 1578 | template <typename T, typename Char> struct type_is_unformattable_for;
      |                                             ^
```

in this change, `fmt::formatter<auth::resource_kind>` is moved up to
where `auth::resource_kind` is defined. so that it can be used by its
caller.

Refs #13245

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

Closes scylladb/scylladb#18316
2024-04-23 12:11:17 +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
e2d5054c53 types: do not include unused headers
these unused includes were identified by clangd. see
https://clangd.llvm.org/guides/include-cleaner#unused-include-warning
for more details on the "Unused include" warning.

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

Closes scylladb/scylladb#18326
2024-04-23 12:08:23 +03:00
Pavel Emelyanov
4445ee9a55 Merge 'install-dependencies.sh: add more dependencies for debian' from Kefu Chai
in this changeset, we install `libxxhash-dev` and `cargo` for debian, and install cxxbridge for all distros, so that at least debian can be built without further preparations after running `install-dependencies.sh`.

Closes scylladb/scylladb#18335

* github.com:scylladb/scylladb:
  install-dependencies.sh: move cargo out of fedora branch
  install-dependencies: install cargo and wabt for debian
  install-dependencies.sh: add libxxhash-dev for debian
2024-04-23 12:04:47 +03:00
Lakshmi Narayanan Sreethar
de6570e1ec serializer_impl, sstables: fix build failure due to missing includes
When building scylla with cmake, it fails due to missing includes in
serializer_impl.hh and sstables/compress.hh files. Fix that by adding
the appropriate include files.

Fixes #18343

Signed-off-by: Lakshmi Narayanan Sreethar <lakshmi.sreethar@scylladb.com>

Closes scylladb/scylladb#18344
2024-04-23 12:03:51 +03:00
Kefu Chai
826f413cad thrift: avoid use-after-move in make_non_overlapping_ranges()
in handler.cc, `make_non_overlapping_ranges()` references a moved
instance of `ColumnSlice` when something unexpected happens to
format the error message in an exception, the move constructor of
`ColumnSlice` is default-generated, so the members' move constructors
are used to construct the new instance in the move constructor. this
could lead to undefined behavior when dereferencing the move instance.

in this change, in order to avoid use-after free, let's keep
a copy of the referenced member variables and reference them when
formatting error message in the exception.

this use-after-move issue was introduced in 822a315dfa, which implemented
`get_multi_slice` verb and this piece in the first place. since both 5.2
and 5.4 include this commit, we should backport this change to them.

Refs 822a315dfa
Fixes #18356
Signed-off-by: Kefu Chai <kefu.chai@scylladb.com>

Closes scylladb/scylladb#18358
2024-04-23 12:02:09 +03:00
Kefu Chai
ad2c26824a main: do not reference moved variable
before this change, we dereference `linfo` after moving it away.
and clang-tidy warns us like

```
[19/171] Building CXX object CMakeFiles/scylla.dir/main.cc.o
/home/kefu/dev/scylladb/main.cc:559:12: warning: 'linfo' used after it was moved [bugprone-use-after-move]
  559 |     return linfo.host_id;
      |            ^
/home/kefu/dev/scylladb/main.cc:558:36: note: move occurred here
  558 |     sys_ks.local().save_local_info(std::move(linfo), snitch.local()->get_location(), broadcast_address, broadcast_rpc_address).get();
      |                                    ^
```

the default-generated move constructor of `local_info` uses the
default-generated move constructor of `locator::host_id`, which in turn
use the default-generated move constructor of
`utils::tagged_uuid<struct host_id_tag>`, and then `utils::UUID` 's
move constructor. since `UUID` does not contain any moveable resources,
what it has is but two `int64_t` member variables. so this is a benign
issue. but still, it is distracting.

in this change, we keep the value of `host_id` locally, and return it
instead to silence this warning, and to improve the maintainability.

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

Closes scylladb/scylladb#18362
2024-04-23 11:58:58 +03:00
Patryk Jędrzejczak
14911051ee db: config: introduce force-gossip-topology-changes
We are going to make the `consistent-topology-changes` experimental
feature unused in 6.0. However, the topology upgrade procedure will
be manual and voluntary, so some 6.0 clusters will be using the
gossip-based topology. Therefore, we need to continue testing the
gossip-based topology. The solution is introducing a new flag,
`force-gossip-topology-changes`, that will enforce the gossip-based
topology in a fresh cluster.

In this patch, we only introduce the parameter without any effect.
Here is the explanation. Making `consistent-topology-changes` unused
and introducing `force-gossip-topology-changes` requires adjustments
in scylla-dtest. We want to merge changes to scylladb and scylla-dtest
in a way that ensures all tests are run correctly during the whole
process. If we merged all changes to scylladb first, before merging
the scylla-dtest changes, all tests would run with the raft-based
topology and the ones excluded in the raft-based topology would fail.
We also can't merge all changes to scylla-dtest first. However, we
can follow this plan:
1. scylladb: merge this patch
2. scylla-dtest: start using `force-gossip-topology-changes`
   in jobs that run without the raft-based topology
3. scylladb: merge the rest of the changes
4. scylla-dtest: merge the rest of the changes

Ref scylladb/scylladb#17802

Closes scylladb/scylladb#18284
2024-04-23 09:42:46 +02:00
Botond Dénes
275ed9a9bc replica/mutation_dump: create_underlying_mutation_sources(): remove false move
transformed_cr is moved in a loop, in each iteration. This is harmless
because the variable is const and the move has no effect, yet it is
confusing to readers and triggers false positives in clang-tidy
(moved-from object reused). Remove it.

Fixes: #18322

Closes scylladb/scylladb#18348
2024-04-23 01:21:36 +02:00
Kamil Braun
e9285e5c04 Merge 'various fixes for topology coordinator' from Gleb
The series contains fixes for some problems found during scalability
testing and one clean up patch.

Ref: scylladb/scylladb#17545

* 'gleb/topology-fixes-v4' of github.com:scylladb/scylla-dev:
  gossiper: disable status check for endpoints in raft mode
  storage_service: introduce a setter for topology_change_kind
  topology coordinator: drop unused structure
  storage_service: yield in get_system_mutations
2024-04-22 17:37:47 +02:00
Calle Wilund
82d97da3e0 commitlog: Remove (benign) use-after-move
Fixes #18329

named_file::assign call uses old object "known_size" after a move
of the object. While this is wholly ok, since the attribute accessed
will not be modified/destroyed by the move, it causes warnings in
"tidy" runs, and might confuse or cause real errors should impl. change.

Closes scylladb/scylladb#18337
2024-04-22 17:20:19 +03:00
Kefu Chai
ff04375016 main: drop unused namespace alias
`fs` namespace alias was introduced in ff4d8b6e85, but we don't
use it anymore. so let's drop it.

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

Closes scylladb/scylladb#18308
2024-04-22 13:50:28 +03:00
Nadav Har'El
59b40484c8 Update seastar submodule
* seastar 8fabb30a...2b43417d (6):
  > future: deprecate future::get0()
  > build: do not export valgrind with export()
  > http: deprecate buggy path param[]
  > http/request: add get_path_param method
  > http/request: get_query_param refactor
  > http/util: add path_decode method

Refs #5883 (fixes https://github.com/scylladb/seastar/issues/725 and
provides a new API to read the decoded paths).

Closes scylladb/scylladb#18297
2024-04-22 11:12:49 +03:00
Kefu Chai
85406a450c install-dependencies.sh: move cargo out of fedora branch
so that we install cxxbridge-cmd on all distros, and cxxbridge is
available when building scylladb.

Signed-off-by: Kefu Chai <kefu.chai@scylladb.com>
2024-04-22 15:41:20 +08:00
Kefu Chai
835742af6d install-dependencies: install cargo and wabt for debian
cargo is used for installing cxxbridge-cmd, which is in turn used
when building the cxx bindings for the rust modules. so we need it
on all distros.

in this change, we add cargo for debian. so that we don't have
build failure like:

```
CMake Error at rust/CMakeLists.txt:32 (find_program):
  Could not find CXXBRIDGE using the following names: cxxbridge
```

for similar reason, we also need wabt, which provides wasm2wat,
without which, we'd have

```
CMake Error at test/resource/wasm/CMakeLists.txt:1 (find_program):
  Could not find WASM2WAT using the following names: wasm2wat
```

Signed-off-by: Kefu Chai <kefu.chai@scylladb.com>
2024-04-22 15:41:20 +08:00
Kefu Chai
a70a288627 install-dependencies.sh: add libxxhash-dev for debian
libxxhash is used for building on both fedora and debian. `xxhash-devel`
is already listed in `fedora_packages`, we should have its counterpart
in `debian_base_packages`. otherwise the build on debian and its
derivatives could fail like

```
CMake Error at /usr/local/share/cmake-3.29/Modules/FindPackageHandleStandardArgs.cmake:230 (message):
  Could NOT find xxHash (missing: xxhash_LIBRARY xxhash_INCLUDE_DIR) (found
  version "")
Call Stack (most recent call first):
  /usr/local/share/cmake-3.29/Modules/FindPackageHandleStandardArgs.cmake:600 (_FPHSA_FAILURE_MESSAGE)
  cmake/FindxxHash.cmake:30 (find_package_handle_standard_args)
  CMakeLists.txt:75 (find_package)
```

if we are using CMake to generate the building system. if we use
`configure.py` to generate `build.ninja`, the build would fails at
build time.

Signed-off-by: Kefu Chai <kefu.chai@scylladb.com>
2024-04-22 15:22:51 +08:00
Gleb Natapov
0c77e96b0b storage_service: topology_coordinator: fix indentation after previous patch 2024-04-21 18:53:21 +03:00
Gleb Natapov
b8ee8911ca storage_service: topology coordinator: drop ring check in node_state::replacing state
Always modify topology metadata in node_state::replacing state. There is
no dependency on the ring value at all.
2024-04-21 18:53:04 +03:00
Gleb Natapov
06e6ed09ed gossiper: disable status check for endpoints in raft mode
Gossiper automatically removes endpoints that do not have tokens in
normal state and either do not send gossiper updates or are dead for a
long time. We do not need this with topology coordinator mode since in
this mode the coordinator is responsible to manage the set of nodes in
the cluster. In addition the patch disables quarantined endpoint
maintenance in gossiper in raft mode and uses left node list from the
topology coordinator to ignore updates for nodes that are no longer part
of the topology.
2024-04-21 16:36:07 +03:00
Gleb Natapov
0e3f92fa49 storage_service: introduce a setter for topology_change_kind
In the next patch we will extend it to have other side affects.
2024-04-21 16:36:07 +03:00
Gleb Natapov
040c6ca0c1 topology coordinator: drop unused structure 2024-04-21 16:36:07 +03:00
Gleb Natapov
d0a00f3489 storage_service: yield in get_system_mutations
Yield in a loop that converts a result to canonical_mutation. We
observed stalls for very large tables.
2024-04-21 16:36:07 +03:00
Avi Kivity
87b08c957f Merge 'treewide: drop FMT_DEPRECATED_OSTREAM macro and homebrew range formatters' from Kefu Chai
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

Closes scylladb/scylladb#17968

* github.com:scylladb/scylladb:
  treewide: do not define FMT_DEPRECATED_OSTREAM
  treewide: include fmt/ranges.h and/or fmt/std.h
  utils/managed_bytes: add support for fmt::to_string() to bytes and friends
2024-04-20 22:25:00 +03:00
Mikołaj Grzebieluch
65cfb9b4e0 storage_service: skip wait_for_gossip_to_settle if topology changes are based on raft
Waiting for gossip to settle slows down the bootstrap of the cluster.
It is safe to disable it if the topology is based on Raft.

Fixes scylladb/scylladb#16055

Closes scylladb/scylladb#17960
2024-04-20 17:56:51 +02:00
Pavel Emelyanov
67a408447f snapshot_ctl: Brush up true_snapshots_size() internals
Previous patches broke indentation in this method. Fix it by shortening
the summation loop with the help of std::accumulate()

Signed-off-by: Pavel Emelyanov <xemul@scylladb.com>
2024-04-19 21:06:06 +03:00
Pavel Emelyanov
50add3314d snapshot_ctl: Remove unused details struct
Now the details are manipulated via some other structs and this one can
just be removed

Signed-off-by: Pavel Emelyanov <xemul@scylladb.com>
2024-04-19 20:04:34 +03:00
Pavel Emelyanov
e8f10be12e snapshot_ctl: No double recoding of details
Currently database::get_snapshot_details() returns a collection of
snapshots. The snapshot_ctl converts this collection into similarly
looking one with slightly different structures inside. The resulting
collection is converted one more time on the API layer into another
similarly looking map.

This patch removes the intermediate conversion.

Signed-off-by: Pavel Emelyanov <xemul@scylladb.com>
2024-04-19 20:04:32 +03:00
Pavel Emelyanov
8ec3f057a8 database,snapshots: Move database::snapshot_details into snapshot_ctl
Similarly to how it looks like for table::snapshot_details

Signed-off-by: Pavel Emelyanov <xemul@scylladb.com>
2024-04-19 20:04:29 +03:00
Pavel Emelyanov
f6bc283bbb database,snapshots: Make database::get_snapshot_details() return map, not vector
So that it's in-sync with table::get_snapshot_details(). Next patches
will improve this place even further.

Also, there can be many snapshots and vector can grow large, but that's
less of an issue here.

Signed-off-by: Pavel Emelyanov <xemul@scylladb.com>
2024-04-19 20:04:25 +03:00
Pavel Emelyanov
a36c13beb3 table,snapshots: Move table::snapshot_details into snapshot_ctl
Signed-off-by: Pavel Emelyanov <xemul@scylladb.com>
2024-04-19 19:59:34 +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
5ab527e669 main: do not echo parsed options when calling scylla interactively
in 2f0f53ac, we added logging of parsed command line options so that we
can see how scylla is launched in case it fails to boot. but when scylla
is called interactively in console. this echo is a little bit annoying.
see following console session
```console
$ scylla --help-loggers
Scylla version 5.5.0~dev-0.20240419.3c9651adf297 with build-id 7dd6a110e608535e5c259a03548eda6517ab4bde starting ...
command used: "./RelWithDebInfo/scylla --help-loggers"
pid: 996503
parsed command line options: [help-loggers]
Available loggers:
    BatchStatement
    LeveledManifest
    alter_keyspace
    alter_table
...
```

so in this change, we check if the stdin is associated with a terminal
device, if that the case, we don't print the scylla version, parsed
command line and pid. and the interactive session looks like:

```console
$ scylla --help-loggers
Available loggers:
    BatchStatement
    LeveledManifest
    alter_keyspace
    alter_table
```
no more distracting information printed. the original behavior
can be tested like:

```console
$ : | ./RelWithDebInfo/scylla --help-loggers
```

assuming scylla is always launched with systemd, which connects
stdin to /dev/null. see
https://www.freedesktop.org/software/systemd/man/latest/systemd.exec.html#Logging%20and%20Standard%20Input/Output
. so this behavior is preserved with this change.

Refs #4203

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

Closes scylladb/scylladb#18309
2024-04-19 15:00:05 +03:00
Raphael S. Carvalho
223214439b compaction: Disconsider active tables in the hourly compaction reevaluation
This hourly reevaluation is there to help tablets that have very low
write activity, which can go a long time without flushing a memtable,
and it's important to reevaluate compaction as data can get expired.
Today it can happen that we reevaluate a table that is being compacted
actively, which is waste of cpu as the reevaluation will happen anyway
when there are changes to sstable set. This waste can be amplified with
a significant tablet count in a given shard.
Eventually, we could make the revaluation time per table based on
expiration histogram, but until we get there, let's avoid this waste
by only reevaluating tables that are compaction idle for more than 1h.

Signed-off-by: Raphael S. Carvalho <raphaelsc@scylladb.com>

Closes scylladb/scylladb#18280
2024-04-19 14:33:40 +03:00