Commit Graph

42114 Commits

Author SHA1 Message Date
Pavel Emelyanov
46bbfc0c53 expression: Shorten making raw_value from FragmetedView
The read_field is std::optional<View>. The raw_value::make_value()
accepts managed_bytes_opt which is std::optional<manager_bytes>.
Finally, there's std::optional<T>::optional(std::optional<U>&&)
move constructor (and its copy-constructor peer).

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

Closes scylladb/scylladb#18128
2024-04-01 16:52:18 +03:00
Benny Halevy
01fc1a9f66 schema_tables: std::move mutation into the mutation vector
To save a copy.

Signed-off-by: Benny Halevy <bhalevy@scylladb.com>

Closes scylladb/scylladb#18120
2024-04-01 14:16:30 +03:00
Pavel Emelyanov
5427967f45 schema: Introduce build() && overload
The schema_builder::build() method creates a copy of raw schema
internaly in a hope that builder will be updated and be asked to build
the resulting schema again (e.g. alternator uses this).

However, there are places that build schema using temporary object once
in a `return schema_builder().with_...().build()` manner. For those
invocations copying raw schema is just waste of cycles.

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

Closes scylladb/scylladb#18094
2024-04-01 14:00:42 +03:00
Nadav Har'El
b6854cbb21 Merge 'test/cql-pytest: match error message formated using {fmt} ' from Kefu Chai
currently, our homebrew formatter formats `std::map` like
```
{{k1, v1}, {k2, v2}}
```
while {fmt} formats a map like:
```
{k1: v1, k2: v2}
```
and if the type of key/value is string, {fmt} quotes it, so a

compaction strategy option is formatted like
```
{"max_threshold": "1"}
```
before switching the formatter to the ones supported by {fmt},
let's update the test to match with the new format. this should
reduce the overhead of reviewing the change of switching the
formatter. we can revert this change, and use a simpler approach
after the change of formatter lands.

Closes scylladb/scylladb#18058

* github.com:scylladb/scylladb:
  test/cql-pytest: match error message formated using {fmt}
  test/cql-pytest: extract scylla_error() for not allowed options test
2024-04-01 11:23:24 +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
Pavel Emelyanov
41a1b1c0d0 move_tablets: Emplace mutations into vector, not push
It's more applicable in this case.

Also, built tablets mutations are casted to canonical_mutations, but
when emplaced compiler can pick-up canonical_mutation(const mutation&)
constructor and the cast is not required.

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

Closes scylladb/scylladb#18090
2024-03-29 15:21:49 +02:00
Kamil Braun
f5603ad9ca Merge 'test.py: test_topology_upgrade_basic: make ring_delay_ms nonzero' from Mikołaj Grzebieluch
Test.py uses `ring_delay_ms = 0` by default. CDC creates generation's timestamp by adding `ring_delay_ms` to it.

In this test, nodes are learning about new generations (introduced by upgrade procedure and then by node bootstrap) concurrently with doing writes that should go to these generations.

