To apply topology_change commands group0_state_machine needs to have an
access to the storage service to support topology changes over raft.
Message-Id: <20230316112801.1004602-10-gleb@scylladb.com>
This series cleans up unit test in preparation for PR #12994.
Helpers are added (or reused) to not rely on specific sstable generation numbers where possible (other than loading reference sstables that are committed to the repo with given generation numbers), and to generate the sstables for tests easily, taking advantage of generation management in `sstable_test_env`, `table_for_tests`, or `replica::table` itself.
Closes#13242
* github.com:scylladb/scylladb:
test: add verify_mutation helpers.
test: add make_sstable_containing memtable
test: table_for_tests: add make_sstable function
test: sstable_test_env: add make_sst_factory methods
test: sstable_compaction_test: do not rely on specific generations
tests: use make_sstable defaults as much as possible
test: sstable_test_env: add make_table_for_tests
test: sstable_datafile_test: do not rely on sepecific sstable generations
test: sstable_test_env: add reusable_sst(shared_sstable)
sstable: expose get_storage function
test: mutation_reader_test: create_sstable: do not rely on specific generations
test: mutation_reader_test: do_test_clustering_order_merger_sstable_set: rely on test_envsstable generation
test: mutation_reader_test: combined_mutation_reader_test: define a local sst_factory function
test: mutation_reader_test: do not use tmpdir
test: use big format by default
test: sstable_compaction_test: use highest sstable version by default
test: test_env: make_db_config: set cfg host_id
test: sstable_datafile_test: fixup indentation
test: sstable_datafile_test: various tests: do_with_async
test: sstable_3_x_test: validate_read, sstable_assertions: get shared_sstable
test: sstable_3_x_test: compare_sstables: get shared_sstable
test: sstable_3_x_test: write_sstables: return shared_sstable
test: sstable_3_x_test: write, compare, validate_sstables: use env.tempdir
test: sstable_3_x_test: compacted_sstable_reader: do not reopen compacted_sst
test: lib: test_services: delete now unused stop_and_keep_alive
test: sstable_compaction_test: use deferred_stop to stop table_for_tests
test: sstable_compaction_test: compound_sstable_set_incremental_selector_test: do_with_async
test: sstable_compaction_test: sstable_needs_cleanup_test: do_with_async
test: sstable_compaction_test: leveled_05: fixup indentation
test: sstable_compaction_test: leveled_05: do_with_async
test: sstable_compaction_test: compact_02: do_with_async
test: sstable_compaction_test: compact_sstables: simplify variable allocation
test: sstable_compaction_test: compact_sstables: reindent
test: sstable_compaction_test: compact_sstables: use thread
test: sstable_compaction_test: sstable_rewrite: simplify variable allocation
test: sstable_compaction_test: sstable_rewrite: fixup indentation
test: sstable_compaction_test: sstable_rewrite: do_with_async
test: sstable_compaction_test: compact: fixup indentation
test: sstable_compaction_test: compact: complete conversion to async thread
test: sstable_compaction_test: compaction_manager_basic_test: rename generations to idx
Some unit tests want to change the sstable::_dir on the fly. However,
the sstable::_dir is going away, so it needs a yet another virtual call
on storage driver.
Signed-off-by: Pavel Emelyanov <xemul@scylladb.com>
Closes#13213
table_for_tests uses a sstables manager to generate sstables
and gets the new generation from
table.calculate_generation_for_new_table().
The version to use is either the highest supported or
an ad-hoc version passed to make_sstable.
Signed-off-by: Benny Halevy <bhalevy@scylladb.com>
The tests extensively use a `std::function<shared_sstable()>`
to generate new tables.
Rather than handcrafting them all over the place,
let sstable_test_env return such factory given a schema
(and another entry point that also gets a version)
and that uses the embedded generation_factory in the test_env
to generate new sstables with unique generations.
Signed-off-by: Benny Halevy <bhalevy@scylladb.com>
No need to maintain a static generation numbers in the test.
Let the sstable_test_env dispatch sstable generations automatically
And use the generated sstable themselves for reference rather
than their generation numbers.
Signed-off-by: Benny Halevy <bhalevy@scylladb.com>
Signed-off-by: Benny Halevy <bhalevy@scylladb.com>
Add a few goodies to sstable_test_env to extend
entry points with default params for make_sstable
and reusable_sst.
Signed-off-by: Benny Halevy <bhalevy@scylladb.com>
Wrap table_for_tests ctor to pass the env sstables_manager
as well as the temporary directory path, as this is the
most common use case, and in preparation for adding
a make_sstable method in table_for_tests.
Signed-off-by: Benny Halevy <bhalevy@scylladb.com>
Allow generating a sstable object from an existing
sstable to get the directory, generation, and version
from it, rather than passing them to reusable_sst
from other sources - since the intention is
to get a new sstable object based on an existing
sstable that was generated by the test.
Signed-off-by: Benny Halevy <bhalevy@scylladb.com>
Tests should just generate the highest sstable version
available. There is no need to ontinue testing old versions,
in particular partially supported ones like "la".
Use also the default values for sstable::format_types, buffer_size,
etc. if there's no particular need to override them.
Signed-off-by: Benny Halevy <bhalevy@scylladb.com>
The `storage_options` describes where sstables should be located. Currently the object reside on keyspace_metadata, but is thus not available at the place it's needed the most -- the `table::make_sstable()` call. This set converts keyspace_metadata::storage_opts to be lw-shared-ptr and shares the ptr with class table.
refs: #12523 (detached small change from large PR)
Closes#13212
* github.com:scylladb/scylladb:
table: Keep storage options lw-shared-ptr
keyspace_metadata: Make storage options lw-shared-ptr
this series intends to deprecate `::join()`, as it always materializes a range into a concrete string. but what we always want is to print the elements in the given range to stream, or to a seastar logger, which is backed by fmtlib. also, because fmtlib offers exactly the same set of features implemented by to_string.hh, this change would allow us to use fmtlib to replace to_string.hh for better maintainability, and potentially better performance. as fmtlib is lazy evaluated, and claims to be performant under most circumstances.
Closes#13163
* github.com:scylladb/scylladb:
utils: to_string: move join to namespace utils
treewide: use fmt::join() when appropriate
row_cache: pass "const cache_entry" to operator<<
Tables need to know which storage their sstables need to be located at,
so class table needs to have itw reference of the storage options. The
thing can be inherited from the keyspace metadata.
Tests sometimes create table without keyspace at hand. For those use
default-initialized storage options (which is local storage).
Signed-off-by: Pavel Emelyanov <xemul@scylladb.com>
now that fmtlib provides fmt::join(). see
https://fmt.dev/latest/api.html#_CPPv4I0EN3fmt4joinE9join_viewIN6detail10iterator_tI5RangeEEN6detail10sentinel_tI5RangeEEERR5Range11string_view
there is not need to revent the wheel. so in this change, the homebrew
join() is replaced with fmt::join().
as fmt::join() returns an join_view(), this could improve the
performance under certain circumstances where the fully materialized
string is not needed.
please note, the goal of this change is to use fmt::join(), and this
change does not intend to improve the performance of existing
implementation based on "operator<<" unless the new implementation is
much more complicated. we will address the unnecessarily materialized
strings in a follow-up commit.
some noteworthy things related to this change:
* unlike the existing `join()`, `fmt::join()` returns a view. so we
have to materialize the view if what we expect is a `sstring`
* `fmt::format()` does not accept a view, so we cannot pass the
return value of `fmt::join()` to `fmt::format()`
* fmtlib does not format a typed pointer, i.e., it does not format,
for instance, a `const std::string*`. but operator<<() always print
a typed pointer. so if we want to format a typed pointer, we either
need to cast the pointer to `void*` or use `fmt::ptr()`.
* fmtlib is not able to pick up the overload of
`operator<<(std::ostream& os, const column_definition* cd)`, so we
have to use a wrapper class of `maybe_column_definition` for printing
a pointer to `column_definition`. since the overload is only used
by the two overloads of
`statement_restrictions::add_single_column_parition_key_restriction()`,
the operator<< for `const column_definition*` is dropped.
Signed-off-by: Kefu Chai <kefu.chai@scylladb.com>
this is the 12nd changeset of a series which tries to give an overhaul to the CMake building system. this series has two goals:
- to enable developer to use CMake for building scylla. so they can use tools (CLion for instance) with CMake integration for better developer experience
- to enable us to tweak the dependencies in a simpler way. a well-defined cross module / subsystem dependency is a prerequisite for building this project with the C++20 modules.
this changeset includes following changes:
- build: cmake: remove Seastar from the option name
- build: cmake: add missing sources in test-lib and utils
- build: cmake: do not include main.cc in scylla-main
- build: cmake: define SEASTAR_TESTING_MAIN for SEASTAR tests
- build: cmake: add more tests
Closes#13180
* github.com:scylladb/scylladb:
build: cmake: add more tests
build: cmake: define SEASTAR_TESTING_MAIN for SEASTAR tests
build: cmake: do not include main.cc in scylla-main
build: cmake: add missing sources in test-lib and utils
build: cmake: remove Seastar from the option name
Simplified, more direct version of "dependency injection".
I.e. caller/initiator (main/cql_test_env) provides a set of
services it will eventually start. Configurable can remember
these. And use, at least after "start" notification.
Closes#13037
- build: cmake: remove test which does not exist yet
- build: cmake: document add_scylla_test()
- build: cmake: extract index, repair and data_dictionary out
- build: cmake: extract scylla-main out
- build: cmake: find Snappy before using it
- build: cmake: add missing linkages
- build: cmake: add missing sources to test-lib
- build: cmake: link sstables against libdeflate
- build: cmake: link Boost::regex against ICU::uc
Closes#13110
* github.com:scylladb/scylladb:
build: cmake: link Boost::regex against ICU::uc
build: cmake: link sstables against libdeflate
build: cmake: add missing sources to test-lib
build: cmake: add missing linkages
build: cmake: find Snappy before using it
build: cmake: extract scylla-main out
build: cmake: extract index, repair and data_dictionary out
build: cmake: document add_scylla_test()
build: cmake: remove test which does not exist yet
Allows a configurable to subscribe to life cycle notifications for scylla app.
I.e. do stuff on start/stop.
Also allow configurables in cql_test_env
v2:
* Fix camel casing
* Make callbacks future<> (should have been. mismerge?)
Closes#13035
When creating a deletion log for a bunch of sstables the code checks
that all sstables share the same "storage" by lexicographically
comparing their prefixes. That's not correct, as filesystem paths may
refer to the same directory even if not being equal.
So far that's been mostly OK, because paths manipulations were done in
simple forms without producing unequal paths. Patch 8a061bd8 (sstables,
code: Introduce and use change_state() call) triggerred a corner case.
fs::path foo("/foo");
sstring sub("");
foo = foo / sub;
produces a correct path of "/foo/", but the trailing slash breaks the
aforementioned assumption about prefixes comparison. As a result, when
an sstable moves between, say, staging and normal locations it may gain
a trailing slash breaking the deletion log creation code.
The fix is to restrict the deletion log creation not to rely on path
strings comparison completely and trim the trailing slash if it happens.
A test is included.
fixes: #13085
Signed-off-by: Pavel Emelyanov <xemul@scylladb.com>
Closes#13090
The method in question is only called with env's tempdir, so there's no
point in explicitly passing it.
Signed-off-by: Pavel Emelyanov <xemul@scylladb.com>
There's a wonderful comment describing what the reusable_sst is for near
one of its wrappers. It's better to drop the wrapper and move the
comment to where it belongs.
Signed-off-by: Pavel Emelyanov <xemul@scylladb.com>
Lots of test cases make sstables with monotonically incrementing
generation values. In Scylla code this counter is maintained in class
table, but sstable tests not always have it. To mimic this behavior, the
test_env can keep track of the generation, so that callers just don't
mess with it (next patch).
Signed-off-by: Pavel Emelyanov <xemul@scylladb.com>
There's a make_sstable_containing() helper that creates sstable and
populates it with mutations (and makes some post validation). The helper
accepts a factory function that should make sstable for it.
This patch shuffles this helper a bit by introducing an overload that
populates (and validates) the already existing sstable.
Signed-off-by: Pavel Emelyanov <xemul@scylladb.com>
There's the lonely test case that uses the mentioned template to carry
its own instance of tempdir over its lifetime. Patch the case to re-use
the already existing env's tempdir and drop the template.
Signed-off-by: Pavel Emelyanov <xemul@scylladb.com>
Lots of (most of) test cases out there generate sstables inside env's
temporary directory. This patch adds some sugar to env that will allow
test cases omit explicit env.tempdir() call.
Signed-off-by: Pavel Emelyanov <xemul@scylladb.com>
This is a RAII-sh helper that cleans temp directory on destruction. To
be used in cases when a test needs to do several checks over clean
temporary directory (future patches).
Signed-off-by: Pavel Emelyanov <xemul@scylladb.com>
The former helper is going to get rid of the fs::path& dir argument,
but the latter cannot yet live without it. The simplest solution is to
open-code the helper until better times.
Signed-off-by: Pavel Emelyanov <xemul@scylladb.com>
these dependencies were found when trying to compile
`user_function_test`. whenever a library libfoo references another one,
say, libbar, the corresponding linkage from libfoo to libbar is added.
Signed-off-by: Kefu Chai <kefu.chai@scylladb.com>
The code for compare_endpoints originates at the dawn of time (bc034aeaec)
and is called on the fast path from storage_proxy via `sort_by_proximity`.
This series considerably reduces the function's footprint by:
1. carefully coding the many comparisons in the function so to reduce the number of conditional banches (apparently the compiler isn't doing a good enough job at optimizing it in this case)
2. avoid sstring copy in topology::get_{datacenter,rack}
Closes#12761
* github.com:scylladb/scylladb:
topology: optimize compare_endpoints
to_string: add print operators for std::{weak,partial}_ordering
utils: to_sstring: deinline std::strong_ordering print operator
move to_string.hh to utils/
test: network_topology: add test_topology_compare_endpoints
There are two places that do it -- commitlog and batchlog replayers. Both can have local system-keyspace reference and use system-keyspace local query-processor for it. The peering save_truncation_record() is not that simple and is not patched by this PR
Closes#13087
* github.com:scylladb/scylladb:
system_keyspace: Unstatic get_truncation_record()
system_keyspace: Unstatic get_truncated_at()
batchlog_manager: Add system_keyspace dependency
main: Swap batchlog manager and system keyspace starts
system_keyspace: Unstatic get_truncated_position()
system_keyspace: Remove unused method
commitlog: Create commitlog_replayer with system keyspace
test: Make cql_test_env::get_system_keyspace() return sharded
commiltlog: Line-up field definitions
The manager will need system ks to get truncation record from, so add it
explicitly. Start-stop sequence no allows that
Signed-off-by: Pavel Emelyanov <xemul@scylladb.com>
the type of return value of `get_table_views()` is a reference, so we
cannot return a reference to a temporary value.
in this change, a member variable is added to hold the _table_schema,
so it can outlive the function call.
this should silence following warning from Clang:
```
test/lib/expr_test_utils.cc:543:16: error: returning reference to local temporary object [-Werror,-Wreturn-stack-address]
return {view_ptr(_table_schema)};
^~~~~~~~~~~~~~~~~~~~~~~~~
```
Signed-off-by: Kefu Chai <kefu.chai@scylladb.com>
- build: cmake: extract more subsystem out into its own CMakeLists.txt
- build: cmake: remove swagger_gen_files
- build: cmake: remove stale TODO comments
- build: cmake: expose scylla_gen_build_dir
- build: cmake: link against cryptopp
- build: cmake: add missing source to utils
- build: cmake: move lib sources into test-lib
- build: cmake: add test/perf
Closes#13059
* github.com:scylladb/scylladb:
build: cmake: add expr_test test
build: cmake: allow test to specify the sources
build: cmake: add test/perf
build: cmake: move lib sources into test-lib
build: cmake: add missing source to utils
build: cmake: link against cryptopp
build: cmake: expose scylla_gen_build_dir
build: cmake: remove stale TODO comments
build: cmake: remove swagger_gen_files
build: cmake: extract more subsystem out into its own CMakeLists.txt
This method requires callers to remember that the sstable is the collection of files on a filesystem and to know what exact directory they are all in. That's not going to work for object storage, instead, sstable should be moved between more abstract states.
This PR replaces move_to_new_dir() call with the change_state() one that accepts target sub-directory string and moves files around. Currently supported state changes:
* staging -> normal
* upload -> normal | staging
* any -> quarantine
All are pretty straightforward and move files between table basedir subdirectories with the exception that upload -> quarantine should move into upload/quarantine subdirectory. Another thing to keep in mind, that normal state doesn't have its subdir but maps directory to table's base directory.
Closes#12648
* github.com:scylladb/scylladb:
sstable: Remove explicit quarantization call
test: Move move_to_new_dir() method from sstable class
sstable, dist.-loader: Introduce and use pick_up_from_upload() method
sstables, code: Introduce and use change_state() call
distributed_loader: Let make_sstables_available choose target directory
these warnings are found by Clang-17 after removing
`-Wno-unused-lambda-capture` and '-Wno-unused-variable' from
the list of disabled warnings in `configure.py`.
Signed-off-by: Kefu Chai <kefu.chai@scylladb.com>
It's known that reading large cells in reverse cause large allocations.
Source: https://github.com/scylladb/scylladb/issues/11642
The loading is preliminary work for splitting large partitions into
fragments composing a run and then be able to later read such a run
in an efficiency way using the position metadata.
The splitting is not turned on yet, anywhere. Therefore, we can
temporarily disable the loading, as a way to avoid regressions in
stable versions. Large allocations can cause stalls due to foreground
memory eviction kicking in.
The default values for position metadata say that first and last
position include all clustering rows, but they aren't used anywhere
other than by sstable_run to determine if a run is disjoint at
clustering level, but given that no splitting is done yet, it
does not really matter.
Unit tests relying on position metadata were adjusted to enable
the loading, such that they can still pass.
Fixes#11642.
Signed-off-by: Raphael S. Carvalho <raphaelsc@scylladb.com>
Closes#12979
The initial intent was to reduce the fanout of shared_sstable.hh through
v.u.g.hh -> cql_test_env.hh chain, but it also resulted in some shots
around v.u.g.hh -> database.hh inclusion.
By and large:
- v.u.g.hh doesn't need database.hh
- cql_test_env.hh doesn't need v.u.g.hh (and thus -- the
shared_sstable.hh) but needs database.hh instead
- few other .cc files need v.u.g.hh directly as they pulled it via
cql_test_env.hh before
- add forward declarations in few other places
Signed-off-by: Pavel Emelyanov <xemul@scylladb.com>
Closes#12952
There's a bunch of test cases that check how moving sstables files
around the filesystem works. These need the generic move_to_new_dir()
method from sstable, so move it there.
Signed-off-by: Pavel Emelyanov <xemul@scylladb.com>
this change is based on Botond Dénes's change which gave an overhaul
to the original CMake building system. this change is not enough
to build tests with CMake, as we still need to sort out the
dependencies.
Signed-off-by: Kefu Chai <kefu.chai@scylladb.com>
The former is a convenience wrapper over the latter. There's no real benefit in using it, but having two test_env-s is worse than just one.
Closes#12794
* github.com:scylladb/scylladb:
sstable_utils: Move the test_setup to perf/
sstable_utils: Remove unused wrappers over test_env
sstable_test: Open-code do_with_cloned_tmp_directory
sstable_test: Asynchronize statistics_rewrite case
tests: Replace test_setup::do_with_tmp_directory with test_env::do_with(_async)?
