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
`partition_range_view` and `i_partition`, and drop their operator<<:s.
Refs #13245
Signed-off-by: Kefu Chai <kefu.chai@scylladb.com>
Closesscylladb/scylladb#17331
During shutdown, as all system tables are closed in parallel, there is a
possibility of a race condition between compaction stoppage and the
closure of the compaction_history table. So, quiesce all the compaction
tasks before attempting to close the tables.
Fixes#15721
Signed-off-by: Lakshmi Narayanan Sreethar <lakshmi.sreethar@scylladb.com>
Closesscylladb/scylladb#17218
A user asked on the ScyllaDB forum several questions on whether
tombstone_gc works on materialized views. This patch includes two
tests that confirm the following:
1. The tombstone_gc may be set on a view - either during its creation
with CREATE MATERIALIZED VIEW or later with ALTER MATERIALIZED VIEW.
2. The tombstone_gc setting is correctly shown - for both base tables
and views - by the "DESC" statement.
3. The tombstone_gc setting is NOT inherited from a base table to a new
view - if you want this option on a view, you need to set it
separately.
Unfortunately, this test could not be a single-node cql-pytest because
we forbid tombstone_gc=repair when RF=1, and since recently, we forbid
setting RF>1 on a single-node setup. So the new tests are written in
the test/topology framework - which may run multiple tests against
a single three-node cluster run multiple tests against it.
To write tests over a shared cluster, we need functions which create
temporary keyspaces, tables and views, which are deleted automatically
as soon as a test ends. The test/topology framework was lacking such
functions, so this tests includes them - currently inside the test
file, but if other people find them useful they can be moved to a more
central location.
The new functions, net_test_keyspace(), new_test_table() and
new_materialized_view() are inspired by the identically-named
functions in test/cql-pytest/util.py, but the implementation is
different: Importantly, the new functions here are *async*
context managers, used via "async with", to fit with the rest
of the asynchronous code used in the topology test framework.
Signed-off-by: Nadav Har'El <nyh@scylladb.com>
Closesscylladb/scylladb#17345
`-fno-sanitize-address-use-after-scope` is used to disable the check for
stack-use-after-scope bugs, but this check is only performed when ASan
is enabled. if we pass this option when ASan is not enabled, we'd have
following warning, so let's apply it only when ASan is enabled.
```
clang-16: error: argument unused during compilation:
'-fno-sanitize-address-use-after-scope' [-Werror,-Wunused-command-line-argument]
```
Signed-off-by: Kefu Chai <kefu.chai@scylladb.com>
Closesscylladb/scylladb#17329
This commit updates the Handling Node Failures page
to specify that the quorum requirement refers to both
schema and topology updates.
Closesscylladb/scylladb#17321
This series makes several changes to how ignored nodes list is treated
by the topology coordinator. First the series makes it global and not
part of a single topology operation, second it extends the list at the
time of removenode/replace invocation and third it bans all nodes in
the list from contacting the cluster ever again.
The main motivation is to have a way to unblock tablet migration in case
of a node failure. Tablet migration knows how to avoid nodes in ignored
nodes list and this patch series provides a way to extend it without
performing any topology operation (which is not possible while tables
migration runs).
Fixesscylladb/scylladb#16108
* 'gleb/ignore-nodes-handling-v2' of github.com:scylladb/scylla-dev:
test: add test for the new ignore nodes behaviour
topology coordinator: cleanup node_state::decommissioning state handling code
topology coordinator: ban ignored nodes just like we ban nodes that are left
storage_service: topology coordinator: validate ignore dead nodes parameters in removenode/replace
topology coordinator: add removed/replaced nodes to ignored_nodes list at the request invocation time
topology coordinator: make ignored_nodes list global and permanent
topology_coordinator: do not cancel rebuild just because some other nodes are dead
topology coordinator: throw more specific error from wait_for_ip() function in case of a timeout
raft_group0: add make_nonvoters function that can make multiple node non voters simultaneously
In a follow-up patch abort_source will be used
inside those methods. Current pattern is that abort_source
is passed everywhere as non const so it needs to be
executed in non const context.
Closesscylladb/scylladb#17312
The test checks that once a node is specified in ignored node list by
one topology operation the information is carried over to the next
operation as well.
The code is shared between decommission and removenode and it has
scattered 'ifs' for different behaviours between those. Change it to
have only one 'if'.
Since now a node that is at one point was marked as dead, either via
--ignore-dead-nodes parameter or by been a target for removenode or
replace, can no longer be made "undead" we need to make sure that they
cannot rejoin the cluster any longer. Do that by banning them on a
messaging layer just like we do for nodes that are left.
Not that the removenode failure test had to be altered since it restarted
a node after removenode failure (which now will not work). Also, since
the check for liveness was removed from the topology coordinator (because
the node is already banned by then), the test case that triggers the
removed code is removed as well.
the quote of "The minimum block size for crc enabled filesystems is
1024" comes from the output of mkfs.xfs, let's quote the source for
better maintainability.
Signed-off-by: Kefu Chai <kefu.chai@scylladb.com>
Closesscylladb/scylladb#17094
The true motivation for this patch is a certain problem with configure.py
in scylla-enterprise, which can only be solved by moving the `extra_cxxflags`
lines before configure_seastar(). This patch does that by hoisting
get_extra_cxxflags() up to create_build_system().
But this patch makes sense even if we disregard the real motivation.
It's weird that a function called `write_build_file()` adds additional
build flags on its own.
Closesscylladb/scylladb#17189
This commit renames keyspace::get_effective_replication_map()
to keyspace::get_vnode_effective_replication_map(). This change
is required to ease the analysis of the usage of this function.
When tablets are enabled, then this function shall not be used.
Instead of per-keyspace, per-table replication map should be used.
The rename was performed to distinguish between those two calls.
The next step will be an audit of usages of
keyspace::get_vnode_effective_replication_map().
Refs: scylladb#16626
Signed-off-by: Patryk Wrobel <patryk.wrobel@scylladb.com>
Closesscylladb/scylladb#17314
All cql-pytest tests use one node, and unsuprisingly most use RF=1.
By default, as part of the "guardrails" feature, we print a warning
when creating a keyspace with RF=1. This warning gets printed on
every cql-pytest run, which creates a "boy who cried wolf" effect
whereby developers get used to seeing these warnings, and won't care
if new warnings start appearing.
The fix is easy - in run.py start Scylla with minimum-replication-factor-
warn-threshold set to -1 instead of the default 3.
Note that we do have cql-pytest tests for this guardrail, but those don't
rely on the default setting of this variable (they can't, cql-pytest
tests can also be run on a Scylla instance run manually by a developer).
Those tests temporarily set the threshold during the test.
Signed-off-by: Nadav Har'El <nyh@scylladb.com>
Closesscylladb/scylladb#17274
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
`collection_mutation_view::printer`, and drop its
operator<<.
Refs #13245
Signed-off-by: Kefu Chai <kefu.chai@scylladb.com>
Closesscylladb/scylladb#17300
Adds a test reproducing https://github.com/scylladb/scylladb/issues/16759, and the instrumentation needed for it.
Closesscylladb/scylladb#17208
* github.com:scylladb/scylladb:
row_cache_test: test cache consistency during memtable-to-cache merge
row_cache: use preemption_source in update()
utils: preempt: add preemption_source
codespell reports that "statics" could be the misspelling of
"statistics". but "static" here means the static column(s). so
replace "static" with more specific wording.
Refs #589
Signed-off-by: Kefu Chai <kefu.chai@scylladb.com>
Closesscylladb/scylladb#17216
when we just want to perform read access to `http_context`, there
is no need to use a non-const reference. so let's add `const` specifier
to make this explicit. this shoudl help with the readability and
maintainability.
Signed-off-by: Kefu Chai <kefu.chai@scylladb.com>
Closesscylladb/scylladb#17219
Support skipping multiple patterns by allowing them to be passed via
multiple '--skip' arguments to test.py.
Example : `test.py --skip=topology --skip=sstables`
Signed-off-by: Lakshmi Narayanan Sreethar <lakshmi.sreethar@scylladb.com>
Closesscylladb/scylladb#17220
"fro" is the short of "from" but the value is an
`optimized_optional<flat_mutation_reader_v2>`. codespell considers
it a misspelling of "for" or "from". neither of them makes sense,
so let's change it to "reader" for better readability, also for
silencing the warning. so that the geniune warning can stands out,
this would help to make the codespell more useful.
Signed-off-by: Kefu Chai <kefu.chai@scylladb.com>
Closesscylladb/scylladb#17221
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 `raft::log`, and drop its
operator<<.
Refs #13245
Signed-off-by: Kefu Chai <kefu.chai@scylladb.com>
Closesscylladb/scylladb#17301
This commit removes the Open Source vs. Enterprise matrix
from the Open Source documentation.
In addition, a redirection is added to prevent 404 in the OSS docs,
and to the removed page is replaced with a link to the same page
in the Enterprise docs.
This commit must be reverted enterprise.git, because
we want to keep the Matrix in the Enterprise docs.
Fixes https://github.com/scylladb/scylladb/issues/17289Closesscylladb/scylladb#17295
Alternator Streams doesn't yet work on tables using tablets (this is
issue #16317). Before this patch, an attempt to enable it results in
an unsightly InternalServerError, which isn't terrible - but we can
do better.
So in this patch, we make the attempt to enable Streams and tablets
together into a clear error. The error message points to the open issue,
and also suggests how to create a table that uses vnodes, not tablets.
Unfortunately, there are slightly two different code paths and error
messages for two cases: One case is the creation of a new table (where
the validation happens before the keyspace is actually created), and
the other case is an attempt to enable streams on an existing table
with an existing keyspace (which already might or might not be using
tablets).
This patch also adds a test that verifies that trying to enable Streams
with tablets is an error - in both cases (table creation and update).
Obviously, this test - and the validation code - should be removed once
the issue is solved and Alternator Streams begins working with tablets.
Fixes#16497
Refs #16807
Signed-off-by: Nadav Har'El <nyh@scylladb.com>
Closesscylladb/scylladb#17311
With tablets, we don't use vnode-oriented sstable cleanup.
So let's just remove unused code and bail out silently if sharding is
tablet based. The reason for silence is that we don't want to break
tests that might be reused for tablets, and it's not a problem for
sstable cleanup to be ignored with tablets.
This approach is actually already used in the higher level code,
implementing the cleanup API.
Signed-off-by: Raphael S. Carvalho <raphaelsc@scylladb.com>
Closesscylladb/scylladb#17296
To unblock tablet migration in case of a node failure we need a way to
dynamically extend a list of ignored_nodes while the migration is
happening. This patch does it by piggybacking on existing topology
operations that assume their target node is already dead. It adds the
target node to now global ignored_nodes list when request is issued and,
for better HA, makes the nodes in ignored_nodes non voters.
Currently ignored_nodes list is part of a request (removenode or
replace) and exists only while a request is handled. This patch
changes it to be global and exist outside of any request. Node stays
in the list until they eventually removed and moved to the "left" state.
If a node is specified in the ignore-dead-nodes option for any command
it will be ignored for all other operations that support ignored_nodes
(like tablet migration).
In sync_raft_topology_nodes we execute a system keyspace
update query for each node of the cluster. The system keyspace
tables use schema commitlog which by default enables use_o_dsync.
This means that each write to the commitlog is accompanied by fsync.
For large clusters this can incur hundreds of writes with fsyncs, which
is very expensive. For example, in #17039 for a moderate size cluster
of 50 nodes sync_raft_topology_nodes took almost 5 seconds.
In this commit we solve this problem by running all such update
queries in parallel. The commitlog should batch them and issue
only one write syscall to the OS.
Closesscylladb/scylladb#17243
Unfortunately, scylladb/python-driver#230 is not fixed yet, so it is
necessary for the sake of our CI's stability to re-create the driver
session after all nodes in the cluster are restarted.
There is one place in test_topology_recovery_basic where all nodes are
restarted but the driver session is not re-created. Even though nodes
are not restarted at once but rather sequentially, we observed a failure
with similar symptoms in a CI run for scylla-enterprise.
Add the missing driver reconnect as a workaround for the issue.
Fixes: scylladb/scylladb#17277Closesscylladb/scylladb#17278
Instead of adding an asterisk next to "liveness" linking to the glossary, we will temporarily replace them with a hyperlink pending the implementation of tooltip functionality.
Closesscylladb/scylladb#17244
Commit 904bafd069 consolidated the two
existing for_each_tablet() overloads, to the one which has a future<>
returning callback. It also added yields to the bodies of said
callbacks. This is unnecessary, the loop in for_each_tablet() already
has a yield per tablet, which should be enough to prevent stalls.
This patch is a follow-up to #17118Closesscylladb/scylladb#17284
RPC is not ready yet at this point, so we should not set this application state yet.
Also, simplify add_local_application_state as it contains dead code
that will never generate an internal error after 1d07a596bf.
Fixes#16932Closesscylladb/scylladb#17263
* github.com:scylladb/scylladb:
gossiper: add_local_application_state: drop internae error
transport: controller: do_start_server: do not set_cql_read for maintenance port
this change is a follow-up of 637dd730. the goal is to use
std::filesystem::path for manipulating paths, and to avoid the
converting between sstring and fs::path back and forth.
Signed-off-by: Kefu Chai <kefu.chai@scylladb.com>
Closesscylladb/scylladb#17257
in the same spirit of e84a0991, let's switch the callers who expect
std::string to fmt::format(). to minimize the impact and to reduce
the risk, the switch will be performed piecemeal.
Signed-off-by: Kefu Chai <kefu.chai@scylladb.com>
Closesscylladb/scylladb#17253
When a node is decommissioned, all tablet replicas need to be moved away
from it. In some cases it may not be possible. If the number of node in
the cluster equals the keysapce RF, one cannot decommission any node
because it's not possible to find nodes for every replica.
The new test case validates this constraint is satisfied.
refs: #16195
Signed-off-by: Pavel Emelyanov <xemul@scylladb.com>
Closesscylladb/scylladb#17248
A materialized view in CQL allows AT MOST ONE view key column that
wasn't a key column in the base table. This is because if there were
two or more of those, the "liveness" (timestamp, ttl) of these different
columns can change at every update, and it's not possible to pick what
liveness to use for the view row we create.
We made an exception for this rule for Alternator: DynamoDB's API allows
creating a GSI whose partition key and range key are both regular columns
in the base table, and we must support this. We claim that the fact that
Alternator allows neither TTL (Alternator's "TTL" is a different feature)
nor user-defined timestamps, does allow picking the liveness for the view
row we create. But we did it wrong!
We claimed in a comment - and implemented in the code before this patch -
that in Alternator we can assume that both GSI key columns will have the
*same* liveness, and in particular timestamp. But this is only true if
one modifies both columns together! In fact, in general it is not true:
We can have two non-key attributes 'a' and 'b' which are the GSI's key
columns, and we can modify *only* b, without modifying a, in which case
the timestamp of the view modification should be b's newer timestamp,
not a's older one. The existing code took a's timestamp, assuming it
will be the same as b's, which is incorrect. The result was that if
we repeatedly modify only b, all view updates will receive the same
timestamp (a's old timestamp), and a deletion will always win over
all the modifications. This patch includes a reproducing test written by
a user (@Zak-Kent) that demonstrates how after a view row is deleted
it doesn't get recreated - because all the modifications use the same
timestamp.
The fix is, as suggested above, to use the *higher* of the two
timestamps of both base-regular-column GSI key columns as the timestamp
for the new view rows or view row deletions. The reproducer that
failed before this patch passes with it. As usual, the reproducer
passes on AWS DynamoDB as well, proving that the test is correct and
should really work.
Fixes#17119
Signed-off-by: Nadav Har'El <nyh@scylladb.com>
Closesscylladb/scylladb#17172
This patch adds a simple reproducer for a regression in Scylla 5.4 caused
by commit 432cb02, breaking LIMIT support in GROUP BY.
Refs #17237
Signed-off-by: Nadav Har'El <nyh@scylladb.com>
Closesscylladb/scylladb#17275
PEP 632 deprecates distutils module, and it is remove from Python 3.12.
we are actually using the one vendored by setuptools, if we are using
3.12. so let's use shutil for finding ninja executable.
see https://peps.python.org/pep-0632/
Signed-off-by: Kefu Chai <kefu.chai@scylladb.com>
Closesscylladb/scylladb#17271
Reimplements stop/start sequence using rolling_restart() which is safe
with regards to UP status propagation and not prone to sudden
connection drop which may cause later CQL queries to time out. It also
ensures that CQL is up on all the remaining nodes when the with_down
callback is executed.
The test was observed to fail in CI like this:
```
cassandra.cluster.NoHostAvailable: ('Unable to complete the operation against any hosts', {<Host: 127.157.135.26:9042 datacenter1>: ConnectionException('Pool for 127.157.135.26:9042 is shutdown')})
...
@pytest.mark.repair
@pytest.mark.asyncio
async def test_tablet_missing_data_repair(manager: ManagerClient):
...
for idx in range(0,3):
s = servers[idx].server_id
await manager.server_stop_gracefully(s, timeout=120)
> await check()
```
Hopefully: Fixes#17107Closesscylladb/scylladb#17252
* github.com:scylladb/scylladb:
test: py: tablets: Fix flakiness of test_tablet_missing_data_repair
test: pylib: manager_client: Wait for driver to catch up in rolling_restart()
test: pylib: manager_client: Accept callback in rolling_restart() to execute with node down
The reason we introduced the tombstone-limit
(query_tombstone_page_limit), was to allow paged queries to return
incomplete/empty pages in the face of large tombstone spans. This works
by cutting the page after the tombstone-limit amount of tombstones were
processed. If the read is unpaged, it is killed instead. This was a
mistake. First, it doesn't really make sense, the reason we introduced
the tombstone limit, was to allow paged queries to process large
tombstone-spans without timing out. It does not help unpaged queries.
Furthermore, the tombstone-limit can kill internal queries done on
behalf of user queries, because all our internal queries are unpaged.
This can cause denial of service.
So in this patch we disable the tombstone-limit for unpaged queries
altogether, they are allowed to continue even after having processed the
configured limit of tombstones.
Fixes: #17241Closesscylladb/scylladb#17242
we recently added -Wextra to configure.py, and this option enables
a bunch of warning options, including `-Wignored-qualifiers`. so
there is no need to enable this specific warning anymore. this change
remove ths option from both `configure.py` and the CMake building system.
Signed-off-by: Kefu Chai <kefu.chai@scylladb.com>
Closesscylladb/scylladb#17272