Because of `ring_delay_ms = 0', the generation could have been committed when it should have already been in use.

This can be seen in the following logs from a node:
```
ERROR 2024-03-22 12:29:55,431 [shard 0:strm] cdc - just learned about a CDC generation newer than the one used the last time streams were retrieved. This generation, or some newer one, should have been used instead (new generation's timestamp: 2024/03/22 12:29:54, last time streams were retrieved: 2024/03/22 12:29:55). The new generation probably arrived too late due to a network partition and we've made a write using the wrong set streams.
```

Creating writes during such a generation can result in assigning them a wrong generation or a failure. Failure may occur if it hits short time window when `generation_service::handle_cdc_generation(cdc::generation_id_v2)` has executed
`svc._cdc_metadata.prepare(...)` but`_cdc_metadata.insert(...)` has not yet been executed. With a nonzero ring_delay_ms it's not a problem, because during this time window, the generation should not be in use.

Write can fail with the following response from a node:
```
cdc: attempted to get a stream from a generation that we know about, but weren't able to retrieve (generation timestamp: 2024/03/22 12:29:54, write timestamp: 2024/03/22 12:29:55). Make sure that the replicas which contain this generation's data are alive and reachable from this node.
```

Set ring_delay_ms to 15000 for the debug mode and 5000 in other modes. Wait for the last generation to be in use and sleep one second to make sure there are writes to the CDC table in this generation.

Fixes scylladb/scylladb#17977

Reapply b4144d14c6.

Closes scylladb/scylladb#17998

* github.com:scylladb/scylladb:
  test.py: test_topology_upgrade_basic: make ring_delay_ms nonzero
  Reapply "test.py: adjust the test for topology upgrade to write to and read from CDC tables"
2024-03-29 12:52:31 +01:00
Tzach Livyatan
4930095d39 Docs: Fix link fro scylla-sstable.rst to /architecture/sstable/
Fix https://github.com/scylladb/scylladb/issues/18096

Closes scylladb/scylladb#18097
2024-03-29 10:48:24 +02:00
Piotr Dulikowski
57719ece4f Merge 'main: reload service levels data accessor after join_cluster' from Marcin Maliszkiewicz
Setting data accessor implicitly depends on node joining the cluster
with raft leader elected as only then service level mutation is put
into scylla_local table. Calling it after join_cluster avoids starting
new cluster with older version only to immediately migrate it to the
latest one in the background.

Closes scylladb/scylladb#18040

* github.com:scylladb/scylladb:
  main: reload service levels data accessor after join_cluster
  service: qos: create separate function for reloading data accessor
2024-03-29 09:39:11 +01:00
Kefu Chai
1632fbbef9 test/cql-pytest: match error message formated using {fmt}
currently, our homebrew formatter formats `std::map` like

{{k1, v1}, {k2, v2}}

while {fmt} formats a map like:

{k1: v1, k2: v2}

and if the type of key/value is string, {fmt} quotes it, so a

compaction strategy option is formatted like

{"max_threshold": "1"}

before switching the formatter to the ones supported by {fmt},
let's update the test to match with the new format. this should
reduce the overhead of reviewing the change of switching the
formatter. we can revert this change, and use a simpler approach
after the change of formatter lands.

Refs #13245

Signed-off-by: Kefu Chai <kefu.chai@scylladb.com>
2024-03-29 08:07:59 +08:00
Kefu Chai
8f47fcedf6 test/cql-pytest: extract scylla_error() for not allowed options test
currently, our homebrew formatter formats `std::map` like

{{k1, v1}, {k2, v2}}

while {fmt} formats a map like:

{k1: v1, k2: v2}

and if the type of key/value is string, {fmt} quotes it, so a

compaction strategy option is formatted like

{"max_threshold": "1"}

as we are switching to the formatters provided by {fmt}, would be
better to support its convention directly.

so, in this change, to prepare the change, before migrating to
{fmt}, let's refactor the test to support both formats by
extracting a helper to format the error message, so that we can
change it to emit both formats.

Refs #13245

Signed-off-by: Kefu Chai <kefu.chai@scylladb.com>
2024-03-29 08:03:02 +08:00
Mikołaj Grzebieluch
1e2607563f test.py: test_topology_upgrade_basic: make ring_delay_ms nonzero
Test.py uses `ring_delay_ms = 0` by default. CDC creates generation's timestamp
by adding `ring_delay_ms` to it.

In this test, nodes are learning about new generations (introduced by upgrade
procedure and then by node bootstrap) concurrently with doing writes that
should go to these generations.

Because of `ring_delay_ms = 0', the generation could have been committed when
it should have already been in use.

This can be seen in the following logs from a node:
```
ERROR 2024-03-22 12:29:55,431 [shard 0:strm] cdc - just learned about a CDC generation newer than the one used the last time streams were retrieved. This generation, or some newer one, should have been used instead (new generation's timestamp: 2024/03/22 12:29:54, last time streams were retrieved: 2024/03/22 12:29:55). The new generation probably arrived too late due to a network partition and we've made a write using the wrong set streams.
```

Creating writes during such a generation can result in assigning them a wrong
generation or a failure. Failure may occur if it hits short time window when
`generation_service::handle_cdc_generation(cdc::generation_id_v2)` has executed
`svc._cdc_metadata.prepare(...)` but`_cdc_metadata.insert(...)` has not yet
been executed. With a nonzero ring_delay_ms it's not a problem, because during
this time window, the generation should not be in use.

Write can fail with the following response from a node:
```
cdc: attempted to get a stream from a generation that we know about, but weren't able to retrieve (generation timestamp: 2024/03/22 12:29:54, write timestamp: 2024/03/22 12:29:55). Make sure that the replicas which contain this generation's data are alive and reachable from this node.
```

Set ring_delay_ms to 15000 for the debug mode and 5000 in other modes.
Wait for the last generation to be in use and sleep one second to make sure
there are writes to the CDC table in this generation.

Fixes #17977
2024-03-28 17:13:43 +01:00
Botond Dénes
4c0dadee7c Merge 'test: changes to prepare for dropping FMT_DEPRECATED_OSTREAM' from Kefu Chai
this series includes test related changes to enable us to drop `FMT_DEPRECATED_OSTREAM` deprecated in {fmt} v10.

Refs #13245

Closes scylladb/scylladb#18054

* github.com:scylladb/scylladb:
  test: unit: add fmt::formatter for test_data in tests
  test/lib: do not print with fmt::to_string()
  test/boost: print runtime_error using e.what()
2024-03-28 15:33:56 +02:00
Kamil Braun
33751f8f4e Merge 'raft topology: drop RAFT_PULL_TOPOLOGY_SNAPSHOT RPC' from Gleb
* 'gleb/raft_snapshot_rpc-v3' of github.com:scylladb/scylla-dev:
  raft topology: drop RAFT_PULL_TOPOLOGY_SNAPSHOT RPC
  Use correct limit for raft commands throughout the code.
2024-03-28 14:25:58 +01:00
Nadav Har'El
566223c34a Merge ' tools/scylla-nodetool: repair: abort on first failed repair' from Botond Dénes
When repairing multiple keyspaces, bail out on the first failed keyspace repair, instead of continuing and reporting all failures at the end. This is what Origin does as well.

To be able to test this, a bit of refactoring was needed, to be able to assert that `scylla-nodetool` doesn't make repair requests, beyond the expected ones.

Refs: https://github.com/scylladb/scylla-cluster-tests/issues/7226

Closes scylladb/scylladb#17678

* github.com:scylladb/scylladb:
  tools/scylla-nodetool: repair: abort on first failed repair
  test/nodetool: nodetool(): add check_return_code param
  test/nodetool: nodetool(): return res object instead of just stdout
  test/nodetool: count unexpected requests
2024-03-28 14:02:29 +02:00
Botond Dénes
81bbfae77a tools/scylla-nodetool: implement the checkAndRepairCdcStreams command
Closes scylladb/scylladb#18076
2024-03-28 13:54:37 +02:00
Pavel Emelyanov
1adf16ce73 Merge 'network_topology_strategy: reallocate_tablets: support for rf changes' from Benny Halevy
This series provides a reallocate_tablets function, that's initially called by allocate_tablets_for_new_table.
The new allocation implementation is independent of vnodes/token ownership.
Rather than using the natural_endpoints_tracker, it implements its own tracking
based on dc/rack load (== number of replicas in rack), with the additional benefit
that tablet allocation will balance the allocation across racks, using a heap structure,
similar to the one we use to balance tablet allocation across shards in each node.

reallocate_tablets may also be called with an optional parameter pointing the the current tablet_map.
In this case the function either allocates more tablet replicas in datacenters for which the replication factor was increased,
or it will deallocate tablet replicas from datacenters for which replication factor was decreased.

The NetworkTopologyStrategy_tablets_test unit test was extended to cover replication factor changes.

Closes scylladb/scylladb#17846

* github.com:scylladb/scylladb:
  network_topology_strategy: reallocate_tablets: consider new_racks before existing racks
  network_topology_startegy_test: add NetworkTopologyStrategy_tablet_allocation_balancing_test
  network_topology_strategy: reallocate_tablets: support deallocation via rf change
  network_topology_startegy_test: tablets_test: randomize cases
  network_topology_strategy: allocate_tablets_for_new_table: do not rely on token ownership
  network_topology_startegy_test: add NetworkTopologyStrategy_tablets_negative_test
  network_topology_strategy_test: endpoints_check: use particular BOOST_CHECK_* functions
  network_topology_strategy_test: endpoints_check: verify that replicas are placed on unique nodes
  network_topology_strategy_test: endpoints_check: strictly check rf for tablets
  network_topology_strategy_test: full_ring_check for tablets: drop unused options param
2024-03-28 11:19:11 +03:00
Kefu Chai
2bfc7324d4 mutation: friend fmt::formatter<atomic_cell> in atomic_cell_view
GCC-14 rightly points out that the constructor of `atomic_cell_view`
is marked private, and cannot be called from its formatter:
```
/usr/bin/g++-14 -DDEBUG -DDEBUG_LSA_SANITIZER -DFMT_SHARED -DSANITIZE -DSCYLLA_BUILD_MODE=debug -DSCYLLA_ENABLE_ERROR_INJECTION -DSEASTAR_API_LEVEL=7 -DSEASTAR_DEBUG -DSEASTAR_DEBUG_PROMISE -DSEASTAR_DEBUG_SHARED_PTR -DSEASTAR_DEFAULT_ALLOCATOR -DSEASTAR_LOGGER_COMPILE_TIME_FMT -DSEASTAR_LOGGER_TYPE_STDOUT -DSEASTAR_SCHEDULING_GROUPS_COUNT=16 -DSEASTAR_SHUFFLE_TASK_QUEUE -DSEASTAR_SSTRING -DSEASTAR_TYPE_ERASE_MORE -DXXH_PRIVATE_API -DCMAKE_INTDIR=\"Debug\" -I/var/ssd/scylladb -I/var/ssd/scylladb/build/gen -I/var/ssd/scylladb/seastar/include -I/var/ssd/scylladb/build/seastar/gen/include -I/var/ssd/scylladb/build/seastar/gen/src -g -Og -g -gz -std=gnu++20 -fvisibility=hidden -Wall -Werror -Wextra -Wno-error=deprecated-declarations -Wimplicit-fallthrough -Wno-deprecated-copy -Wno-mismatched-tags -Wno-missing-field-initializers -Wno-overloaded-virtual -Wno-unused-parameter -ffile-prefix-map=/var/ssd/scylladb=. -march=westmere -Wstack-usage=40960 -U_FORTIFY_SOURCE -Wno-maybe-uninitialized -Werror=unused-result -fstack-clash-protection -fsanitize=address -fsanitize=undefined -fno-sanitize=vptr -MD -MT mutation/CMakeFiles/mutation.dir/Debug/atomic_cell.cc.o -MF mutation/CMakeFiles/mutation.dir/Debug/atomic_cell.cc.o.d -o mutation/CMakeFiles/mutation.dir/Debug/atomic_cell.cc.o -c /var/ssd/scylladb/mutation/atomic_cell.cc
In file included from /var/ssd/scylladb/mutation/atomic_cell.cc:9:
/var/ssd/scylladb/mutation/atomic_cell.hh: In member function ‘auto fmt::v10::formatter<atomic_cell>::format(const atomic_cell&, fmt::v10::format_context&) const’:
/var/ssd/scylladb/mutation/atomic_cell.hh:413:67: error: ‘atomic_cell_view::atomic_cell_view(basic_atomic_cell_view<is_mutable>) [with mutable_view is_mutable = mutable_view::yes]’ is private within this context
  413 |         return fmt::format_to(ctx.out(), "{}", atomic_cell_view(ac));
      |                                                                   ^
/var/ssd/scylladb/mutation/atomic_cell.hh:275:5: note: declared private here
  275 |     atomic_cell_view(basic_atomic_cell_view<is_mutable> view)
      |     ^~~~~~~~~~~~~~~~
```
so, in this change, we make the formatter a friend of
`atomic_cell_view`.
since the operator<< was dropped, there is no need to keep its friend
declaration around, so it is dropped in this change.

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

Closes scylladb/scylladb#18081
2024-03-28 09:44:00 +02:00
Kefu Chai
99e743de9d test: nodetool: match with vector printed by {fmt}
our homebrew formatter for std::vector<string> formats like

```
{hello, world}
```

while {fmt}'s formatter for sequence-like container formats like

```
["hello", "world"]
```

since we are moving to {fmt} formatters. and in this context,
quoting the verbatim text makes more sense to user. let's
support the format used by {fmt} as well.

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

Closes scylladb/scylladb#18057
2024-03-28 09:35:37 +02:00
Kefu Chai
c2ffa0d813 bytes.hh: stop at '}' in fmt::formatter<fmt_hex>
according to {fmt}'s document at
https://fmt.dev/latest/api.html#formatting-user-defined-types,

```
  // the range will contain "f} continued". The formatter should parse
  // specifiers until '}' or the end of the range. In this example the
  // formatter should parse the 'f' specifier and return an iterator
  // pointing to '}'.
```

so we should check for _both_ '}' and end of the range. when building
scylla with {fmt} 10.2.1, it fails to build code like

```c++
fmt::format_to(out, "{}", fmt_hex(frag))
```

as {fmt}'s compile-time checker fails to parse this format string
along with given argument, as at compile time,
```c++
throw format_error("invalid group_size")
```
is executed.

so, in this change, we check both '}' and the end of range.

the change which introduced this formatter was
2f9dfba800

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

Closes scylladb/scylladb#18080
2024-03-28 08:58:36 +02:00
Marcin Maliszkiewicz
50e0032bca test: auth: remove if not exists from auth cql statement
They were added due to https://github.com/scylladb/python-driver/issues/296
but looks like it no longer reproduces.

Change was tested with ./test.py -vv --repeat=100 test_auth
to minimize chance of introducing flakiness.

Closes scylladb/scylladb#18043
2024-03-28 06:06:45 +01:00
Raphael S. Carvalho
902c71bac8 storage_service: Fix undefined behavior in stream_tablet()
correctness when constructing range_streamer depends on compiler
evaluation order of params.

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

Closes scylladb/scylladb#18079
2024-03-27 23:50:37 +01:00
Gleb Natapov
6e6aefc9ab raft topology: drop RAFT_PULL_TOPOLOGY_SNAPSHOT RPC
We have new, more generic, RPC to pull group0 mutations now: RAFT_PULL_SNAPSHOT.
Use it instead of more specific RAFT_PULL_TOPOLOGY_SNAPSHOT one.
2024-03-27 19:18:45 +02:00
Gleb Natapov
c1dcf0fae7 Use correct limit for raft commands throughout the code.
Raft uses schema commitlog, so all its limits should be derived from
this commitlog segment size, but many places used regular commitlog size
to calculate the limits and did not do what they really suppose to be
doing.
2024-03-27 19:16:09 +02:00
Kamil Braun
c3989d8e03 Merge 'storage_service: keep subscription to raft topology feature alive' from Piotr Dulikowski
The storage_service::track_upgrade_progress_to_topology_coordinator
function is supposed to wait on the SUPPORTS_CONSISTENT_TOPOLOGY_CHANGES
cluster feature (among other things) before starting the
raft_state_monitor_fiber. The wait is realized by passing a callback to
feature::when_enabled which sets a shared_promise that is waited on by
the tracking fiber. If the feature is already enabled, when_enabled will
call the callback immediately. However, if it's not, then it will return
a non-null listener_registration object - as long as it is alive, the
callback is registered. The listener_registration object was not
assigned to a variable which caused it to be destroyed shortly after the
when_enabled function returns.

Due to that, if upgrade was requested but the current group0 leader
didn't have the SUPPORTS_CONSISTENT_TOPOLOGY_CHANGES feature enabled
right after boot, the upgrade would not start until the leader is
changed to a node which has that cluster feature already enabled on
boot. Moreover, the topology coordinator would not start on such a node
until the node were rebooted.

Fix the issue by assigning the subscription to a variable.

Fixes: scylladb/scylladb#18049

Closes scylladb/scylladb#18051

* github.com:scylladb/scylladb:
  gms: feature: mark when_enabled(func) with nodiscard
  storage_service: keep subscription to raft topology feature alive
2024-03-27 14:46:43 +01:00
Avi Kivity
96a3544739 Merge 'alternator: reduce stall for Query and Scan with large pages' from Nadav Har'El
Before this series, Alternator's Query and Scan operations convert an
entire result page to JSON without yielding. For a page of maximum
size (1MB) and tiny rows, this can cause a significant stall - the
test included in this PR reported stalls of 14-26ms on my laptop.

The problem is the describe_items() function, which does this conversion
immediately, without yielding. This patch changes this function to
return a future, and use a new result_set::visit_gently() method
that does what visit() does, but with yields when needed.

This PR improves #17995, but does not completely fix is as the stalls in the
are not completely eliminated. But on my laptop it usually reduces the stalls
to around 5ms. It appears that the remaining stalls some from other places
not fixed in this PR, such as perhaps query_page::handle_result(), and will need
to be fixed by additional patches.

Closes scylladb/scylladb#18036

* github.com:scylladb/scylladb:
  alternator: reduce stall for Query and Scan with large pages
  result_set: introduce visit_gently()
  alternator: coroutinize do_query() function
2024-03-27 15:06:32 +02:00
Kamil Braun
404406e6a1 Merge ' test/cql-pytest: test_select_from_mutation_fragments.py: move away from memtables' from Botond Dénes
Memtables are fickle, they can be flushed when there is memory pressure,
if there is too much commitlog or if there is too much data in them. The
tests in test_select_from_mutation_fragments.py currently assume data
written is in the memtable. This is tru most of the time but we have
seen some odd test failures that couldn't be understood.  To make the
tests more robust, flush the data to the disk and read it from the
sstables. This means that some range scans need to filter to read from
just a single mutation source, but this does not influence the tests.
Also fix a use-after-return found when modifying the tests.

This PR tentatively fixes the below issues, based on our best guesses on why they failed (each was seen just once):
Fixes: scylladb/scylladb#16795
Fixes: scylladb/scylladb#17031

Closes scylladb/scylladb#17562

* github.com:scylladb/scylladb:
  test/cql-pytest: test_select_from_mutation_fragments.py: move away from memtables
  cql3: select_statement: mutation_fragments_select_statement: fix use-after-return
2024-03-27 13:21:19 +01:00
Botond Dénes
fdd5367974 Merge 'compaction: implement unchecked_tombstone_compaction' from Ferenc Szili
This change adds the missing Cassandra compaction option unchecked_tombstone_compaction.
Setting this option to true causes the compaction to ignore tombstone_threshold, and decide whether to do a compaction only based on the value of tombstone_compaction_interval

Fixes #1487

Closes scylladb/scylladb#17976

* github.com:scylladb/scylladb:
  removed forward declaration of resharding_descriptor
  compaction options and troubleshooting docs
  cql-pytest/test_compaction_strategy_validation.py
  test/boost/sstable_compaction_test.cc
  compaction: implement unchecked_tombstone_compaction
2024-03-27 13:56:02 +02:00
Kefu Chai
6bd0be71ab mutation: add fmt::formatter for invalid_mutation_fragment_stream
before this change, we rely on the default-generated fmt::formatter
created from operator<<. but this depends on the
`FMT_DEPRECATED_OSTREAM` macro which is not respected in {fmt} v10.

this change addresses the formatting with fmtlib < 10, and without
`FMT_DEPRECATED_OSTREAM` defined. please note, in {fmt} v10 and up,
it defines formatter for classes derived from `std::exception`, so
our formatter is only added when compiled with {fmt} < 10.

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

Refs scylladb#13245

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

Closes scylladb/scylladb#18053
2024-03-27 13:37:48 +02:00
Kefu Chai
d1e8d89ae2 doc: topology-over-raft: add transition_state to node state diagram
in order to help the developers to understand the transitions
of `node_state` and the `transition_state` on each of the `node_state`,
in this change, the nested state machine diagram is added to the
node state diagram.

please note, instead of trying to merge similar states like
bootstrapping and replacing into a single state, we keep them as
separate ones, and replicate the nested state machine diagram in them
as well, to be more clear.

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

Closes scylladb/scylladb#18025
2024-03-27 12:16:35 +01:00
Andrei Chekun
0752ef1481 test: remove skip annotation for multi-DC test with 5 DCs with one node in each
As a follow-up of the https://github.com/scylladb/scylladb/pull/17503 remove skip annotation for the multi-DC test with a reduced amount of the DC used in it: from 30 DCs to 5 DCs

Closes scylladb/scylladb#17898
2024-03-27 13:13:13 +02:00
Michał Chojnowski
295b27a07b cache_flat_mutation_reader: only call get_iterator_in_latest() when pointing at a row
Calling `_next_row.get_iterator_in_latest()` is illegal when `_next_row` is not
pointing at a row. In particular, the iterator returned by such call might be
dangling.

We have observed this to cause a use-after-free in the field, when a reverse
read called `maybe_add_to_cache` after `_latest_it` was left dangling after
a dead row removal in `copy_from_cache_to_buffer`.

To fix this, we should ensure that we only call `_next_row.get_iterator_in_latest`
is pointing at a row.

Only the occurrences of this problem in `maybe_add_to_cache` are truly dangerous.
As far as I can see, other occurrences can't break anything as of now.
But we apply fixes to them anyway.

Closes scylladb/scylladb#18046
2024-03-27 11:48:42 +01:00
Kamil Braun
d274f63d89 Merge 'Add support for "initial-token" parameter in raft mode' from Gleb
Fixes scylladb/scylladb#17893

* 'gleb/initial-token-v1' of github.com:scylladb/scylla-dev:
  dht: drop unused parameter from get_random_bootstrap_tokens() function
  test: add test for initial_token parameter
  topology coordinator: use provided initial_token parameter to choose bootstrap tokens
  topology cooordinator: propagate initial_token option to the coordinator
2024-03-27 11:41:06 +01:00
Kefu Chai
71a519dee8 test: unit: add fmt::formatter for test_data in tests
this change is created in same spirit of d1c35f943d.

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 test_data in
radix_tree_stress_test.cc, and drop its operator<<.

Refs #13245

Signed-off-by: Kefu Chai <kefu.chai@scylladb.com>
2024-03-27 18:18:32 +08:00
Kefu Chai
4f8c1a4729 test/lib: do not print with fmt::to_string()
we should not format a variable unless we want to print it. in this
case, we format `first_row` using `fmt::to_string()` to a string,
and then insert the string to another string, despite that this is
in a cold path, this is still a anti pattern -- both convoluted,
and not performant.

so let's just pass `first_row` to `format()`.

Refs #13245

Signed-off-by: Kefu Chai <kefu.chai@scylladb.com>
2024-03-27 18:18:32 +08:00
Kefu Chai
d0ceb35e7e test/boost: print runtime_error using e.what()
before this change, we rely on the default-generated fmt::formatter
created from operator<<, but fmt v10 dropped the default-generated
formatter. but fortunately, fmt v10 brings the builtin
formatter for classes derived from `std::exception`. but before
switching to {fmt} v10, and after dropping `FMT_DEPRECATED_OSTREAM`
macro, we need to print out `std::runtime_error`. so far, we don't
have a shared place for formatter for `std::runtime_error`. so we
are addressing the needs on a case-by-case basis.

in this change, we just print it using `e.what()`. it's behavior
is identical to what we have now.

Refs #13245

Signed-off-by: Kefu Chai <kefu.chai@scylladb.com>
2024-03-27 18:18:32 +08:00
Benny Halevy
8a77319cb7 network_topology_strategy: reallocate_tablets: consider new_racks before existing racks
Allocate first from new (unpopulated) racks before
allocating from racks that are already populated
with replicas.

Still, rotate both new and existing racks by tablet id
to ensure fairness.

Signed-off-by: Benny Halevy <bhalevy@scylladb.com>
2024-03-27 12:06:24 +02:00
Benny Halevy
c5ff060dee network_topology_startegy_test: add NetworkTopologyStrategy_tablet_allocation_balancing_test
Test that tablet allocation is balanced across
racks, nodes, and shards.

Signed-off-by: Benny Halevy <bhalevy@scylladb.com>
2024-03-27 12:06:24 +02:00
Benny Halevy
4a7d57525e network_topology_strategy: reallocate_tablets: support deallocation via rf change
Add support for deallocating tablet replicas when the
datacenter replication factor is decreased.

We deallocate replicas back-to-front order to maintain
replica pairing between the base table and
its materialized views.

Signed-off-by: Benny Halevy <bhalevy@scylladb.com>
2024-03-27 12:06:24 +02:00
Benny Halevy
1e8f8db5b8 network_topology_startegy_test: tablets_test: randomize cases
Instead of deterministically testing a very small set of cases,
randomize the the shard_count per node, the cluster topology
and the NetworkTopologyStrategy options.

The next patch will extend the test to also test
`reallocate_tablets` with randomized options.

Signed-off-by: Benny Halevy <bhalevy@scylladb.com>
2024-03-27 12:06:24 +02:00
Benny Halevy
898cd1d404 network_topology_strategy: allocate_tablets_for_new_table: do not rely on token ownership
Base initial tablets allocation for new table
on the dc/rack topology, rather then on the token ring,
to remove the dependency on token ownership.

We keep the rack ordinal order in each dc
to facilitate in-rack pairing of base/view
replica pairing, and we apply load-balancing
principles by sorting the nodes in each rack
by their load (number of tablets allocated to
the node), and attempting to fill lease-loaded
nodes first.

This method is more efficient than circling
the token ring and attemting to insert the endpoints
to the natural_endpoint_tracker until the replication
factor per dc is fulfilled, and it allows an easier
way to incrementally allocate more replicas after
rf is increased.

Signed-off-by: Benny Halevy <bhalevy@scylladb.com>
2024-03-27 12:06:21 +02:00
Botond Dénes
f70f04c240 tools/scylla-nodetool: repair: abort on first failed repair
When repairing multiple keyspaces, bail out on the first failed keyspace
repair, instead of continuing and reporting all failures at the end.
This is what Origin does as well.
2024-03-27 05:46:18 -04:00
Mikołaj Grzebieluch
fa4193e09f Reapply "test.py: adjust the test for topology upgrade to write to and read from CDC tables"
This reverts commit 230f23004b.
2024-03-27 10:39:01 +01:00
Benny Halevy
40a4b349bd network_topology_startegy_test: add NetworkTopologyStrategy_tablets_negative_test
Test that we attempting to allocate tablets
throws an error when there are not enough nodes
for the configured replication factor.

Signed-off-by: Benny Halevy <bhalevy@scylladb.com>
2024-03-27 10:35:04 +02:00
Benny Halevy
f19dbb4ae5 network_topology_strategy_test: endpoints_check: use particular BOOST_CHECK_* functions
Using e.g. `BOOST_CHECK_EQUAL(endpoints.size(), total_rf)`
rather than `BOOST_CHECK(endpoints.size() == total_rf)`
prints a more detailed error message that includes the
runtime valies, if it fails.

Signed-off-by: Benny Halevy <bhalevy@scylladb.com>
2024-03-27 10:35:04 +02:00
Benny Halevy
93b6573a90 network_topology_strategy_test: endpoints_check: verify that replicas are placed on unique nodes
Signed-off-by: Benny Halevy <bhalevy@scylladb.com>
2024-03-27 10:35:04 +02:00
Benny Halevy
c11ffd14cc network_topology_strategy_test: endpoints_check: strictly check rf for tablets
With tablet we want to verify that the number of
replicas allocated per tablet per dc exactly matches
the replication strategy per-dc replication factor options.

Signed-off-by: Benny Halevy <bhalevy@scylladb.com>
2024-03-27 10:35:04 +02:00
Benny Halevy
ffa5870758 network_topology_strategy_test: full_ring_check for tablets: drop unused options param
Signed-off-by: Benny Halevy <bhalevy@scylladb.com>
2024-03-27 10:35:04 +02:00
Botond Dénes
764e9a344d test/nodetool: nodetool(): add check_return_code param
When set to false, the returncode is not checked, this is left to the
caller. This in turn allows for checking the expected and unexpected
requests which is not checked when the nodetool process fails.
This is used by utils._do_check_nodetool_fails_with(), so that expected
and unexpected requests are checked even for failed invocations.

Some test need adjustment to the stricter checks.
2024-03-27 04:18:19 -04:00