The test is booting nodes, and then immediately starts shutting down
nodes and removing them from the cluster. The shutting down and
removing may happen before driver manages to connect to all nodes in the
cluster. In particular, the driver didn't yet connect to the last
bootstrapped node. Or it can even happen that the driver has connected,
but the control connection is established to the first node, and the
driver fetched topology from the first node when the first node didn't
yet consider the last node to be normal. So the driver decides to close
connection to the last node like this:
```
22:34:03.159 DEBUG> [control connection] Removing host not found in
peers metadata: <Host: 127.42.90.14:9042 datacenter1>
```
Eventually, at the end of the test, only the last node remains, all
other nodes have been removed or stopped. But the driver does not have a
connection to that last node.
Fix this problem by ensuring that:
- all nodes see each other as NORMAL,
- the driver has connected to all nodes
at the beginning of the test, before we start shutting down and removing
nodes.
Fixesscylladb/scylladb#16373Closesscylladb/scylladb#17676
Scylla-ccm uses function `wait_for_binary_interface` that waits for
scylla logs to print "Starting listening for CQL clients". If this log
is printed far before the regular cql_controller is initialized,
scylla-ccm assumes too early that node is initialized.
It can result in timeouts that throw errors, for example in the function
`watch_rest_for_alive`.
Closesscylladb/scylladb#17496
The following incompatibilities were identified by `listsnapshots_test.py` in dtests:
* Command doesn't bail out when there are no snapshots, instead it prints meaningless empty report
* Formatting is incompatible
Both are fixed in this mini-series.
Closesscylladb/scylladb#17541
* github.com:scylladb/scylladb:
tools/scylla-nodetool: listsnapshots: make the formatting compatible with origin's
tools/scylla-nodetool: listsnapshots: bail out if there are no snapshots
It might happen that multiple tablets co-habit the same shard, so we want load-and-stream to jump into a new streaming session for every tablet, such that the receiver will have the data properly segregated. That's a similar treatment we gave to repair. Today, load-and-stream fails due to sstables spanning more than 1 tablet in the receiver.
Synchronization with migration is done by taking replication map, so migrations cannot advance while streaming new data. A bug was fixed too, where data must be streamed to pending replicas too, to handle case where migration is ongoing and new data must reach both old and new replica set. A test was added stressing this synchronization path.
Another bug was fixed in sstable loading, which expected sharder to not be invalidated throughout the operation, but that breaks during migrations.
Fixes#17315.
Closesscylladb/scylladb#17449
* github.com:scylladb/scylladb:
test: test_tablets: Add load-and-stream test
sstables_loader: Stream to pending tablet replica if needed
sstables_loader: Implement tablet based load-and-stream
sstables_loader: Virtualize sstable_streamer for tablet
sstables_loader: Avoid reallocations in vector
sstable_loader: Decouple sstable streaming from selection
sstables_loader: Introduce sstable_streamer
Fix online SSTable loading with concurrent tablet migration
This one-line patch fixes a failure in the dtest
lwt_schema_modification_test.py::TestLWTSchemaModification
::test_table_alter_delete
Where an update sometimes failed due to an internal server error, and the
log had the mysterious warning message:
"std::logic_error (Empty materialized view updated)"
We've also seen this log-message in the past in another user's log, and
never understood what it meant.
It turns out that the error message was generated (and warning printed)
while building view updates for a base-table mutation, and noticing that
the base mutation contains an *empty* row - a row with no cells or
tombstone or anything whatsoever. This case was deemed (8 years ago,
in d5a61a8c48) unexpected and nonsensical,
and we threw an exception. But this case actually *can* happen - here is
how it happened in test_table_alter_delete - which is a test involving
a strange combination of materialized views, LWT and schema changes:
1. A table has a materialized view, and also a regular column "int_col".
2. A background thread repeatedly drops and re-creates this column
int_col.
3. Another thread deletes rows with LWT ("IF EXISTS").
4. These LWT operations each reads the existing row, and because of
repeated drop-and-recreate of the "int_col" column, sometimes this
read notices that one node has a value for int_col and the other
doesn't, and creates a read-repair mutation setting int_col (the
difference between the two reads includes just in this column).
5. The node missing "int_col" receives this mutation which sets only
int_col. It upgrade()s this mutation to its most recent schema,
which doesn't have int_col, so it removes this column from the
mutation row - and is left with a completely empty mutation row.
This completely empty row is not useful, but upgrade() doesn't
remove it.
6. The view-update generation code sees this empty base-mutation row
and fails it with this std::logic_error.
7. The node which sent the read-repair mutation sees that the read
repair failed, so it fails the read and therefore fails the LWT
delete operation.
It is this LWT operation which failed in the test, and caused
the whole test to fail.
The fix is trivial: an empty base-table row mutation should simply be
*ignored* when generating view updates - it shouldn't cause any error.
Before this patch, test_table_alter_delete used to fail in roughly
20% of the runs on my laptop. After this patch, I ran it 100 times
without a single failure.
Fixes#15228Fixes#17549
Signed-off-by: Nadav Har'El <nyh@scylladb.com>
Closesscylladb/scylladb#17607
When no keyspace is provided, request all keyspaces from the server,
then scrub all of them. This is what the legacy nodetool does, for some
reason this was missed when re-implementing scrub.
Closesscylladb/scylladb#17495
There are 4 barrier-only stages when migrating a tablet and the test needs to fail pending/leaving replica that handles it in order to validate how coordinator handles dead node. Failing the barrier is done by suspending it with injection code and stopping the node without waking it up. The main difficulty here is how to tell one barrier RPC call from another, because they don't have anything onboard that could tell which stage the barrier is run for. This PR suggests that barrier injection code looks directly into the system.tablets table for the transition stage, the stage is already there by the time barrier is about to ack itself over RPC.
refs: #16527Closesscylladb/scylladb#17450
* github.com:scylladb/scylladb:
topology.tablets_migration: Handle failed use_new
topology.tablets_migration: Handle failed write_both_read_new
topology.tablets_migration: Handle failed write_both_read_old
topology.tablets_migration: Handle failed allow_write_both_read_old
test/tablets_migration: Add conditional break-point into barrier handler
replica: Add helper to read tablet transition stage
topology_coordinator: Add action_failed() helper
The author (me) tried to be clever and fix the formatting, but then he
realized this just means a lot of unnecessary fighting with tests. So
this patch makes the formatting compatible with that of the legacy
nodetool:
* Use compatible rounding and precision formatting
* Use incorrect unit (KB instead of KiB)
* Align numbers to the left
* Add trailing white-space to "Snapshot Details: "
These two parameters are not used by the native nodetool, because
ScyllaDB itself doesn't support them. These should be just ignored and
indeed there was a unit test checking that this is the case. However,
due to a mistake in the unit test, this was not actually tested and
nodetool complained when seeing these params.
This patch fixes both the test and the native nodetool.
Closesscylladb/scylladb#17477
this changeset addresses some warnings raised by flake8 in hope to improve the readability of this script in general.
Closesscylladb/scylladb#17668
* github.com:scylladb/scylladb:
scylla-gdb: s/if not foo is None/if foo is not None/
scylla-gdb.py: add space after keyword
scylla-gdb.py: remove extraneous spaces
scylla-gdb.py: use 2 empty lines between top-level funcs/classes
scylla-gdb.py: replace <tab> with 4 spaces
scylla-gdb: fix the indent
Currently, the github docs-pages workflow is triggered only when changes are merged to the master/enterprise branches, which means that in the case of changes to a release branch, for example, a fix to branch-5.4, or a branch-5.4>branch-2024.1 merge, the docs-pages is not triggering and therefore the documentation is not updated with the new change,
In this change, I added the `branch-**` pattern, so changes to release branches will trigger the workflow
Closesscylladb/scylladb#17281
* github.com:scylladb/scylladb:
docs: always build from the default branch
docs: trigger the docs-pages workflow on release branches
* tools/cqlsh b8d86b76...e5f5eafd (2):
> dist/debian: fix the trailer line format
> `COPY TO STDOUT` shouldn't put None where a function is expected
Fixes: scylladb/scylladb#17451Closesscylladb/scylladb#17447
key_view::explode() contains a blatant use-after-free:
unless the input is already linearized, it returns a view to a local temporary buffer.
This is rare, because partition keys are usually not large enough to be fragmented.
But for a sufficiently large key, this bug causes a corrupted partition_key down
the line.
Fixes#17625Closesscylladb/scylladb#17626
instead of using the hand-crafted operator==, use the default-generated
one, which is equivalent to the former.
regarding the difference between global operator== and member operator==,
the default-generated operator in C++20 is now symmetric. so we don't
need to worry about the problem of `max_result_size` being lhs or rhs.
but neither do we need to worry about the implicit conversion, because
all constructors of `max_result_size` are marked explicit. so we don't
gain any advantage by making the operator== global instead of a member
operator.
Signed-off-by: Kefu Chai <kefu.chai@scylladb.com>
Closesscylladb/scylladb#17536
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
* tablet_id
* tablet_replica
* tablet_metadata
* tablet_map
their operator<<:s are dropped
Refs scylladb/scylladb#13245
Signed-off-by: Kefu Chai <kefu.chai@scylladb.com>
Closesscylladb/scylladb#17504
in 5202bb9d, we introduced repair/table_check.cc, but we didn't
update repair/CMakeLists.txt accordingly. but the symbols defined
by this compilation unit is referenced by other source files when
building scylla.
so, in this change, we add this table_check.cc to the "repair"
target.
Signed-off-by: Kefu Chai <kefu.chai@scylladb.com>
Closesscylladb/scylladb#17517
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
* position_range
* mutation_fragment
* range_tombstone_stream
* mutation_fragment_v2::printer
Refs #13245Closesscylladb/scylladb#17521
* github.com:scylladb/scylladb:
mutation: add fmt::formatter for position_range
mutation: add fmt::formatter for mutation_fragment and range_tombstone_stream
mutation: add fmt::formatter for mutation_fragment_v2::printer
This stage doesn't need any special treatment, because we cannot revert
to old replicas and should proceed normally. The barrier itself won't
get stuck, because it already handles excluded/ignored nodes.
Just make the test validate it.
Signed-off-by: Pavel Emelyanov <xemul@scylladb.com>
Two options here -- go revert to old replicas by jumping into
cleanup_target stage or proceed noramlly. The choice depends on which
replica set has less number of dead nodes.
Signed-off-by: Pavel Emelyanov <xemul@scylladb.com>
At this stage it can happen that target replica got some writes, so its
tablet needs to be cleaned up, so jump to cleanup_target stage.
Signed-off-by: Pavel Emelyanov <xemul@scylladb.com>
There are several transition stages that are executed by the topology
coordinator with the help of barrier-and-drain raft commands. For the
test to stop and remove a node while handling this stage it must inject
a break-point into barrier handler, wait for it to happen and then stop
the node without resuming the break-point. Then removenode from the
cluster.
The break-point suspends barrier handling when a specific tablet is in
specific transition stage. Tablet ID and desired stage are configured
via injector parameters.
With today's error-injection facilities the way to suspend code
execution is with injecting a lambda that waits for a message from the
injection engine.
Signed-off-by: Pavel Emelyanov <xemul@scylladb.com>
it'd be more pythonic to just put an expression after `assert`,
instead of quoting it with a pair of parenthesis. and there is no need
to add `;` after `break`.
Signed-off-by: Kefu Chai <kefu.chai@scylladb.com>
Make a specialized sstable_set for tablets
via tablet_storage_group_manager::make_sstable_set.
This sstable set takes a snapshot of the storage_groups
(compound) sstable_sets and maps the selected tokens
directly into the tablet compound_sstable_set.
This sstable_set provides much more efficient access
to the table's sstable sets as it takes advantage of the disjointness
of sstable sets between tablets/storage_groups, and making it is cheaper
that rebuilding a complete partitioned_sstable_set from all sstables in the table.
Fixes#16876
Cassandra-stress setup:
```
$ sudo cpupower frequency-set -g userspace
$ build/release/scylla (developer-mode options) --smp=16 --memory=8G --experimental-features=consistent-topology-changes --experimental-features=tablets
cqlsh> CREATE KEYSPACE keyspace1 WITH replication={'class':'NetworkTopologyStrategy', 'replication_factor':1} AND tablets={'initial':2048};
$ ./tools/java/tools/bin/cassandra-stress write no-warmup n=10000000 -pop 'seq=1...10000000' -rate threads=128
$ scylla-api-client system drop_sstable_caches POST
$ ./tools/java/tools/bin/cassandra-stress read no-warmup duration=60s -pop 'dist=uniform(1..10000000)' -rate threads=128
$ scylla-api-client system drop_sstable_caches POST
$ ./tools/java/tools/bin/cassandra-stress mixed no-warmup duration=60s -pop 'dist=uniform(1..10000000)' -rate threads=128
```
Baseline (0a7854ea4d) vs. fix (0c2c00f01b)
Throughput (op/s):
workload | baseline | fix
---------|----------|----------
write | 76,806 | 100,787
read | 34,330 | 106,099
mixed | 32,195 | 79,246
Closesscylladb/scylladb#17149
* github.com:scylladb/scylladb:
table: tablet_storage_group_manager: make tablet_sstable_set
storage_group_manager: add make_sstable_set
tablet_storage_group_manager: handle_tablet_split_completion: pre-calc new_tablet_count
table: tablet_storage_group_manager: storage_group_of: do not validate in release build mode
table: move compaction_group_list and storage_group_vector to storage_group_manager
compaction_group::table_state: get_group_id: become self-sufficient
compaction_group, table: make_compound_sstable_set: declare as const
tablet_storage_group_manager: precalculate my_host_id and _tablet_map
table: coroutinize update_effective_replication_map
instead of passing fmt string as a plain `const char*`, pass it as
a consteval type, so that `fmt::format()` can perform compile-time
format check against it and the formatted params.
Signed-off-by: Kefu Chai <kefu.chai@scylladb.com>
Closesscylladb/scylladb#17656
before this change, we used the lower-case CMake build configuration
name for the rust profile names. but this was wrong, because the
profiles are named with the scylla build mode.
in this change, we translate the `$<CONFIG>` to scylla build mode,
and use it for the profile name and for the output directory of
the built library.
Closesscylladb/scylladb#17648
* github.com:scylladb/scylladb:
build: cmake: use scylla build mode for rust profile name
build: cmake: define per-config build mode
Repairs have to obtain a permit to the reader concurrency semaphore on
each shard they have a presence on. This is prone to deadlocks:
node1 node2
repair1_master (takes permit) repair1_follower (waits on permit)
repair2_master (waits for permit) repair2_follower (takes permit)
In lieu of strong central coordination, we solved this by making permits
evictable: if repair2 can evict repair1's permit so it can obtain one
and make progress. This is not efficient as evicting a permit usually
means discarding already done work, but it prevents the deadlocks.
We recently discovered that there is a window when deadlocks can still
happen. The permit is made evictable when the disk reader is created.
This reader is an evictable one, which effectively makes the permit
evictable. But the permit is obtained when the repair constrol
structrure -- repair meta -- is create. Between creating the repair meta
and reading the first row from disk, the deadlock is still possible. And
we know that what is possible, will happen (and did happen). Fix by
making the permit evictable as soon as the repair meta is created. This
is very clunky and we should have a better API for this (refs #17644),
but for now we go with this simple patch, to make it easy to backport.
Refs: #17644Fixes: #17591Closesscylladb/scylladb#17646
This patch series makes all auth writes serialized via raft. Reads stay
eventually consistent for performance reasons. To make transition to new
code easier data is stored in a newly created keyspace: system_auth_v2.
Internally the difference is that instead of executing CQL directly for
writes we generate mutations and then announce them via raft group0. Per
commit descriptions provide more implementation details.
Refs https://github.com/scylladb/scylladb/issues/16970
Fixes https://github.com/scylladb/scylladb/issues/11157Closesscylladb/scylladb#16578
* github.com:scylladb/scylladb:
test: extend auth-v2 migration test to catch stale static
test: add auth-v2 migration test
test: add auth-v2 snapshot transfer test
test: auth: add tests for lost quorum and command splitting
test: pylib: disconnect driver before re-connection
test: adjust tests for auth-v2
auth: implement auth-v2 migration
auth: remove static from queries on auth-v2 path
auth: coroutinize functions in password_authenticator
auth: coroutinize functions in standard_role_manager
auth: coroutinize functions in default_authorizer
storage_service: add support for auth-v2 raft snapshots
storage_service: extract getting mutations in raft snapshot to a common function
auth: service: capture string_view by value
alternator: add support for auth-v2
auth: add auth-v2 write paths
auth: add raft_group0_client as dependency
cql3: auth: add a way to create mutations without executing
cql3: run auth DML writes on shard 0 and with raft guard
service: don't loose service_level_controller when bouncing client_state
auth: put system_auth and users consts in legacy namespace
cql3: parametrize keyspace name in auth related statements
auth: parametrize keyspace name in roles metadata helpers
auth: parametrize keyspace name in password_authenticator
auth: parametrize keyspace name in standard_role_manager
auth: remove redundant consts auth::meta::*::qualified_name
auth: parametrize keyspace name in default_authorizer
db: make all system_auth_v2 tables use schema commitlog
db: add system_auth_v2 tables
db: add system_auth_v2 keyspace
When a tool application is invoked with an unknown operation, an error
message is printed, which includes all the known operations, with all
their aliases. This is collected in `std::vector<std::string_view>`. The
problem is that the vector containing alias names, is returned as a
value, so the code ends up creating views to temporaries.
Fix this by returning alias vector with const&.
Fixes: #17584Closesscylladb/scylladb#17586
The helper in question is supposed to spawn a background fiber with
tablet migration stage action and repeat it in case action fails (until
operator intervention, but that's another story). In case action fails
a message with ERROR level is logger about the failure.
This error confuses some tests that scan scylla log messages for
ERROR-s at the end, treat most of them (if not all) as ciritical and
fail. But this particular message is not in fact an error -- topology
coordinator would re-execute this action anyway, so let's demote the
message to be WARN instead.
refs: #17027
Signed-off-by: Pavel Emelyanov <xemul@scylladb.com>
Closesscylladb/scylladb#17568
This series adds a Python script that searches the code for metrics definition and their description.
Because part of the code uses a nonstandard way of definition, it uses a configuration file to resolve parameter values.
The script supports the code that uses string format and string concatenation with variables.
The documentation team will use the results to both document the existing metrics and to get the metrics changes between releases.
Replaces #16328Closesscylladb/scylladb#17479
* github.com:scylladb/scylladb:
Adding scripts/metrics-config.yml
Adding scripts/get_description.py to fetch metrics description
before this change, we failed to apply the filtering of tablestats
command in the right way:
1. `table_filter` failed to check if delimiter is npos before
extract the cf component from the specified table name.
2. the stats should not included the keyspace which are not
included by the filter.
3. the total number of tables in the stats report should contain
all tables no matter they are filtered or not.
in this change, all the problems above are addressed. and the tests
are updated to cover these use cases.
Signed-off-by: Kefu Chai <kefu.chai@scylladb.com>
Closesscylladb/scylladb#17468
Make a specialized sstable_set for tablets
via tablet_storage_group_manager::make_sstable_set.
This sstable set takes a snapshot of the storage_groups
(compound) sstable_sets and maps the selected tokens
directly into the tablet compound_sstable_set.
Refs #16876
Signed-off-by: Benny Halevy <bhalevy@scylladb.com>
Move the responsibility for preparing the table_set
covering all sstables in the table to the storage_group_manager
so it can specialize the sstable_set.
Signed-off-by: Benny Halevy <bhalevy@scylladb.com>
Mini-cleanup of `new_tablet_count`, similar
to pre-calculating `old_tablet_count` once.
While at it, add some missing coding-style related spaces.
Signed-off-by: Benny Halevy <bhalevy@scylladb.com>
No validation is really required in release build.
Add `#ifndef SCYLLA_BUILD_MODE_RELEASE` before
adding another term to the logic in the next patch
that adds support for sparse allocation in a cloned
tablet_storage_group_manager.
Signed-off-by: Benny Halevy <bhalevy@scylladb.com>
