Because they are indeed unused -- they are initialized, passed down
through some layers, but not actually used. No idea why only Clang 12
in debug mode in Nix devenv complains about it, though.
Specifically libabsl_strings{,_internal}.a.
This fixes failure to link tests in the Nix devenv; since presumably
all is good in other setups, it must be something weird having to do
with inlining?
The extra linked libraries shouldn't hurt in any case.
`ranges_to_stream` is a map of ` std::unordered_multimap<dht::token_range, inet_address>` per keyspace.
On large clusters with a large number of keyspace, copying it may cause reactor stalls as seen in #12332
This series eliminates this copy by using std::move and also
turns `stream_ranges` into a coroutine, adding maybe_yield calls to avoid further stalls down the road.
Fixes#12332Closes#12343
* github.com:scylladb/scylladb:
storage_service: stream_ranges: unshare streamer
storage_service: stream_ranges: maybe_yield
storage_service: coroutinize stream_ranges
storage_service: unbootstrap: move ranges_to_stream_by_keyspace to stream_ranges
Now that stream_ranges is a coroutine
streamer can be an automatic variable on the
coroutine stack frame.
Signed-off-by: Benny Halevy <bhalevy@scylladb.com>
Avoid a potentially large memory copy causing
a reactor stall with a large number of keyspaces.
Fixes#12332
Signed-off-by: Benny Halevy <bhalevy@scylladb.com>
This is to define the API sstable needs from underlying storage. When implementing object-storage backend it will need to implement those. The API looks like
future<> snapshot(const sstable& sst, sstring dir, absolute_path abs) const;
future<> quarantine(const sstable& sst, delayed_commit_changes* delay);
future<> move(const sstable& sst, sstring new_dir, generation_type generation, delayed_commit_changes* delay);
void open(sstable& sst, const io_priority_class& pc); // runs in async context
future<> wipe(const sstable& sst) noexcept;
future<file> open_component(const sstable& sst, component_type type, open_flags flags, file_open_options options, bool check_integrity);
It doesn't have "list" or alike, because it's not a method of an individual sstable, but rather the one from sstables_manager. It will come as separate PR.
Closes#12217
* github.com:scylladb/scylladb:
sstable, storage: Mark dir/temp_dir private
sstable: Remove get_dir() (well, almost)
sstable: Add quarantine() method to storage
sstable: Use absolute/relative path marking for snapshot()
sstable: Remove temp_... stuff from sstable
sstable: Move open_component() on storage
sstable: Mark rename_new_sstable_component_file() const
sstable: Print filename(type) on open-component error
sstable: Reorganize new_sstable_component_file()
sstable: Mark filename() private
sstable: Introduce index_filename()
tests: Disclosure private filename() calls
sstable: Move wipe_storage() on storage
sstable: Remove temp dir in wipe_storage()
sstable: Move unlink parts into wipe_storage
sstable: Remove get_temp_dir()
sstable: Move write_toc() to storage
sstable: Shuffle open_sstable()
sstable: Move touch_temp_dir() to storage
sstable: Move move() to storage
sstable: Move create_links() to storage
sstable: Move seal_sstable() to storage
sstable: Tossing internals of seal_sstable()
sstable: Move remove_temp_dir() to storage
sstable: Move create_links_common() to storage
sstable: Move check_create_links_replay() to storage
sstable: Remove one of create_links() overloads
sstable: Remove create_links_and_mark_for_removal()
sstable: Indentation fix after prevuous patch
sstable: Coroutinize create_links_common()
sstable: Rename create_links_common()'s "dir" argument
sstable: Make mark_for_removal bool_class
sstable, table: Add sstable::snapshot() and use in table::take_snapshot
sstable: Move _dir and _temp_dir on filesystem_storage
sstable: Use sync_directory() method
test, sstable: Use component_basename in test
sstables: Move read_{digest|checksum} on sstable
Fix a bug in failure handling and log level.
Closes#12336
* github.com:scylladb/scylladb:
test.py: convert param to str
test.py: fix error level for CQL tests
Type of operation is related to a specific implementation
of a task. Then, it should rather be access with a virtual
method in tasks::task_manager::task::impl than be
its attribute.
Closes#12326
* github.com:scylladb/scylladb:
api: delete unused type parameter from task_manager_test api
tasks: repair: api: remove type attribute from task_manager::task::status
tasks: add type() method to task_manager::task::impl
repair: add reason attribute to repair_task
After commit a57724e711, off-strategy no longer races with view
building, therefore deletion code can be simplified and piggyback
on mechanism for deleting all sstables atomically, meaning a crash
midway won't result in some of the files coming back to life,
which leads to unnecessary work on restart.
Signed-off-by: Raphael S. Carvalho <raphaelsc@scylladb.com>
Closes#12245
The format_unidiff() function takes str, not pathlib PosixPath, so
convert it to str.
This prevented diff output of unexpected result to be shown in the log
file.
Signed-off-by: Alejo Sanchez <alejo.sanchez@scylladb.com>
Recently, the pytest script shipped by Fedora started invoking python
with the `-s` flag, which disables python considering user site
packages. This caused problems for our tests which install the cassandra
driver in the user site packages. This was worked around in e5e7780f32
by providing our own pytest interposer launcher script which does not
pass the above mentioned flag to python. Said patch fixed test.py but
not the run.py in cql-pytest. So if the cql-pytest suite is ran via
test.py it works fine, but if it is invoked via the run script, it fails
because it cannot find the cassandra driver. This patch patches run.py
to use our own pytest launcher script, so the suite can be run via the
run script as well.
Since run.py is shared with the alternator pytest suite, this patch also
fixes said test suite too.
Closes#12253
This PR fixes several bugs related to handling of non-full
clustering keys.
One is in trim_clustering_row_ranges_to(), which is broken for non-full keys in reverse
mode. It will trim the range to position_in_partition_view::after_key(full_key) instead of
position_in_partition_view::before_key(key), hence it will include the
key in the resulting range rather than exclude it.
Fixes#12180
after_key() was creating a position which is after all keys prefixed
by a non-full key, rather than a position which is right after that
key.
This will issue will be caught by cql_query_test::test_compact_storage
in debug mode when mutation_partition_v2 merging starts inserting
sentinels at position after_key() on preemption.
It probably already causes problems for such keys as after_key() is used
in various parts in the read path.
Refs #1446Closes#12234
* github.com:scylladb/scylladb:
position_in_partition: Make after_key() work with non-full keys
position_in_partition: Introduce before_key(position_in_partition_view)
db: Fix trim_clustering_row_ranges_to() for non-full keys and reverse order
types: Fix comparison of frozen sets with empty values
Now all storage access via sstable happens with the help of storage
class API so its internals can be finally made private.
Signed-off-by: Pavel Emelyanov <xemul@scylladb.com>
The sstable::get_dir() is now gone, no callers know that sstable lives
in any path on a filesystem. There are only few callers left.
One is several places in code that need sstable datafile, toc and index
paths to print them in logs. The other one is sstable_directory that is
to be patched separately.
For both there's a storage.prefix() method that prepends component name
with where the sstable is "really" located.
Signed-off-by: Pavel Emelyanov <xemul@scylladb.com>
Moving sstable to quarantine has some specific -- if the sstable is in
staging/ directory it's anyway moved into root/quarantine dir, not into
the quarantine subdir of its current location.
Encapsulate this feture in storage class method.
Signed-off-by: Pavel Emelyanov <xemul@scylladb.com>
The snapshotting code uses full paths to files to manipulate snapshotted
sstables. Until this code is patched to use some proper snapshotting API
from sstable/ module, it will continue doing so.
Nowever, to remove the get_dir() method from sstable() the
seal_sstable() needs to put relative "backup" directory to
storage::snapshot() method. This patch adds a temporary bool_class for
this distinguishing.
Signed-off-by: Pavel Emelyanov <xemul@scylladb.com>
There's a bunch of helpers around XFS-specific temp-dir sitting in
publie sstable part. Drop it altogether, no code needs it for real.
Signed-off-by: Pavel Emelyanov <xemul@scylladb.com>
The file path is going to disappear soon, so print the filename() on
error. For now it's the same, but the meaning of the filename()
returning string is changing to become "random label for the log
reader".
Signed-off-by: Pavel Emelyanov <xemul@scylladb.com>
The helper consists of three stages:
1. open a file (probably in a temp dir)
2. decorate it with extentions and checked_file
3. optionally rename a file from temp dir
The latter is done to trigger XFS allocate this file in separate block
group if the file was created in temp dir on step 1.
This patch swaps steps 2 and 3 to keep filesystem-specific opening next
to each other.
Signed-off-by: Pavel Emelyanov <xemul@scylladb.com>
Currently the sstable::filename(Index) is used in several places that
get the filename as a printable or throwable string and don't treat is
as a real location of any file.
For those, add the index_filename() helper symmetrical to toc_filename()
and (in some sense) the get_filename() one.
Signed-off-by: Pavel Emelyanov <xemul@scylladb.com>
The sstable::filename() is going to become private method. Lots of tests
call it, but tests do call a lot of other sstable private methods,
that's OK. Make the sstable::filename() yet another one of that kind in
advance.
Signed-off-by: Pavel Emelyanov <xemul@scylladb.com>
Now when the filesystem cleaning code is sitting in one method, it can
finally be made the storage class one.
Exception-safe allocation of toc_name (spoiler: it's copied anyway one
step later, so it's "not that safe" actually) is moved into storage as
well. The caller is left with toc_filename() call in its exception
handler.
Signed-off-by: Pavel Emelyanov <xemul@scylladb.com>
When unlinking an sstable for whatever reason it's good to check if the
temp dir is handing around. In some cases it's not (compaction), but
keeping the whole wiping code together makes it easier to move it on
storage class in one go.
Signed-off-by: Pavel Emelyanov <xemul@scylladb.com>
This method initiates the sstable creation. Effectively it's the first
step in sstable creation transaction implemented on top of rename()
call. Thus this method is moved onto storage under respective name.
Signed-off-by: Pavel Emelyanov <xemul@scylladb.com>
When an sstable is prepared to be written on disk the .write_toc() is
called on it which created temporary toc file. Prior to this, the writer
code calls generate_toc() to collect components on the sstable.
This patch adds the .open_sstable() API call that does both. This
prepares the write_toc() part to be moved to storage, because it's not
just "write data into TOC file", it's the first step in transaction
implemeted on top of rename()s.
The test need care -- there's rewrite_toc_without_scylla_component()
thing in utils that doesn't want the generate_toc() part to be called.
It's not patched here and continues calling .write_toc().
Signed-off-by: Pavel Emelyanov <xemul@scylladb.com>
The sstable can be "moved" in two cases -- to move from staging or to
move to quarantine. Both operation are sstable API ones, but the
implementation is storage-specific. This patch makes the latter a method
of storage class.
One thing to note is that only quarantine() touched the target directly.
Now also the move_to_new_dir() happenning on load also does it, but
that's harmless.
Signed-off-by: Pavel Emelyanov <xemul@scylladb.com>
This method is currently used in two places: sstable::snapshot() and
sstable::seal_sstable(). The latter additionally touches the target
backup/ subdir.
This patch moves the whole thing on storage and adds touch for all the
cases. For snapshots this might be excessive, but harmless.
Tests get their private-disclosure way to access sstable._storage in
few places to call create_links directly.
Signed-off-by: Pavel Emelyanov <xemul@scylladb.com>
Now the sstable sealing is split into storage part, internal-state part
and the seal-with-backup kick.
This move makes remove_temp_dir() private.
Signed-off-by: Pavel Emelyanov <xemul@scylladb.com>
There are two of them -- one API call and the other one that just
"seals" it. The latter one also changes the _marked_for_deletion bit on
the sstable.
This patch makes the latter method prepared to be moved onto storage,
because sealing means comitting TOC file on disk with the help of rename
system call which is purely storage thing.
Signed-off-by: Pavel Emelyanov <xemul@scylladb.com>
Same as previous patch. This move makes the previously moved
check_create_links_replay() a private method of the storage class.
Signed-off-by: Pavel Emelyanov <xemul@scylladb.com>
It needs to get sstable const reference to get the filename(s) from it.
Other than that it's pure filesystem-accessing method.
Signed-off-by: Pavel Emelyanov <xemul@scylladb.com>
There are two -- one that accepts generation and the other one that does
not. The latter is only called by the former, so no need in keeping both.
Signed-off-by: Pavel Emelyanov <xemul@scylladb.com>
