In the view update code, the function get_view_natural_endpoint()
determines which view replica this base replica should send an update
to. It currently gets the *view* table's replication map (i.e., the map
from view tokens to lists of replicas holding the token), but assumes
that this is also the *base* table's replication map.
This assumption was true with vnodes, but is no longer true with
tablets - the base table's replication map can be completely different
from the view table's. By looking at the wrong mapping,
get_view_natural_endpoint() can believe that this node isn't really
a base-replica and drop the view update. Alternatively, it can think
it is a base replica - but use the wrong base-view pairing and create
base-view inconsistencies.
This patch solves this bug - get_view_natural_endpoint() now gets two
separate replication maps - the base's and the view's. The callers
need to remember what the base table was (in some cases they didn't
care at the point of the call), and pass it to the function call.
This patch also includes a simple test that reproduces the bug, and
confirms it is fixed: The test has a 6-node cluster using tablets
and a base table with RF=1, and writes one row to it. Before this
patch, the code usually gets confused, thinking the base replica
isn't a replica and loses the view update. With this patch, the
view update works.
Fixes#16227.
Signed-off-by: Nadav Har'El <nyh@scylladb.com>
Closesscylladb/scylladb#16228
Prototype implementation of format suggested/requested by @avikivity:
Divides segments into disk-write-alignment sized pages, each tagged with segment ID + CRC of data content.
When read, we both verify sector integrity (CRC) to detect corruption, as well as matching ID read with expected one.
If the latter mismatches we have a prematurely terminated segment (read truncation), which, depending on whether the CL is
written in batch or periodic mode, as well as explicit sync, can mean data loss.
Note: all-zero pages are treated as kosher, both to align with newly allocated segments, as well as fully terminated (zero-page) ones.
Note: This is a preview/RFC - the rest of the file format is not modified. At least parts of entry CRC could probably be removed, but I have not done so yet (needs some thinking).
Note: Some slight abstraction breaks in impl. and probably less than maximal efficiency.
v2:
* Removed entry CRC:s in file format.
* Added docs on format v3
* Added one more test for recycling-truncation
v3:
* Fixed typos in size calc and docs
* Changed sect metadata order
* Explicit iter type
Closesscylladb/scylladb#15494
* github.com:scylladb/scylladb:
commitlog_test: Add test for replaying large-ish mutation
commitlog_test: Add additional test for segmnent truncation
docs: Add docs on commitlog format 3
commitlog: Remove entry CRC from file format
commitlog: Implement new format using CRC:ed sectors
commitlog: Add iterator adaptor for doing buffer splitting into sub-page ranges
fragmented_temporary_buffer: Add const iterator access to underlying buffers
commitlog_replayer: differentiate between truncated file and corrupt entries
The helper in question complicates the logic of sstable_directory::process() by making garbage collection differently for sstables deleted "atomically" and deleted "one-by-one". Also, the code that deletes sstables one-by-one and uses remove_by_toc_name() renders excessive TOC file reading, because there's sstable object at hand and it had all_components() ready for use.
Surprisingly, there was no test for the deletion-log functionality. This PR adds one. The test passes before the g.c. and regular unlink fix, and (of course) continues passing after it.
Closesscylladb/scylladb#16240
* github.com:scylladb/scylladb:
sstables: Drop remove_by_name()
sstables/fs_storage: Wipe by recognized+unrecognized components
sstable_directory: Enlight deletion log replay
sstables: Split remove_by_toc_name()
test: Add test case to validate deletion log work
sstable_directory: Close dir on exception
sstable_directory: Fix indentation after previous patch
sstable_directory: Coroutinize delete_with_pending_deletion_log()
test: Sstable on_delete() is not necessarily in a thread
sstable_directory: Split delete_with_pending_deletion_log()
Fixes some typos as found by codespell run on the code. In this commit, I was hoping to fix only comments, not user-visible alerts, output, etc. Follow-up commits will take care of them.
Refs: https://github.com/scylladb/scylladb/issues/16255Closesscylladb/scylladb#16257
* github.com:scylladb/scylladb:
Update service/topology_state_machine.hh
Update raft/tracker.hh
Update db/view/view.cc
Typos: fix typos in comments
Fixes some typos as found by codespell run on the code.
In this commit, I was hoping to fix only comments, not user-visible alerts, output, etc.
Follow-up commits will take care of them.
Refs: https://github.com/scylladb/scylladb/issues/16255
Signed-off-by: Yaniv Kaul <yaniv.kaul@scylladb.com>
This PR is a necessary step to fix#15854 -- making consistent
cluster management mandatory on master.
Before making consistent cluster management mandatory, we have
to get rid of all tests that depend on the
`consistent_cluster_management=false` config. These are the tests
in the `topology_raft_disabled` suite.
There's the internal Raft upgrade procedure, which is the bulk of the
upgrade logic. Then, there are two thin "layers" around it that
invoke it underneath: recovery procedure and
enable-raft-in-the-cluster procedure. We're getting rid of the
second one by making Raft always enabled, so we naturally have to
get rid of tests that depend on it. The idea is to replace every
necessary enable-raft-in-the-cluster procedure in these tests with
the recovery procedure. Then, we will still be testing the internal
Raft upgrade procedure in the in-tree tests. The
enable-raft-in-the-cluster procedure is already tested by QA tests,
so we don't need to worry about these changes.
Unfortunately, we cannot adapt `test_raft_upgrade_no_schema`.
After making consistent cluster management mandatory on master,
schema commitlog will also become mandatory because
`consistent_cluster_management: True`,
`force_schema_commit_log: False`
is considered a bad configuration. These changes will make
`test_raft_upgrade_no_schema` unimplementable in the Scylla repo.
Therefore, we remove this test. If we want to keep it, we must
rewrite it as an upgrade dtest.
After making all tests in `topology_raft_disabled` use consistent
cluster management, there is no point in keeping this suite.
Therefore, we delete it and move all the tests to `topology_custom`.
Closesscylladb/scylladb#16192
* github.com:scylladb/scylladb:
test: delete topology_raft_disabled suite
test: topology_raft_disabled: move tests to topology_custom suite
test: topology_raft_disabled: move utils to topology suite
test: topology_raft_disabled: use consistent cluster management
test: topology_raft_disabled: add new util functions
test: topology_raft_disabled: delete test_raft_upgrade_no_schema
The test sequence is
- create several sstables
- create deletion log for a sub-set of them
- partially unlink smaller sub-sub-set
- make sstable directory do the processing with g.c.
- check that the sstables loaded do NOT include the deleted ones
The .throw_on_missing_toc bit set additionally validates that the
directory doesn't contain garbage not attached to any other TOCs
Signed-off-by: Pavel Emelyanov <xemul@scylladb.com>
One of the test cases injects an observer into sstable->unlink() method
via its _on_delete() callback. The test's callback assumes that it runs
in an async context, but it's a happy coincidence, because deletion via
the deletion log runs so. Next patch is changing it and the test case
will no longer work. But since it's a test case it can just directly
call a libc function for its needs
Signed-off-by: Pavel Emelyanov <xemul@scylladb.com>
Add `LIST EFFECTIVE SERVICE LEVEL` statement to be able to display from which service level come which service level options.
Example:
There are 2 roles: role1 and role2. Role1 is assigned with sl1 (timeout = 2s, workload_type = interactive) and role2 is assigned with sl2 (timeout = 10s, workload_type = batch).
Then, if we grant role1 to role2, the user with role2 will have 2s timeout (from sl1) and batch workload type (from sl2).
```
> LIST EFFECTIVE SERVICE LEVEL OF role2;
service_level_option | effective_service_level | value
----------------------+-------------------------+-------------
workload_type | sl2 | batch
timeout | sl1 | 2s
```
Fixes: https://github.com/scylladb/scylladb/issues/15604Closesscylladb/scylladb#14431
* github.com:scylladb/scylladb:
cql-pytest: add `LIST EFFECTIVE SERVICE LEVEL OF` test
docs: add `LIST EFFECTIVE SERVICE LEVEL` statement docs
cql3:statements: add `LIST EFFECTIVE SERVICE LEVEL` statement
service:qos: add option to include effective names to SLO
We move the remaining tests in topology_raft_disabled to
topology_custom. We choose topology_custom because these tests
cannot use consistent topology changes.
We need to modify these tests a bit because we cannot pass
RandomTables to a test case function if the initial cluster size
equals 0. RandomTables.__init__ requires manager.cql to be present.
We move all used util functions from topology_raft_disabled to
topology before we remove topology_raft_disabled. After this
change, util.py in topology will be the single util file for all
topology tests.
Some util functions in topology_raft_disabled aren't used anymore.
We don't move such functions and remove them instead.
Soon, we will make consistent cluster management mandatory on
master. Before this, we have to change all tests in the
topology_raft_disabled suite so that they do not depend on the
consistent_cluster_management=false config.
Adapting test_raft_upgrade_majority_loss is simple. We only have
to get rid of the initial upgrade. This initial upgrade didn't
test anything. Every test in topology_raft_disabled had to do it
at the beginning because of consistent_cluster_management=false.
Adapting test_raft_upgrade_basic and test_raft_upgrade_stuck is
more difficult. It requires changing the initial upgrade to
clearing Raft data in RECOVERY mode on all servers and restarting
them. Then, the servers will run the same upgrade procedure as
before.
After changing the tests, we also update their names appropriately.
test_raft_upgrade_stuck becomes a bit slower, so we remove the
comment about running time. Also, one TODO was fixed in the process
of rewriting the test. This fix forced us to skip the test in the
release mode since we cannot update the list of error injections
through manager.server_update_config in this mode.
After making consistent cluster management mandatory on master,
schema commitlog will also become mandatory because
consistent_cluster_management: True,
force_schema_commit_log: False
is considered a bad configuration. These changes will make
test_raft_upgrade_no_schema unimplementable in the Scylla repo, so
we remove it.
If we want to keep this test, we must rewrite it as an upgrade
dtest.
under most circumstances, we don't care the ordering of the sstable
identifiers, as they are just identifiers. so, as long as they can be
compared, we are good. but we have tests with expect that the sstables
can be ordered by the time they are created. for instance,
sstable_run_based_compaction_test has this expectaion.
before this change, we compare two UUID-based generations by its
(MSB, LSB) lexicographically. but UUID v1 put the lower bits of
the timestamp at the higher bits of MSB, so the ordering of the
"time" in timeuuid is not preserved when comparing the UUID-based
generations. this breaks the test of sstable_run_based_compaction_test,
which feeds the sstables to be compacted in a set, and the set is
ordered with the generation of the sstables.
after this change, we consider the UUID-based generation as
a timeuuid when comparing them.
Fixes#16215
Signed-off-by: Kefu Chai <kefu.chai@scylladb.com>
Closesscylladb/scylladb#16238
This series adds handling for more failures during a topology operation
(we already handle a failure during streaming). Here we add handling of
tablet draining errors by aborting the operation and handling of errors
after streaming where an operation cannot be aborted any longer. If the
error happens when rollback is no longer possible we wait for ring delay
and proceed to the next step. Each individual patch that adds the sleep
has an explanation what the consequences of the patch are.
* 'gleb/topology-coordinator-failures' of github.com:scylladb/scylla-dev:
test: add test to check errro handling during tablet draining
test: fix test_topology_streaming_failure test to not grep the whole file
storage_service: add error injection into the tablet migration code
storage_service: topology coordinator: rollback on handle_tablet_migration failure during tablet_draining stage
storage_service: topology coordinator: do not retry the metadata barrier forever in write_both_read_new state
storage_service: topology coordinator: do not retry the metadata barrier forever in left_token_ring state
storage_service: topology coordinator: return a node that is being removed from get_excluded_nodes
storage_service: topology_coordinator: use new rollback_to_normal state in the rollback procedure
storage_service: topology coordinator: add rollback_to_normal node state
storage_service: topology coordinator: put fence version into the raft state
storage_service: topology coordinator: do fencing even if draining failed
TOC file is read and parsed in several places in the code. All do it differently, and it's worth generalizing this place.
To make it happen also fix the S3 readable_file so that it could be used inside file_input_stream.
Closesscylladb/scylladb#16175
* github.com:scylladb/scylladb:
sstable: Generalize toc file read and parse
s3/client: Don't GET object contents on out-of-bound reads
s3/client: Cache stats on readable_file
This situation before this patch is that when tablets are enabled for
a keyspace, we can create a materialized view but later any write to
the base table fails with an on_internal_error(), saying that:
"Tried to obtain per-keyspace effective replication map of test
but it's per-table."
Indeed, with tablets, the replication is different for each table - it's
not the same for the entire keyspace.
So this patch changes the view update code to take the replication
map from the specific base table, not the keyspace.
This is good enough to get materialized-views reads and writes working
in a simple single-node case, as the included test demonstrates (the
test fails with on_internal_error() before this patch, and passes
afterwards).
But this fix is not perfect - the base-view pairing code really needs
to consider not only the base table's replication map, but also the
view table's replication map - as those can be different. We'll fix
this remaining problem as a followup in a separate patch - it will
require a substantially more elaborate test to reproduce the need
for the different mapping and to verify that fix.
Fixes#16209.
Signed-off-by: Nadav Har'El <nyh@scylladb.com>
Closesscylladb/scylladb#16211
If S3 readable file is used inside file input stream, the latter may
call its read methods with position that is above file size. In that
case server replies with generic http error and the fact that the range
was invalid is encoded into reply body's xml.
That's not great to catch this via wrong reply status exception and xml
parsing all the more so we can know that the read is out-of-bound in
advance.
Signed-off-by: Pavel Emelyanov <xemul@scylladb.com>
Fixes#16207
commitlog::delete_segments deletes (or recycles) segments replayed.
The actual file size here is added to footprint so actual delete then
can determine iff things should be recycled or removed.
However, we build a pending delete list of named_files, and the files
we added did not have size set. Bad. Actual deletion then treated files
as zero-byte sized, i.e. footprint calculations borked.
Simple fix is just filling in the size of the objects when addind.
Added unit test for the problem.
Closesscylladb/scylladb#16210
Major compaction already flushes each table to make
sure it considers any mutations that are present in the
memtable for the purpose of tombstone purging.
See 64ec1c6ec6
However, tombstone purging may be inhibited by data
in commitlog segments based on `gc_time_min` in the
`tombstone_gc_state` (See f42eb4d1ce).
Flushing all sstables in the database release
all references to commitlog segments and there
it maximizes the potential for tombstone purging,
which is typically the reason for running major compaction.
However, flushing all tables too frequently might
result in tiny sstables. Since when flushing all
keyspaces using `nodetool flush` the `force_keyspace_compaction`
api is invoked for keyspace successively, we need a mechanism
to prevent too frequent flushes by major compaction.
Hence a `compaction_flush_all_tables_before_major_seconds` interval
configuration option is added (defaults to 24 hours).
In the case that not all tables are flushed prior
to major compaction, we revert to the old behavior of
flushing each table in the keyspace before major-compacting it.
Fixesscylladb/scylladb#15777Closesscylladb/scylladb#15820
* github.com:scylladb/scylladb:
docs: nodetool: flush: enrich examples
docs: nodetool: compact: fix example
api: add /storage_service/compact
api: add /storage_service/flush
compaction_manager: flush_all_tables before major compaction
database: add flush_all_tables
api: compaction: add flush_memtables option
test/nodetool: jmx: fix path to scripts/scylla-jmx
scylla-nodetool, docs: improve optional params documentation
Before this patch, trying to create a materialized view when tablets
are enabled for a keyspace results in a failure: "Tablet map not found
for table <uuid>", with uuid referring to the new view.
When a table schema is created, the handler on_before_create_column_family()
is called - and this function creates the tablet map for the new table.
The bug was that we forgot to do the same when creating a materialized
view - which also a bona-fide table.
In this patch we call on_before_create_column_family() also when
creating the materialized view. I decided *not* to create a new
callback (e.g., on_before_create_view()) and rather call the existing
on_before_create_column_family() callback - after all, a view is
a column family too.
This patch also includes a test for this issue, which fails to create
the view before this patch, and passes with the patch. The test is
in the test/topology_experimental_raft suite, which runs Scylla with
the tablets experimental feature, and will also allow me to create
tests that need multiple nodes. However, the first test added here
only needs a single node to reproduce the bug and validate its fix.
Fixes#16194.
Signed-off-by: Nadav Har'El <nyh@scylladb.com>
Closesscylladb/scylladb#16205
For major compacting all tables in the database.
The advantage of this api is that `commitlog->force_new_active_segment`
happens only once in `database::flush_all_tables` rather than
once per keyspace (when `nodetool compact` translates to
a sequence of `/storage_service/keyspace_compaction` calls).
Signed-off-by: Benny Halevy <bhalevy@scylladb.com>
For flushing all tables in the database.
The advantage of this api is that `commitlog->force_new_active_segment`
happens only once in `database::flush_all_tables` rather than
once per keyspace (when `nodetool flush` translates to
a sequence of `/storage_service/keyspace_flush` calls).
Signed-off-by: Benny Halevy <bhalevy@scylladb.com>
When flushing is done externally, e.g. by running
`nodetool flush` prior to `nodetool compact`,
flush_memtables=false can be passed to skip flushing
of tables right before they are major-compacted.
This is useful to prevent creation of small sstables
due to excessive memtable flushing.
Signed-off-by: Benny Halevy <bhalevy@scylladb.com>
The current implementation makes no sense.
Like `nodetool_path`, base the default `jmx_path`
on the assumption that the test is run using, e.g.
```
(cd test/nodetool; pytest --nodetool=cassandra test_compact.py)
```
Signed-off-by: Benny Halevy <bhalevy@scylladb.com>
Nowadays if memtable gets flushed into misconfigured S3 storage, the flush fails and aborts the whole scylla process. That's not very elegant. First, because upon restart garbage collecting non-sealed sstables would fail again. Second, because re-configuring an endpoint can be done runtime, scylla re-reads this config upon HUP signal.
Flushing memtable restarts when seeing ENOSPC/EDQUOT errors from on-disk sstables. This PR extends this to handle misconfigured S3 endpoints as well.
fixes: #13745Closesscylladb/scylladb#15635
* github.com:scylladb/scylladb:
test: Add object_store test to validate config reloading works
test: Add config update facility to test cluster
test: Make S3_Server export config file as pathlib.Path
config: Make object storage config updateable_value_source
memtable: Extend list of checking codes
sstables/storage/s3: Fix missing TOC status check
s3/client: Map http exceptions into storage_io_error
exceptions: Extend storage_io_error construction options
Compaction tasks which do not have a parent are abortable
through task manager. Their children are aborted recursively.
Compaction tasks of the lowest level are aborted using existing
compaction task executors stopping mechanism.
Closesscylladb/scylladb#16177
* github.com:scylladb/scylladb:
test: test abort of compaction task that isn't started yet
test: test running compaction task abort
tasks: fail if a task was aborted
compaction: abort task manager compaction tasks
The implementation of "SELECT TOJSON(t)" or "SELECT JSON t" for a column
of type "time" forgot to put the time string in quotes. The result was
invalid JSON. This is patch is a one-liner fixing this bug.
This patch also removes the "xfail" marker from one xfailing test
for this issue which now starts to pass. We also add a second test for
this issue - the existing test was for "SELECT TOJSON(t)", and the second
test shows that "SELECT JSON t" had exactly the same bug - and both are
fixed by the same patch.
We also had a test translated from Cassandra which exposed this bug,
but that test continues to fail because of other bugs, so we just
need to update the xfail string.
The patch also fixes one C++ test, test/boost/json_cql_query_test.cc,
which enshrined the *wrong* behavior - JSON output that isn't even
valid JSON - and had to be fixed. Unlike the Python tests, the C++ test
can't be run against Cassandra, and doesn't even run a JSON parser
on the output, which explains how it came to enshrine wrong output
instead of helping to discover the bug.
Fixes#7988
Signed-off-by: Nadav Har'El <nyh@scylladb.com>
Closesscylladb/scylladb#16121
- remove some code that is obsolete in newer Scylla versions,
- fix some minor bugs. These bugs appear to be benign, there are no known issues caused by them, but fixing them is a good idea nevertheless,
- refactor some code for better maintainability.
Parts of this PR were extracted from https://github.com/scylladb/scylladb/pull/15331 (which was merged but later reverted), parts of it are new.
Closesscylladb/scylladb#16162
* github.com:scylladb/scylladb:
test/pylib: log_browsing: fix type hint
migration_manager: take `abort_source&` in get_schema_for_read/write
migration_manager: inline merge_schema_in_background
migration_manager: remove unused merge_schema_from overload
migration_manager: assume `canonical_mutation` support
migration_manager: add `std::move` to avoid a copy
schema_tables: refactor `scylla_tables(schema_features)`
schema_tables: pass `reload` flag when calling `merge_schema` cross-shard
system_keyspace: fix outdated comment
`run_first` lists in `suite.yaml` files provide a simple way to
shorten the tests' average running time by running the slowest
tests at first.
We update these lists, since they got outdated over time:
- `test_topology_ip` was renamed to `test_replace`
and changed suite,
- `test_tablets` changed suite,
- new slow tests were added:
- `test_cluster_features`,
- `test_raft_cluster_features`,
- `test_raft_ignore_nodes`,
- `test_read_repair`.
Closesscylladb/scylladb#16104
We add a test for concurrent bootstrap support in the raft-based
topology.
The plan is to make this test temporary. In the future, we will:
- use ManagerClient.servers_add in other tests wherever possible,
- start initial servers concurrently in all suites with
initial_size > 0.
So, this test will not test anything unique.
We could make the changes proposed above now instead of adding
this small test. However, if we did that and it turned out that
concurrent bootstrap is flaky in CI, we would make almost every CI
run fail with many failures. We want to avoid such a situation.
Running only this test for some time in CI will reduce the risk
and make investigating any potential failures easier.
We add a new function - servers_add - that allows adding multiple
servers concurrently to a cluster. It makes use of a concurrent
bootstrap now supported in the raft-based topology.
servers_add doesn't have the replace_cfg parameter. The reason is
that we don't support concurrent replace operations, at least for
now.
There is an implementation detail in ScyllaCluster.add_servers. We
cannot simply do multiple calls to add_server concurrently. If we
did that in an empty cluster, every node would take itself as the
only seed and start a new cluster. To solve this, we introduce a
new field - initial_seed. It is used to choose one of the servers
as a seed for all servers added concurrently to an empty cluster.
Note that the add_server calls in asyncio.gather in add_servers
cannot race with each other when setting initial_seed because
there is only one thread.
In the future, we will also start all initial servers concurrently
in ScyllaCluster.install_and_start. The changes in this commit were
designed in a way that will make changing install_and_start easy.
before this change, in sstable_run_based_compaction_test, we check
every 4 sstables, to verify that we close the sstable to be replaced
in a batch of 4.
since the integer-based generation identifier is monotonically
incremental, we can assume that the identifiers of sstables are like
0, 1, 2, 3, .... so if the compaction consumes sstable in a
batch of 4, the identifier of the first one in the batch should
always be the multiple of 4. unfortunately, this test does not work
if we use uuid-based identifier.
but if we take a closer look at how we create the dataset, we can
have following facts:
1. the `compaction_descriptor` returned by
`sstable_run_based_compaction_strategy_for_tests` never
set `owned_ranges` in the returned descriptor
2. in `compaction::setup_sstable_reader`, `mutation_reader::forward::no`
is used, if `_owned_ranges_checker` is empty
3. `mutation_reader_merger` respects the `fwd_mr` passed to its
ctor, so it closes current sstable immediately when the underlying
mutation reader reaches the end of stream.
in other words, we close every sstable once it is fully consumed in
sstable_ompaction_test. and the reason why the existing test passes
is that we just sample the sstables whose generation id is a multiple
of 4. what happens when we perform compaction in this test is:
1. replace 5 with 33, closing 5
2. replace 6 with 34, closing 6
3. replace 7 with 35, closing 7
4. replace 8 with 36, closing 8 << let's check here.. good, go on!
5. replace 13 with 37, closing 13
...
8. replace 16 with 40, closing 16 << let's check here.. also, good, go on!
so, in this change, we just check all old sstables, to verify that
we close each of them once it is fully consumed.
Fixes https://github.com/scylladb/scylladb/issues/16073Closesscylladb/scylladb#16074
* github.com:scylladb/scylladb:
test/sstable_compaction_test: check every sstable replaced sstable
test/sstable_compaction_test: s/old_sstables.front()/old_sstable/
Go through the rollback_to_normal state when the node needs to move to
normal during the rollback and update fence in this state before moving
the node to normal. This guaranties that the fence update will not
be missed. Not that when a node moves to left state it already passes
through left_token_ring which guaranties the same.
The replace operation is defined to succeed only if the node being
replaced is dead. We should reject this operation when the failure
detector considers the node being replaced alive.
Apart from adding this change, this PR adds a test case -
`test_replacing_alive_node_fails` - that verifies it. A few testing
framework adjustments were necessary to implement this test and
to avoid flakiness in other tests that use the replace operation after
the change. From now, we need to ensure that all nodes see the
node being replaced as dead before starting the replace. Otherwise,
the check added in this PR could reject the replace.
Additionally, this PR changes the replace procedure in a way that
if the replacing node reuses the IP of the node being replaced, other
nodes can see it as alive only after the topology coordinator accepts
its join request. The replacing node may become alive before the
topology coordinator checks if the node being replaced is dead. If
that happens and the replacing node reuses the IP of the node being
replaced, the topology coordinator cannot know which of these two
nodes is alive and whether it should reject the join request.
Fixes#15863Closesscylladb/scylladb#15926
* github.com:scylladb/scylladb:
test: add test_replacing_alive_node_fails
raft topology: reject replace if the node being replaced is not dead
raft topology: add the gossiper ref to topology_coordinator
test: test_cluster_features: stop gracefully before replace
test: decrease failure_detector_timeout_in_ms in replace tests
test: move test_replace to topology_custom
test: server_add: wait until the node being replaced is dead
test: server_add: add support for expected errors
raft topology: join: delay advertising replacing node if it reuses IP
raft topology: join: fix a condition in validate_joining_node
the operator<<() based formatter is only used in its test, so
let's move it to where it is used.
we can always bring it back later if it is required in other places.
but better off implementing it as a fmt::formatter<> then.
Refs #13245
Signed-off-by: Kefu Chai <kefu.chai@scylladb.com>
Closesscylladb/scylladb#16142
