Number of the repair operation was counted both with
next_repair_command from tracer and sequence number
from task_manager::module.
To get rid of redundancy next_repair_command was deleted and all
methods using its value were moved to repair_module.
Execution shard is one of the traits specific to repair tasks.
Child task should freely access shard id of its parent. Thus,
the shard number is kept in a repair_uniq_id struct.
Create repair_task_impl and repair_module inheriting from respectively
task manager task_impl and module to integrate repair operations with
task manager.
We currently avoid compiling C code in "pip3 install scylla-driver", but
we actually providing portable binary distributions of the package,
so we should use it by "pip3 install --only-binary=:all: scylla-driver".
The binary distribution contains dependency libraries, so we won't have
problem loading it on relocatable python3.
Closes#11852
The PR adds changes to task manager that allow more convenient integration with modules.
Introduced changes:
- adds internal flag in task::impl that allows user to filter too specific tasks
- renames `parent_data` to more appropriate name `task_info`
- creates `tasks/types.hh` which allows using some types connected with task manager without the necessity to include whole task manager
- adds more flexible version of `make_task` method
Closes#11821
* github.com:scylladb/scylladb:
tasks: add alternative make_task method
tasks: rename parent_data to task_info and move it
tasks: move task_id to tasks/types.hh
tasks: add internal flag for task_manager::task::impl
Prevent copying shared_ptr across shards
in do_sync_data_using_repair by allocating
a shared_ptr<abort_source> per shard in
node_ops_meta_data and respectively in node_ops_info.
Fixes#11826
Signed-off-by: Benny Halevy <bhalevy@scylladb.com>
Closes#11827
* github.com:scylladb/scylladb:
repair: use sharded abort_source to abort repair_info
repair: node_ops_info: add start and stop methods
storage_service: node_ops_abort_thread: abort all node ops on shutdown
storage_service: node_ops_abort_thread: co_return only after printing log message
storage_service: node_ops_meta_data: add start and stop methods
repair: node_ops_info: prevent accidental copy
The lsa-segment command tries to walk LSA segment objects by decoding
their descriptors and (!) object sizes as well. Some objects in LSA have
dynamic sizes, i.e. those depending on the object contents. The script
tries to drill down the object internals to get this size, but bad news
is that nowadays there are many dynamic objects that are not covered.
Once stepped upon unsupported object, scylla-gdb likely stops because
the "next" descriptor happens to be in the middle of the object and its
parsing throws.
This patch fixes this by taking advantage of the virtual size() call of
the migrate_fn_type all LSA objects are linked with (indirectly). It
gets the migrator object, the LSA object itself and calls
((migrate_fn_type*)<migrator_ptr>)->size((const void*)<object_ptr>)
with gdb. The evaluated value is the live dynamic size of the object.
fixes: #11792
refs: #2455
Signed-off-by: Pavel Emelyanov <xemul@scylladb.com>
Closes#11847
Currently, when specifying nodes to ignore for replace or removenode,
we support specifying them only using their ip address.
As discussed in https://github.com/scylladb/scylladb/issues/11839 for removenode,
we intentionally require the host uuid for specifying the node to remove,
so the nodes to ignore (that are also done, otherwise we need not ignore them),
should be consistent with that and be specified using their host_id.
The series extends the apis and allows either the nodes ip address or their host_id
to be specified, for backward compatibility.
We should deprecate the ip address method over time and convert the tests and management
software to use the ignored nodes' host_id:s instead.
Closes#11841
* github.com:scylladb/scylladb:
api: doc: remove_node: improve summary
api, service: storage_service: removenode: allow passing ignore_nodes as uuid:s
storage_service: get_ignore_dead_nodes_for_replace: use tm.parse_host_id_and_endpoint
locator: token_metadata: add parse_host_id_and_endpoint
api: storage_service: remove_node: validate host_id
The current summary of the operation is obscure.
It refers to a token in the ring and the endpoint associated with it,
while the operation uses a host_id to identify a whole node.
Instead, clarify the summary to refer to a node in the cluster,
consistent with the description for the host_id parameter.
Also, describe the effect the call has on the data the removed node
logically owned.
Signed-off-by: Benny Halevy <bhalevy@scylladb.com>
Currently the api is inconsistent: requiring a uuid for the
host_id of the node to be removed, while the ignored nodes list
is given as comma-separated ip addresses.
Instead, support identifying the ignored_nodes either
by their host_id (uuid) or ip address.
Also, require all ignore_nodes to be of the same kind:
either UUIDs or ip addresses, as a mix of the 2 is likely
indicating a user error.
Signed-off-by: Benny Halevy <bhalevy@scylladb.com>
To be used for specifying nodes either by their
host_id or ip address and using the token_metadata
to resolve the mapping.
Signed-off-by: Benny Halevy <bhalevy@scylladb.com>
The node to be removed must be identified by its host_id.
Validate that at the api layer and pass the parsed host_id
down to storage_service::removenode.
Signed-off-by: Benny Halevy <bhalevy@scylladb.com>
Currently, --disks options does not allow symlinks such as
/dev/disk/by-uuid/* or /dev/disk/azure/*.
To allow using them, is_unused_disk() should resolve symlink to
realpath, before evaluating the disk path.
Fixes#11634Closes#11646
It seems like distribution original sysconfig files does not use double
quote to set the parameter when the value does not contain space.
Adding function to detect spaces in the value, don't usedouble quote
when it not detected.
Fixes#9149Closes#9153
* github.com:scylladb/scylladb:
scylla_util.py: adding unescape for sysconfig_parser
scylla_util.py: on sysconfig_parser, don't use double quote when it's possible
Currently we use a single shared_ptr<abort_source>
that can't be copied across shards.
Instead, use a sharded<abort_source> in node_ops_info so that each
repair_info instance will use an (optional) abort_source*
on its own shard.
Added respective start and stop methodsm plus a local_abort_source
getter to get the shard-local abort_source (if available).
Fixes#11826
Signed-off-by: Benny Halevy <bhalevy@scylladb.com>
Prepare for adding a sharded<abort_source> member.
Wire start/stop in storage_service::node_ops_meta_data.
Signed-off-by: Benny Halevy <bhalevy@scylladb.com>
A later patch adds a sharded<abort_source> to node_ops_info.
On shutdown, we must orderly stop it, so use node_ops_abort_thread
shutdown path (where node_ops_singal_abort is called will a nullopt)
to abort (and stop) all outstanding node_ops by passing
a null_uuid to node_ops_abort, and let it iterate over all
node ops to abort and stop them.
Signed-off-by: Benny Halevy <bhalevy@scylladb.com>
Currently the function co_returns if (!uuid_opt)
so the log info message indicating it's stopped
is not printed.
Signed-off-by: Benny Halevy <bhalevy@scylladb.com>
Delete node_ops_info copy and move constructors before
we add a sharded<abort_source> member for the per-shard repairs
in the next patch.
Signed-off-by: Benny Halevy <bhalevy@scylladb.com>
Even we have __escape() for escaping " middle of the value to writing
sysconfig file, we didn't unescape for reading from sysconfig file.
So adding __unescape() and call it on get().
It seems like distribution original sysconfig files does not use double
quote to set the parameter when the value does not contain space.
Adding function to detect spaces in the value, don't usedouble quote
when it not detected.
Fixes#9149
Task manager tasks should be created with make_task method since
it properly sets information about child-parent relationship
between tasks. Though, sometimes we may want to keep additional
task data in classes inheriting from task_manager::task::impl.
Doing it with existing make_task method makes it impossible since
implementation objects are created internally.
The commit adds a new make_task that allows to provide a task
implementation pointer created by caller. All the fields except
for the one connected with children and parent should be set before.
parent_data struct contains info that is common for each task,
not only in parent-child relationship context. To use it this way
without confusion, its name is changed to task_info.
In order to be able to widely and comfortably use task_info,
it is moved from tasks/task_manager.hh to tasks/types.hh
and slightly extended.
It is convenient to create many different tasks implementations
representing more and more specific parts of the operation in
a module. Presenting all of them through the api makes it cumbersome
for user to navigate and track, though.
Flag internal is added to task_manager::task::impl so that the tasks
could be filtered before they are sent to user.
When doing shadow round for replacement the bootstrapping node needs to
know the dc/rack info about the node it replaces to configure it on
topology. This topology info is later used by e.g. repair service.
fixes: #11829
Signed-off-by: Pavel Emelyanov <xemul@scylladb.com>
Closes#11838
After calling filter_for_query() the extra_replica to speculate to may
be left default-initialized which is :0 ipv6 address. Later below this
address is used as-is to check if it belongs to the same DC or not which
is not nice, as :0 is not an address of any existing endpoint.
Recent move of dc/rack data onto topology made this place reveal itself
by emitting the internal error due to :0 not being present on the
topology's collection of endpoints. Prior to this move the dc filter
would count :0 as belonging to "default_dc" datacenter which may or may
not match with the dc of the local node.
The fix is to explicitly tell set extra_replica from unset one.
fixes: #11825
Signed-off-by: Pavel Emelyanov <xemul@scylladb.com>
Closes#11833
On most of the software distribution tar.gz, it has sub-directory to contain
everything, to prevent extract contents to current directory.
We should follow this style on our unified package too.
To do this we need to increment relocatable package version to '3.0'.
Fixes#8349Closes#8867
We added UUID device file existance check on #11399, we expect UUID
device file is created before checking, and we wait for the creation by
"udevadm settle" after "mkfs.xfs".
However, we actually getting error which says UUID device file missing,
it probably means "udevadm settle" doesn't guarantee the device file created,
on some condition.
To avoid the error, use var-lib-scylla.mount to wait for UUID device
file is ready, and run the file existance check when the service is
failed.
Fixes#11617Closes#11666
Since the end bound is exclusive, the end position should be
before_key(), not after_key().
Affects only tests, as far as I know, only there we can get an end
bound which is a clustering row position.
Would cause failures once row cache is switched to v2 representation
because of violated assumptions about positions.
Introduced in 76ee3f029cCloses#11823
We should scan all sstables in the table directory and its
subdirectories to determine the highest sstable version and generation
before using it for creating new sstables (via reshard or reshape).
Otherwise, the generations of new sstables created when populating staging (via reshard or reshape) may collide with generations in the base directory, leading to https://github.com/scylladb/scylladb/issues/11789
Refs scylladb/scylladb#11789
Fixes scylladb/scylladb#11793
Closes#11795
* github.com:scylladb/scylladb:
distributed_loader: populate_column_family: reindent
distributed_loader: coroutinize populate_column_family
distributed_loader: table_population_metadata: start: reindent
distributed_loader: table_population_metadata: coroutinize start_subdir
distributed_loader: table_population_metadata: start_subdir: reindent
distributed_loader: pre-load all sstables metadata for table before populating it
As described in issue #11801, we saw in Alternator when a GSI has both partition and sort keys which were non-key attributes in the base, cases where updating the GSI-sort-key attribute to the same value it already had caused the entire GSI row to be deleted.
In this series fix this bug (it was a bug in our materialized views implementation) and add a reproducing test (plus a few more tests for similar situations which worked before the patch, and continue to work after it).
Fixes#11801Closes#11808
* github.com:scylladb/scylladb:
test/alternator: add test for issue 11801
MV: fix handling of view update which reassign the same key value
materialized views: inline used-once and confusing function, replace_entry()
The storage_service::stop() calls repair_service::abort_repair_node_ops() but at that time the sharded<repair_service> is already stopped and call .local() on it just crashes.
The suggested fix is to remove explicit storage_service -> repair_service kick. Instead, the repair_infos generated for the sake of node-ops are subscribed on the node_ops_meta_data's abort source and abort themselves automatically.
fixes: #10284Closes#11797
* github.com:scylladb/scylladb:
repair: Remove ops_uuid
repair: Remove abort_repair_node_ops() altogether
repair: Subscribe on node_ops_info::as abortion
repair: Keep abort source on node_ops_info
repair: Pass node_ops_info arg to do_sync_data_using_repair()
repair: Mark repair_info::abort() noexcept
node_ops: Remove _aborted bit
node_ops: Simplify construction of node_ops_metadata
main: Fix message about repair service starting
The test wants to see that no allocations larger than 128k are present,
but sets the warning threshold to exactly 128k. Due to an off-by-one in
Seastar, this went unnoticed. However, now that the off-by-one in Seastar
is fixed [1], this test starts to fail.
Fix by setting the warning threshold to 128k + 1.
[1] 429efb5086Closes#11817
The vector(initializer_list<T>) constructor copies the T since
initializer_list is read-only. Move the mutation instead.
This happens to fix a use-after-return on clang 15 on aarch64.
I'm fairly sure that's a miscompile, but the fix is worthwhile
regardless.
Closes#11818
This PR adds some unit tests for the `expr::evaluate()` function.
At first I wanted to add the unit tests as part of #11658, but their size grew and grew, until I decided that they deserve their own pull request.
I found a few places where I think it would be better to behave in a different way, but nothing serious.
Closes#11815
* github.com:scylladb/scylladb:
test/boost: move expr_test_utils.hh to .hh and .cc in test/lib
cql3: expr: Add unit tests for bind_variable validation of collections
cql3: expr: Add test for subscripted list and map
cql3: expr: Add test for usertype_constructor
cql3: expr: Add test for tuple_constructor
cql3: expr: Add tests for evaluation of collection constructors
cql3: expr: Add tests for evaluation of column_values and bind_variables
cql3: expr: Add constant evaluation tests
test/boost: Add expr_test_utils.hh
cql3: Add ostream operator for raw_value
cql3: add is_empty_value() to raw_value and raw_value_view
expr_test_utils.hh was a header file with helper methods for
expression tests. All functions were inline, because I didn't
know how to create and link a .cc file in test/boost.
Now the header is split into expr_test_utils.hh and expr_test_utils.cc
and moved to test/lib, which is designed to keep this kind of files.
Signed-off-by: Jan Ciolek <jan.ciolek@scylladb.com>
"
Snitch was the junction of several services' deps because it was the
holder of endpoint->dc/rack mappings. Now this information is all on
topology object, so snitch can be finally made main-local
"
* 'br-deglobalize-snitch' of https://github.com/xemul/scylla:
code: Deglobalize snitch
tests: Get local reference on global snitch instance once
gossiper: Pass current snitch name into checker
snitch: Add sharded<snitch_ptr> arg to reset_snitch()
api: Move update_snitch endpoint
api: Use local snitch reference
api: Unset snitch endpoints on stop
storage_service: Keep local snitch reference
system_keyspace: Don't use global snitch instance
snitch: Add const snitch_ptr::operator->()
These cannot be meaningfully define for a vector value like resources.
To prevent instinctive misuse, remove them. Operator bool is replaced
with `non_zero()` which hopefully better expresses what to expected.
The comparison operator is just removed and inlined into its own user,
which actually help said user's readability.
Closes#11813
