If the server is not started there is not point
in starting the driver, it would fail because there
are no nodes to connect to. On the other hand, we
should connect the driver in server_start()
if it's not connected yet.
If this parameter is set, we use its value for
the scylla.yaml of the new node, otherwise we
use IPs of all running nodes as before.
We'll need this parameter in subsequent commits to
restrict the communication between nodes.
We remove default values for _create_server_add_data parameters
since they are redundant - in the two call sites we pass all
of them.
In this commit we extend the timeout error message with
additional context - if we see that there is no quorum of
available nodes, we report this as the most likely
cause of the error.
We adjust the test by adding this new information to the
expected_error. We need raft-group-registry-fd-threshold-in-ms
to make _direct_fd threshold less than
group0-raft-op-timeout-in-ms.
In the test, we use the group0-raft-op-timeout-in-ms parameter to
reduce the timeout to one second so as not to waste time.
The join_node_request_handler method contains other group0 calls
which should have timeouts (make_nonvoters and add_entry). They
will be handled in a separate commit.
In subsequent commits we are going to add test.py
tests for raft_timeout{} feature. The problem is that
aarch/debug configuration is infamously slow. Timeout
settings used in tests work for all platforms but aarch/debug.
In this commit we extend the skip_mode attribute with the
platform_key property. We'll use @skip_mode('debug', platform_key='aarch64')
to skip the tests for this specific configuration.
The tests will still be run for aarch64/release.
The only place where we don't need raft_timeout{}
is migrate_to_auth_v2 since it's called from
topology_coordinator fiber. All other places are
called from user context, so raft_timeout{} is used.
In this commit we add raft_timeout parameter to
start_operation and add_entry method.
We fix compilation in default_authorizer.cc,
bind_front doesn't account for default parameter
values. We should use raft_timeout{} here, but this
is for another commit.
In this commit 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 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
get_server_with_timeouts(group_id) and group0_with_timeouts()
methods. They return an instance of raft_server_with_timeouts for
a specified group id or for group0. The timeout value for it is configured in
create_server_for_group0. It's one minute by default, 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 subsequent commits we are going to review all 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 won't
add much value. The only thing the background fiber can do
with a timeout is to retry, and this will have the same effect
as not having a timeout at all.
The set_server_done function is called only
when a node is fully initialized. To allow error
injection to be used during initialization we
move the handler registration to set_server_init,
which is called as soon as the api http server
is started.
In this commit we extend the error_injector
with a new method inject_parameter. It allows
to pass parameters from tests to scylla, e.g. to
lower timeouts or limits. A typical use cases is
described in scylladb/scylladb#15571.
It's logically the same as inject_with_handler,
whose lambda reads the parameter named 'value'.
The only difference is that the inject_parameter
doesn't return future, it just read the
parameter from the injection shared_data.
In subsequent commit we'll need the injection_name from inside
injection_shared_data, so in this commit we move it there.
Additionally, we fix the todo about switching the injections dictionary
from map to unordered_set, now unordered_map contains
string_views, pointing to injection_name inside
injection_shared_data.
Injection parameters can be used in the lambda passed to
inject_with_handler method to take some values from
the test. However, there was no way to set values to these
parameters on node startup, only through
the error injection REST api. Therefore, we couldn't rely
on this when inject_with_handler is used during
node startup, it could trigger before we call the api
from the test.
In this commit with solve this problem by allowing these
parameters to be assigned through scylla.yaml config.
The defer.hh header was added to error_injection.hh to fix
compilation after adding error_injection.hh to config.hh,
defer function is used in error_injection.hh.
sstables_manager now depends on system_keyspace for access to the
system.sstables table, needed by object storage. This violates
modularity, since sstables_manager is a relatively low-level leaf
module while system_keyspace integrates large parts of the system
(including, indirectly, sstables_manager).
One area where this is grating is sstables::test_env, which has
to include the much higher level cql_test_env to accommodate it.
Fix this by having sstables_manager expose its dependency on
system_keyspace as an interface, sstables_registry, and have
system_keyspace implement the glue logic in
system_keyspace_sstables_manager.
Closesscylladb/scylladb#17868
This commit updates the Upgrade ScyllaDB Image page.
- It removes the incorrect information that updating underlying OS packages is mandatory.
- It adds information about the extended procedure for non-official images.
Closesscylladb/scylladb#17867
group0 operations a valid on shard 0 only. Assert that. We already do
that in the version of the function that gets abort source.
Message-ID: <ZeCti70vrd7UFNim@scylladb.com>
Lot's of BOOST_REQUIRES in this test require some integers to be in some
eq/gt/le relations to each other. And one place that compares rack names
as strings. Using more verbose boost checkers is preferred in such cases
Signed-off-by: Pavel Emelyanov <xemul@scylladb.com>
Closesscylladb/scylladb#17866
this change is a follow up of ca7f7bf8e2, which changed the output path to build/$<CONFIG>/debian. but what dist/docker/debian/build_docker.sh expects is `build/dist/$config/debian/*.deb`, where `$config` is the normalized mode, when the debian packages are built using CMake generated rules, `$mode` is CMake configuration name, i.e., `$<CONFIG>`. so, ca7f7bf8e2 made a mistake, as it does not match the expectation of `build_docker.sh`.
in this change, this issue is addressed. so we use the same path in both `dist/CMakeLists.txt` and `dist/docker/debian/build_docker.sh`.
Closesscylladb/scylladb#17848
* github.com:scylladb/scylladb:
build: cmake: add dist-* targets to the default build target
build: cmake: put server deb packages under build/dist/$<CONFIG>/debian
This PR fixes a problem with replacing a node with tablets when
RF=N. Currently, this will fail because tablet replica allocation for
rebuild will not be able to find a viable destination, as the replacing node
is not considered to be a candidate. It cannot be a candidate because
replace rolls back on failure and we cannot roll back after tablets
were migrated.
The solution taken here is to not drain tablet replicas from replaced
node during topology request but leave it to happen later after the
replaced node is in left state and replacing node is in normal state.
The replacing node waits for this draining to be complete on boot
before the node is considered booted.
Fixes https://github.com/scylladb/scylladb/issues/17025
Nodes in the left state will be kept in tablet replica sets for a while after node
replace is done, until the new replica is rebuilt. So we need to know
about those node's location (dc, rack) for two reasons:
1) algorithms which work with replica sets filter nodes based on their location. For example materialized views code which pairs base replicas with view replicas filters by datacenter first.
2) tablet scheduler needs to identify each node's location in order to make decisions about new replica placement.
It's ok to not know the IP, and we don't keep it. Those nodes will not
be present in the IP-based replica sets, e.g. those returned by
get_natural_endpoints(), only in host_id-based replica
sets. storage_proxy request coordination is not affected.
Nodes in the left state are still not present in token ring, and not
considered to be members of the ring (datacanter endpoints excludes them).
In the future we could make the change even more transparent by only
loading locator::node* for those nodes and keeping node* in tablet replica sets.
Currently left nodes are never removed from topology, so will
accumulate in memory. We could garbage-collect them from topology
coordinator if a left node is absent in any replica set. That means we
need a new state - left_for_real.
Closesscylladb/scylladb#17388
* github.com:scylladb/scylladb:
test: py: Add test for view replica pairing after replace
raft, api: Add RESTful API to query current leader of a raft group
test: test_tablets_removenode: Verify replacing when there is no spare node
doc: topology-on-raft: Document replace behavior with tablets
tablets, raft topology: Rebuild tablets after replacing node is normal
tablets: load_balancer: Access node attributes via node struct
tablets: load_balancer: Extract ensure_node()
mv: Switch to using host_id-based replica set
effective_replication_map: Introduce host_id-based get_replicas()
raft topology: Keep nodes in the left state to topology
tablets: Introduce read_required_hosts()
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 two different tests
* row_cache_stress_test::reader_id
and drop its operator<<.
Refs #13245
Signed-off-by: Kefu Chai <kefu.chai@scylladb.com>
Closesscylladb/scylladb#17861
Clang > 12 starts to complain like
```
warning: '-fuse-ld=' taking a path is deprecated; use '--ld-path=' instead [-Wfuse-ld-path]'
```
this option is not supported by GCC yet. also instead of using
the generic driver's name, use the specific name. otherwise ld
fails like
```
lld is a generic driver.
Invoke ld.lld (Unix), ld64.lld (macOS), lld-link (Windows), wasm-ld (WebAssembly) instead
```
Signed-off-by: Kefu Chai <kefu.chai@scylladb.com>
Closesscylladb/scylladb#17825
For now test is incomplete in several ways
1. It xfails, until #17116
2. It doesn't rebuild/repair tablets
3. It doesn't check that tablet data actually exists on replicas
refs: #17575
Signed-off-by: Pavel Emelyanov <xemul@scylladb.com>
Closesscylladb/scylladb#17808
When Mergify open a backport PR and identify conflicts, it adding the
`conflicts` label. Since GitHub can't identify conflicts in PR, setting
a role to move PR to draft, this way we will not trigger CI
Once we resolve the conflicts developer should make the PR `ready for
review` (which is not draft) and then CI will be triggered
`conflict` label can also be removed
Closesscylladb/scylladb#17834
Currently, when dividing memory tracked for a batch of updates
we do not take into account the overhead that we have for processing
every update. This patch adds the overhead for single updates
and joins the memory calculation path for batches and their parts
so that both use the same overhead.
Fixes#17854Closesscylladb/scylladb#17855
also, add a target of `dist-server`, which mirrors the structure
of the targets created by `configure.py`, and it is consistent
with the ones defined by `build_submodule()`.
so that they are built when our CI runs `ninja -C $build`. CI
expects that all these rpm and deb packages to built when
`ninja -C $build` finishes. so that it can continue with
building the container image. let's make it happen. so that
the CMake-based rules can work better with CI.
Signed-off-by: Kefu Chai <kefu.chai@scylladb.com>
It was observed that some use cases might append old data constantly to
memtable, blocking GC of expired tombstones.
That's because timestamp of memtable is unconditionally used for
calculating max purgeable, even when the memtable doesn't contain the
key of the tombstone we're trying to GC.
The idea is to treat memtable as we treat L0 sstables, i.e. it will
only prevent GC if it contains data that is possibly shadowed by the
expired tombstone (after checking for key presence and timestamp).
Memtable will usually have a small subset of keys in largest tier,
so after this change, a large fraction of keys containing expired
tombstones can be GCed when memtable contains old data.
Fixes#17599.
Signed-off-by: Raphael S. Carvalho <raphaelsc@scylladb.com>
Closesscylladb/scylladb#17835
There is no point in checking `sst->filter_has_key(*hk)`
if the sstable contains no data older than the running
minimum timestamp, since even if it matches, it won't change
the minimum.
Signed-off-by: Benny Halevy <bhalevy@scylladb.com>
Closesscylladb/scylladb#17839
Fix writing cassandra-rackdc.properties with correct format data instead of yaml
Add a parameter to overwrite RF for specific DC
Add the possibility to connect cql to the specific node
In this PR 4 tests were added to test multi-DC functionality. One is added from initial commit were multi-DC possibility were introduced, however, this test was not commited. Three of them are migrations from dtest, that later will be deleted. To be able to execute migrated tests additional functionality is added: the ability to connect cql to the specific node in the cluster instead of pooled connection and the possibility to overwrite the replication factor for the specific DC. To be able to use the multi DC in test.py issue with the incorrect format of the properties file fixed in this PR.
Closesscylladb/scylladb#17503
With large schemas, unfreezing can stall, especially as it requires
a lot of memory. Switch to a gentle version that will not stall.
As a preparation step, we add unfreeze_gently() for a span of mutations.
Fixes#17841Closesscylladb/scylladb#17842
* github.com:scylladb/scylladb:
schema_tables: unfreeze frozen_mutation:s gently
frozen_mutation: add unfreeze_gently(span<frozen_mutation>)
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 `perf_result_with_aio_writes`,
and drop its operator<<.
Refs #13245
Signed-off-by: Kefu Chai <kefu.chai@scylladb.com>
Closesscylladb/scylladb#17849
Newly joining nodes may not have a host id yet. Handle this and print a
"?" for these nodes, instead of the host-id.
Extend the existing test for joining node case (also rename it and add
comment).
Closesscylladb/scylladb#17853
this change is a follow up of ca7f7bf8e2, which changed the output path
to build/$<CONFIG>/debian. but what dist/docker/debian/build_docker.sh
expects is `build/dist/$config/debian/*.deb`, where `$config` is the
normalized mode, when the debian packages are built using CMake
generated rules, `$mode` is CMake configuration name, i.e., `$<CONFIG>`.
so, ca7f7bf8e2 made a mistake, as it does not match the expectation of
`build_docker.sh`.
in this change, this issue is addressed. so we use the same path
in both `dist/CMakeLists.txt` and `dist/docker/debian/build_docker.sh`.
apply the same change to `dist-server-rpm`.
Signed-off-by: Kefu Chai <kefu.chai@scylladb.com>
test/raft/replication.cc defines a symbol named `tlogger`, while
test/raft/randomized_nemesis_test.cc also defines a symbol with
the same name. when linking the test with mold, it identified the ODR
violation.
in this change, we extract test-raft-helper out, so that
randomized_nemesis_test can selectively only link against this library.
this also matches with the behavior of the rules generated by `configure.py`.
Signed-off-by: Kefu Chai <kefu.chai@scylladb.com>
Closesscylladb/scylladb#17836
in gossiping_property_file_snitch_test, we use
`BOOST_REQUIRE_EQUAL(dc_racks[i], dc_racks[0])` to check the equality
of two instances of `pair<sstring, sstring`, like:
```c++
BOOST_REQUIRE_EQUAL(dc_racks[i], dc_racks[0])
```
since the standard library does not provide the formatter for printing
`std::pair<>`, we rely on the homebrew generic formatter to
print `std::pair<>, which in turn uses operator<< to format the
elements in the `pair`, but we intend to remove this formatter
in future, as the last step of #13245 .
so in order to enable Boost.test to print out lhs and rhs when
`BOOST_REQUIRE_EQUAL` check fails, we are adding
`boost_test_print_type()` for `pair<sstring,sstring>`. the helper
function uses {fmt} to print the `pair<>`.
Refs #13245
Signed-off-by: Kefu Chai <kefu.chai@scylladb.com>
Closesscylladb/scylladb#17831
this change prepares for the fmt::formatter based formatter used by
tests, which will use {fmt} to print the elements in a container,
so we need to define the formatter using fmt::formatter for these
element. the operator<< for service_level_options::workload_type is
preserved, as the tests are still using it.
Refs #13245
Signed-off-by: Kefu Chai <kefu.chai@scylladb.com>
Closesscylladb/scylladb#17837
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
row_level_diff_detect_algorithm. please note, we already have
`format_as()` overload for this type, but we cannot use it as a
fallback of the proper `fmt::formatter<>` specialization before
{fmt} v10. so before we update our CI to a distro with {fmt} v10,
`fmt::formatter<row_level_diff_detect_algorithm>` is still
needed.
Refs #13245
Signed-off-by: Kefu Chai <kefu.chai@scylladb.com>
Closesscylladb/scylladb#17824
The assert_that_failed(future) pair of helpers are templates with
variadic futures, but since they are gone in seastar, so should they in
test/lib
Signed-off-by: Pavel Emelyanov <xemul@scylladb.com>
Closesscylladb/scylladb#17830
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 `db::functions::function`.
please note, because we use `std::ostream` as the parameter of
the polymorphism implementation of `function::print()`.
without an intrusive change, we have to use `fmt::ostream_formatter`
or at least use similar technique to format the `function` instance
into an instance of `ostream` first. so instead of implementing
a "native" `fmt::formatter`, in this change, we just use
`fmt::ostream_formatter`.
Refs #13245
Signed-off-by: Kefu Chai <kefu.chai@scylladb.com>
Closesscylladb/scylladb#17832
Empty histograms are missing some of the members that non-empty
histograms have. The code handling these histograms assumed all required
members are always present and thus error out when receiving an empty
histogram.
Add tests for empty histograms and fix the code handling them to check
for the potentially missing members, instead of making assumptions.
Closesscylladb/scylladb#17816
The test is changed to be more strict. Verifies the case of replacing
when RF=N in which case tablet replicas have to be rebuilt using the
replacing node.
This would fail if tablets are drained as part of replace operation,
since replacing node is not yet a viable target for tablet migration.
