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>
There are several places in code that calculate replica sets associated
with specific tablet transision. Having a helper to substract two sets
improves code readability.
Signed-off-by: Pavel Emelyanov <xemul@scylladb.com>
Closesscylladb/scylladb#18033
The CDC feature is not supported on a table that uses tablets
(Refs https://github.com/scylladb/scylladb/issues/16317), so if a user creates a keyspace with tablets enabled
they may be surprised later (perhaps much later) when they try to enable
CDC on the table and can't.
The LWT feature always had issue Refs https://github.com/scylladb/scylladb/issues/5251, but it has become potentially
more common with tablets.
So it was proposed that as long as we have missing features (like CDC or
LWT), every time a keyspace is created with tablets it should output a
warning (a bona-fide CQL warning, not a log message) that some features
are missing, and if you need them you should consider re-creating the
keyspace without tablets.
This PR does this.
The warning text which will be produced is the following (obviously, it can
be improved later, as we perhaps find more missing features):
> "Tables in this keyspace will be replicated using tablets, and will
> not support the CDC feature (issue https://github.com/scylladb/scylladb/issues/16317) and LWT may suffer from
> issue https://github.com/scylladb/scylladb/issues/5251 more often. If you want to use CDC or LWT, please drop
> this keyspace and re-create it without tablets, by adding AND TABLETS
> = {'enabled': false} to the CREATE KEYSPACE statement."
This PR also includes a test - that checks that this warning is is
indeed generated when a keyspace is created with tablets (either by default
or explicitly), and not generated if the keyspace is created without
tablets. It also fixes existing tests which didn't like the new warning.
Fixes https://github.com/scylladb/scylladb/issues/16807Closesscylladb/scylladb#17318
* github.com:scylladb/scylladb:
tablets: add warning on CREATE KEYSPACE
test/cql-pytest: fix guadrail tests to not be sensitive to more warnings
feature_service.hh is a high-level header that integrates much
of the system functionality, so including it in lower-level headers
causes unnecessary rebuilds. Specifically, when retiring features.
Fix by removing feature_service.hh from headers, and supply forward
declarations and includes in .cc where needed.
Closesscylladb/scylladb#18005
When loading topology state, nodes are checked to have or not to have
"tokens" field set. The check is done based on node state and it's
spread across the loading method.
Signed-off-by: Pavel Emelyanov <xemul@scylladb.com>
Closesscylladb/scylladb#17957
These commands manage to avoid detection because they are not documented on https://opensource.docs.scylladb.com/stable/operating-scylla/nodetool.html.
They were discovered when running dtests, with ccm tuned to use the native nodetool directly. See https://github.com/scylladb/scylla-ccm/pull/565.
The commands come with tests, which pass with both the native and Java nodetools. I also checked that the relevant dtests pass with the native implementation.
Closesscylladb/scylladb#17979
* github.com:scylladb/scylladb:
tools/scylla-nodetool: implement the sstableinfo command
tools/scylla-nodetool: implement the getsstables command
tools/scylla-nodetool: move get_ks_cfs() to the top of the file
test/nodetool: rest_api_mock.py: add expected_requests context manager
before this change, we rely on the default-generated fmt::formatter
created from operator<<, but fmt v10 dropped the default-generated
formatter.
also, it's impossible to partial specialize a nested type of a
template class, we cannot specialize the `fmt::formatter` for
`stop_crash<M>::result_type`, as a workaround, a new type is
added.
in this change,
* define a new type named `stop_crash_result`
* add fmt::formatter for `stop_crash_result`
* define stop_crash::result_type as an alias of `stop_crash_result`
Refs #13245
Signed-off-by: Kefu Chai <kefu.chai@scylladb.com>
Closesscylladb/scylladb#18018
`UUID::to_sstring()` relies on `FMT_DEPRECATED_OSTREAM` to generated `fmt::formatter` for `UUID`, and this feature is deprecated in {fmt} v9, and dropped in {fmt} v10.
in this series, all callers of `UUID::to_sstring()` are switched to `fmt::to_string()`, and this function is dropped.
Closesscylladb/scylladb#18020
* github.com:scylladb/scylladb:
utils: UUID: drop UUID::to_sstring()
treewide: use fmt::to_string() to transform a UUID to std::string
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:
* add `format_as()` for `segment` so we can use it as a fallback
after upgrading to {fmt} v10
* use fmt::streamed() when formatting `segment`, this will be used
the intermediate solution before {fmt} v10 after dropping
`FMT_DEPRECATED_OSTREAM` macro
Refs #13245
Signed-off-by: Kefu Chai <kefu.chai@scylladb.com>
Closesscylladb/scylladb#18019
To create the list of tests to run there's a loop that fist collects all tests from suits, then filters the list in two ways -- excludes opt-out-ed lists (disabled and matching the skip pattern) or leaves there only opt-in-ed (those, specified as positional arguments).
This patch keeps both list-checking code close to each other so that the intent is explicitly clear.
Closesscylladb/scylladb#17981
* github.com:scylladb/scylladb:
test.py: Give local variable meaningful name
test.py: Sanitize test list creation
We move the mode check so that the raft-based decommission also uses
it. Without this check, it hanged after the drain operation instead
of instantly failing. `test_decommission_after_drain_is_invalid` was
failing because of it with the raft-based topology enabled.
Fixesscylladb/scylladb#16761Closesscylladb/scylladb#18000
There are skip_in_<mode> lists in suite yaml that tells test.py not to run the test from it. This PR sanitizes these lists in two ways.
First, to skip pytests the skip-decorators are much more convenient, e.g. because they show the reason why the test is skipped.
Also, if a test wants to be opt-in-ed for some mode only, it's opt-out-ed in all other lists instead. There's run_in_<mode> list in suite for that.
Closesscylladb/scylladb#17964
* github.com:scylladb/scylladb:
test: Do not duplicate test name in several skip-lists
test: Mark tests with skip_mode instead of suite skip-list
this function is not used anymore, and it relies on
`FMT_DEPRECATED_OSTREAM` to generated `fmt::formatter` for
`UUID`, and this feature is deprecated in {fmt} v9, and
dropped in {fmt} v10.
in this change, let's drop this member function.
Signed-off-by: Kefu Chai <kefu.chai@scylladb.com>
without `FMT_DEPRECATED_OSTREAM` macro, `UUID::to_sstring()` is
implemented using its `fmt::formatter`, which is not available
at the end of this header file where `UUID` is defined. at this moment,
we still use `FMT_DEPRECATED_OSTREAM` and {fmt} v9, so we can
still use `UUID::to_sstring()`, but in {fmt} v10, we cannot.
so, in this change, we change all callers of `UUID::to_sstring()`
to `fmt::to_string()`, so that we don't depend on
`FMT_DEPRECATED_OSTREAM` and {fmt} v9 anymore.
Signed-off-by: Kefu Chai <kefu.chai@scylladb.com>
When a view update has both a local and remote target endpoint,
it extends the lifetime of its memory tracking semaphore units
only until the end of the local update, while the resources are
actually used until the remote update finishes.
This patch changes the semaphore transferring so that in case
of both local and remote endpoints, both view updates share the
units, causing them to be released only after the update that
takes longer finishes.
Fixes#17890Closesscylladb/scylladb#17891
So tests and fixtures can use `with expected_requests():` and have
cleanup be taken care for them. I just discovered that some tests do not
clean up after themselves and when running all tests in a certain order,
this causes unrelated tests to fail.
Fix by using the context everywhere, getting guaranteed cleanup after
each test.
The test creates ut4 with a lot of fields,
this may take a while in debug builds,
to avoid raft operation timeout set the threshold
to some big value.
The error injector is disabled in release builds,
so this settings won't be applied to them.
This shouldn't be a problem since release builds
are fast enough, even on arm.
Fixesscylladb/scylladb#17987Closesscylladb/scylladb#17997
Currently, the tests in test/cql-pytest can be run against both ScyllaDB and Cassandra.
Running the test for either will first output the test results, and subsequently
print the stdout output of the process under test. Using the command line
option --omit-scylla-output it is possible to disable this print for Scylla,
but it is not possible for tests run against Cassandra.
This change adds the option to suppress output for Cassandra tests, too. By default,
the stdout of the Cassandra run will still be printed after the test results, but
this can now be disabled with --omit-scylla-output
Closesscylladb/scylladb#17996
Some tests are only run in dev mode for some reason. For such tests
there's run_in_dev list, no need in putting it in all the non-dev
skip_in_... ones.
Signed-off-by: Pavel Emelyanov <xemul@scylladb.com>
There are many tests that are skipped in release mode becuase they rely
on error-injection machinery which doesn't work in release mode. Most of
those tests are listed in suite's skip_in_release, but it's not very
handy, mainly because it's not clear why the test is there. The
skip_mode decoration is much more convenient.
Signed-off-by: Pavel Emelyanov <xemul@scylladb.com>
To create the list of tests to run there's a loop that fist collects all
tests from suits, then filters the list in two ways -- excludes
opt-out-ed lists (disabled and matching the skip pattern) or leaves
there only opt-in-ed (those, specified as positional arguments).
This patch keeps both list-checking code close to each other so that the
intent is explicitly clear.
Signed-off-by: Pavel Emelyanov <xemul@scylladb.com>
Fixesscylladb/scylladb#17513
* 'gleb/raft-snitch-change-v3' of github.com:scylladb/scylla-dev:
doc: amend snitch changing procedure to work with raft
test: add test to check that snitch change takes effect.
raft topology: update rack/dc info in topology state on reboot if changed
To change snitch with raft all nodes need to be started simultaneously
since each node will try to update its state in the raft and for that
quorum is required.
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, instead of printing the `unique_ptr` instance, we
print the pointee of it. since `server_impl` uses pimpl paradigm,
`_fsm` is always valid after `server_impl::start()`, we can always
deference it without checking for null.
Refs #13245
Signed-off-by: Kefu Chai <kefu.chai@scylladb.com>
Closesscylladb/scylladb#17953
The test creates two node cluster with default snitch (SimpleSnitch) and
checks that dc and rack names are as expected. Then it changes the
config to use GossipingPropertyFileSnitch with different names, restart
nodes and check that now peers table has new names.
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<service::wait_for_ip_timeout>` is
added for backward compatibility with {fmt} < 10.
Refs scylladb#13245
Signed-off-by: Kefu Chai <kefu.chai@scylladb.com>
Closesscylladb/scylladb#17955
before this change, SCYLLA-{PRODUCT,VERSION,RELEASE}-FILE is generated
at the first run of `configure.py`, once these files are around, they
are not updated despite that `SCYLLA_VERSION_GEN` does not generate
them as long as the release string retrieved from git sha1 is identical
the one stored in `SCYLLA-RELEASE-FILE`, because we don't rerun
`SCYLLA_VERSION_GEN` at all.
but the pain is, when performing incremental build, like other built
artifacts, these generated files stay with the build directory, so
even if the sha1 of the workspace changes, the SCYLLA-RELEASE-FILE
keeps the same -- it still contains the original git sha1 when it
was created. this could leads to confusion if developer or even our
CI perform incremental build using the same workspace and build
directory, as the built scylla executables always report the same
version number.
in this change, we always rebuilt the said
SCYLLA-{PRODUCT,VERSION,RELEASE}-FILE files, and instruct ninja
to re-stat the output files, see
https://ninja-build.org/manual.html#ref_rule, in order to avoid
unnecessary rebuild. so the downside is that `SCYLLA_VERSION_GEN`
is executed every time we run `ninja` even if all targets are updated.
but the upside is that the release number reported by scylla is
accurate even if we perform incremental build.
also, since we encode the product, version and release stored
in the above files in the generated `build.ninja` file, in this change,
these three files are added as dependencies of `build.ninja`,
so that this file is regenerated if any of them is newer than
`build.ninja`.
Fixes#8255
Signed-off-by: Kefu Chai <kefu.chai@scylladb.com>
Closesscylladb/scylladb#17974
before this change, SCYLLA-{PRODUCT,VERSION,RELEASE}-FILE is generated
when CMake generate `build.ninja` for the first time, once these files
are around, they are not updated anymore. despite that
`SCYLLA_VERSION_GEN` does not generate them as long as the release
string retrieved from git sha1 is identical the one stored in
`SCYLLA-RELEASE-FILE`, because we don't rerun `SCYLLA_VERSION_GEN` at
all.
but the pain is, when performing incremental build, like other built
artifacts, these generated files stay with the build directory, so
even if the sha1 of the workspace changes, the SCYLLA-RELEASE-FILE
keeps the same -- it still contains the original git sha1 when it
was created. this could leads to confusion if developer or even our
CI perform incremental build using the same workspace and build
directory, as the built scylla executables always report the same
version number.
in this change, we always rebuilt the said
SCYLLA-{PRODUCT,VERSION,RELEASE}-FILE files, and instruct CMake
to regenerate `build.ninja` if any of these files is updated.
Fixes#17975
Signed-off-by: Kefu Chai <kefu.chai@scylladb.com>
Closesscylladb/scylladb#17983
codespell workflow checks for misspellings to identify common
mispellings. it considers "raison" in "raison d'etre" (the accent
mark over "e" is removed , so the commit message can be encoded in
ASCII), to the misspelling of "reason" or "raisin". apparently, the
dictionary it uses does not include les mots francais les plus
utilises.
so, in this change, let's ignore "raison" for this very use case,
before we start the l10n support of the document.
Signed-off-by: Kefu Chai <kefu.chai@scylladb.com>
Closesscylladb/scylladb#17985
since we already have `format_as()` for `cql3_type::raw`, there is no
need to provide `cql3_type::raw` if the tree is compiled with {fmt} >= 10,
otherwise compiler is not able to figure out which one to match, see the
errror at the end of this commit message. so, in this change, we only
provide the specialized `fmt::formatter` for `cql3_type::raw` when
{fmt} < 10. this should address the FTBFS with {fmt} >= 10.
```
/usr/lib/gcc/x86_64-redhat-linux/13/../../../../include/c++/13/type_traits:1040:25: error: ambiguous partial specializations of 'formatter<cql3::cql3_type::raw>'
1040 | = __bool_constant<__is_constructible(_Tp, _Args...)>;
| ^
/usr/lib/gcc/x86_64-redhat-linux/13/../../../../include/c++/13/type_traits:1046:16: note: in instantiation of template type alias '__is_constructible_impl' requested here
1046 | : public __is_constructible_impl<_Tp, _Args...>
| ^
/usr/include/fmt/core.h:1420:13: note: in instantiation of template class 'std::is_constructible<fmt::formatter<cql3::cql3_type::raw>>' requested here
1420 | !has_formatter<T, Context>::value))>
| ^
/usr/include/fmt/core.h:1421:22: note: while substituting prior template arguments into non-type template parameter [with T = cql3::cql3_type::raw]
1421 | FMT_CONSTEXPR auto map(const T&) -> unformattable_pointer {
| ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
1422 | return {};
| ~~~~~~~~~~
1423 | }
| ~
```
Refs #13245
Signed-off-by: Kefu Chai <kefu.chai@scylladb.com>
Closesscylladb/scylladb#17986
When openning a backport PR, adding a reference to the original PR.
This will be used later for updating the original PR/issue once the
backport is done (with different label)
Closesscylladb/scylladb#17973
The loader is writing to pending replica even when write selector is set
to previous. If migration is reverted, then the writes won't be rolled
back as it assumes pending replicas weren't written to yet. That can
cause data resurrection if tablet is later migrated back into the same
replica.
NOTE: write selector is handled correctly when set to next, because
get_natural_endpoints() will return the next replica set, and none
of the replicas will be considered leaving. And of course, selector
set to both is also handled correctly.
Fixes#17892.
Signed-off-by: Raphael S. Carvalho <raphaelsc@scylladb.com>
Closesscylladb/scylladb#17902
To cause the stale topology exception the test reads
the version from the last bootstrapped host and assigns its decremented
value to version and fence_version fields of system.topology.
The test assumes that version == fence_version here, if version
is greater than fence_version we won't get state topology
exception in this setup. Tablet balancer can break
this -- it may increment the version after the last node is
bootstrapped.
Fix this by disabling the tablet balancer earlier.
fixesscylladb/scylladb#17807Closesscylladb/scylladb#17940
This patch introduces raft-based service levels.
The difference to the current method of working is:
- service levels are stored in `system.service_levels_v2`
- reads are executed with `LOCAL_ONE`
- writes are done via raft group0 operation
Service levels are migrated to v2 in topology upgrade.
After the service levels are migrated, `key: service_level_v2_status; value: data_migrated` is written to `system.scylla_local` table. If this row is present, raft data accessor is created from the beginning and it handles recovery mode procedure (service levels will be read from v2 table even if consistent topology is disabled then)
Fixes#17926Closesscylladb/scylladb#16585
* github.com:scylladb/scylladb:
test: test service levels v2 works in recovery mode
test: add test for service levels migration
test: add test for service levels snapshot
test:topology: extract `trigger_snapshot` to utils
main: create raft dda if sl data was migrated
service:qos: store information about sl data migration
service:qos: service levels migration
main: assign standard service level DDA before starting group0
service:qos: fix `is_v2()` method
service:qos: add a method to upgrade data accessor
test: add unit_test_raft_service_levels_accessor
service:storage_service: add support for service levels raft snapshot
service:qos: add abort_source for group0 operations
service:qos: raft service level distributed data accessor
service:qos: use group0_guard in data accessor
cql3:statements: run service level statements on shard0 with raft guard
test: fix overrides in unit_test_service_levels_accessor
service:qos: fix indentation
service:qos: coroutinize some of the methods
db:system_keyspace: add `SERVICE_LEVELS_V2` table
service:qos: extract common service levels' table functions
In this PR, we ensure unpublished CDC generation's data is
never removed, which was theoretically possible. If it happened,
it could cause problems. CDC generation publisher would then try
to publish the generation with its data removed. In particular, the
precondition of calling `_sys_ks.read_cdc_generation` wouldn't be
satisfied.
We also add a test that passes only after the fix. However, this test
needs to block execution of the CDC generation publisher's loop
twice. Currently, error injections with handlers do not allow it
because handlers always share received messages. Apart from the
first created handler, all handlers would be instantly unblocked by
a message from the past that has already unblocked the first
handler. This seems like a general limitation that could cause
problems in the future, so in this PR, we extend injections with
handlers to solve it once and for all. We add the `share_messages`
parameter to the `inject` (with handler) function. Depending on its
value, handlers will share messages (as before) or not.
Fixesscylladb/scylladb#17497Closesscylladb/scylladb#17934
* github.com:scylladb/scylladb:
topology_coordinator: clean_obsolete_cdc_generations: fix log
topology_coordinator: do not clear unpublished CDC generation's data
topology_coordinator: cdc_generation_publisher_fiber injection: make handlers share messages
error_injection: allow injection handlers to not share messages
In this PR we add timeouts support to raft groups registry. We introduce
the `raft_server_with_timeouts` class, which wraps the `raft::server`
add exposes its interface with additional `raft_timeout` parameter. If
it's set, the wrapper cancels the `abort_source` after certain amount of
time. The value of the timeout can be specified either in the
`raft_timeout` parameter, or the default value can be set in `the
raft_server_with_timeouts` class constructor.
The `raft_group_registry` interface is extended with
`group0_with_timeouts()` method. It returns an instance of
`raft_server_with_timeouts` for group0 raft server. The timeout value
for it is configured in `create_server_for_group0`. It's one minute by
default and can be overridden for tests with
`group0-raft-op-timeout-in-ms` parameter.
The new api allows the client to decide whether to use timeouts or not.
In this PR we are reviewing all the group0 call sites and add
`raft_timeout` if that makes sense. The general principle is that if the
code is handling a client request and the client expects a potential
error, we use timeouts. We don't use timeouts for background fibers
(such as topology coordinator), since they wouldn't add much value. The
only thing the background fiber can do with a timeout is to retry, and
this will have the same end effect as not having a timeout at all.
Fixesscylladb/scylladb#16604Closesscylladb/scylladb#17590
* github.com:scylladb/scylladb:
migration_manager: use raft_timeout{}
storage_service::join_node_response_handler: use raft_timeout{}
storage_service::start_upgrade_to_raft_topology: use raft_timeout{}
storage_service::set_tablet_balancing_enabled: use raft_timeout{}
storage_service::move_tablet: use raft_timeout{}
raft_check_and_repair_cdc_streams: use raft_timeout{}
raft_timeout: test that node operations fail properly
raft_rebuild: use raft_timeout{}
do_cluster_cleanup: use raft_timeout{}
raft_initialize_discovery_leader: use raft_timeout{}
update_topology_with_local_metadata: use with_timeout{}
raft_decommission: use raft_timeout{}
raft_removenode: use raft_timeout{}
join_node_request_handler: add raft_timeout to make_nonvoters and add_entry
raft_group0: make_raft_config_nonvoter: add raft_timeout parameter
raft_group0: make_raft_config_nonvoter: add abort_source parameter
manager_client: server_add with start=false shouldn't call driver_connect
scylla_cluster: add seeds parameter to the add_server and servers_add
raft_server_with_timeouts: report the lost quorum
join_node_request_handler: add raft_timeout{} for start_operation
skip_mode: add platform_key
auth: use raft_timeout{}
raft_group0_client: add raft_timeout parameter
raft_group_registry: add group0_with_timeouts
utils: add composite_abort_source.hh
error_injection: move api registration to set_server_init
error_injection: add inject_parameter method
error_injection: move injection_name string into injection_shared_data
error_injection: pass injection parameters at startup
Reduce the sprawl of sstables::test_env in .cc and .hh files, to ease
maintenance and reduce recompilations.
Closesscylladb/scylladb#17965
* github.com:scylladb/scylladb:
test: sstables::test_env: complete pimplification
test/lib: test_env: move test_env::reusable_sst() to test_services.cc
* rename `sync_labels.yaml` to `sync-labels.yaml`
* use more descrptive name for workflow
Closesscylladb/scylladb#17971
* github.com:scylladb/scylladb:
github: sync-labels: use more descriptive name for workflow
github: sync_labels: rename sync_labels to sync-labels
When a keyspace uses tablets, then effective ownership
can be obtained per table. If the user passes only a
keyspace, then /storage_service/ownership/{keyspace}
returns an error.
This change:
- adds an additional positional parameter to 'status'
command that allows a user to query status for table
in a keyspace
- makes usage of /storage_service/ownership/{keyspace}
optional to avoid errors when user tries to obtain
effective ownership of a keyspace that uses tablets
- implements new frontend tests in 'test_status.py'
that verify the new logic
Refs: scylladb#17405
Signed-off-by: Patryk Wrobel <patryk.wrobel@scylladb.com>
Closesscylladb/scylladb#17827
