Commit Graph

2893 Commits

Author SHA1 Message Date
Nadav Har'El
06fdce82aa test/*/run: clearer error message on wrong SCYLLA setting
The test runners cql-pytest/run et al. try to automatically find the
last-compile Scylla executable, but this decision can be overriden by
the SCYLLA environment variable. If the user sets by mistake SCYLLA to
something which is not a valid path of an executable, the result was a
long and obscure Python stack trace.

So after this patch, if SCYLLA points to something which is not an
executable, a clear error is produced immediately, directing the user
to set it this variable to a correct executable

Signed-off-by: Nadav Har'El <nyh@scylladb.com>
Message-Id: <20220323164427.3301828-1-nyh@scylladb.com>
2022-03-23 19:01:17 +02:00
Jadw1
1438be5311 CQL3: Bloom filter efficacy test
Added CQL pytest to check bloom filter efficacy by
inserting `N` rows and reading `M` non-existing keys.

Added `bloom_filter_false_positives` method to Python `nodetool`
module. Method gets fp number by calling Scylla's API.

Fixes: #1055

Closes #10186
2022-03-23 16:51:50 +02:00
Calle Wilund
56c383ba8e test/perf/perf_commitlog: Add a small commitlog throughput test
Based on perf_simple_query, just bashes data into CL using
normal distribution min/max data chunk size, allowing direct
freeing of segments, _but_ delayed by a normal dist as well,
to "simulate" secondary delay in data persistance.

Needs more stuff.

Some baseline measurements on master:

--min-flush-delay-in-ms 10 --max-flush-delay-in-ms 200
--commitlog-use-hard-size-limit true
--commitlog-total-space-in-mb 10000 --min-data-size 160 --max-data-size 1024
--smp1

median 2065648.59 tps (  1.1 allocs/op,   0.0 tasks/op,    1482 insns/op)
median absolute deviation: 48752.44
maximum: 2161987.06
minimum: 1984267.90

--min-data-size 256 --max-data-size 16384

median 269385.25 tps (  2.2 allocs/op,   0.7 tasks/op,    3244 insns/op)
median absolute deviation: 15719.13
maximum: 323574.43
minimum: 228206.28

--min-data-size 4096 --max-data-size 61440

median 67734.22 tps (  6.4 allocs/op,   2.9 tasks/op,    9153 insns/op)
median absolute deviation: 2070.93
maximum: 82833.17
minimum: 61473.57

--min-data-size 61440 --max-data-size 1843200

median 2281.37 tps ( 79.7 allocs/op,  43.5 tasks/op,  202963 insns/op)
median absolute deviation: 128.87
maximum: 3143.84
minimum: 2140.80

--min-data-size 368640 --max-data-size 6144000

median 679.76 tps (225.5 allocs/op, 116.3 tasks/op,  662700 insns/op)
median absolute deviation: 39.30
maximum: 1148.95
minimum: 586.86

Actual throughput obviously meaningless, as it is run on my slow
machine, but IPS might be relevant.
Note that transaction throughput plummets as we increase median data
sizes above ~200k, since we then more or less always end up replacing
buffers in every call.

Closes #10230
2022-03-22 15:18:25 +02:00
Avi Kivity
72c6859c25 Merge "readers: get rid of v1 mutation from fragments" from Botond
"
The only real user is view building, which is converted to v2 and then
the v1 version of the mutation from fragments reader is removed.

Tests: unit(dev, release)
"

* 'v2-only-from-fragments-mutations/v1' of https://github.com/denesb/scylla:
  readers: remove now unused v1 reader from fragments
  test/boost: flat_mutation_reader_test: remove reader from fragments test
  replica/table: migrate generate_and_propagate_view_updates() to v2
  replica/table: migrate populate_views() to v2
  db/view: convert view_update_builder interface to v2
  db/view: migrate view_update_builder to v2
2022-03-22 15:18:25 +02:00
Nadav Har'El
f76f6dbccb secondary index: avoid special characters in default index names
In CQL, table names are limited to so-called word characters (letters,
numbers and underscores), but column names don't have such a limitation.
When we create a secondary index, its default name is constructed from
the column name - so can contain problematic characters. It can include
even the "/" character. The problem is that the index name is then used,
like a table name, to create a directory with that name.

The test included in this patch demonstrates that before this patch, this
can be misused to create subdirectories anywhere in the filesystem, or to
crash Scylla when it fails to create a directory (which it considers an
unrecoverable I/O error).

In this patch we do what Cassandra does - remove all non-word
characters from the indexed column name before constructing the default
index name. In the included test - which can run on both Scylla and
Cassandra - we verify that the constructed index name is the same as
in Cassandra, which is useful to know (e.g., because knowing the index
name is needed to DROP the index).

Also, this patch adds a second line of defense against the security problem
described above: It is now an error to create a schema with a slash or
null (the two characters not allowed in Unix filenames) in the keyspace
or table names. So if the first line of defense (CQL checking the validity
of its commands) fails, we'll have that second line of defense. I verified
that if I revert the default-index-name fix, the second line of defense
kicks in, and the index creation is aborted and cannot create files in
the wrong place to crash Scylla.

Fixes #3403

Signed-off-by: Nadav Har'El <nyh@scylladb.com>
Message-Id: <20220320162543.3091121-1-nyh@scylladb.com>
2022-03-20 18:33:48 +02:00
Tomasz Grabiec
cd5fec8a23 Merge "raft: re-advertise gossiper features when raft feature support changes" from Pavel
Prior to the change, `USES_RAFT_CLUSTER_MANAGEMENT` feature wasn't
properly advertised upon enabling `SUPPORTS_RAFT_CLUSTER_MANAGEMENT`
raft feature.

This small series consists of 3 parts to fix the handling of supported
features for raft:
1. Move subscription for `SUPPORTS_RAFT_CLUSTER_MANAGEMENT` to the
   `raft_group_registry`.
2. Update `system.local#supported_features` directly in the
   `feature_service::support()` method.
3. Re-advertise gossiper state for `SUPPORTED_FEATURES` gossiper
   value in the support callback within `raft_group_registry`.

* manmanson/track_supported_set_recalculation_v7:
  raft: re-advertise gossiper features when raft feature support changes
  raft: move tracking `SUPPORTS_RAFT_CLUSTER_MANAGEMENT` feature to raft
  gms: feature_service: update `system.local#supported_features` when feature support changes
  test: cql_test_env: enable features in a `seastar::thread`
2022-03-18 12:34:17 +01:00
Pavel Solodovnikov
011942dcce raft: move tracking SUPPORTS_RAFT_CLUSTER_MANAGEMENT feature to raft
Move the listener from feature service to the `raft_group_registry`.

Enable support for the `USES_RAFT_CLUSTER_MANAGEMENT`
feature when the former is enabled.

Signed-off-by: Pavel Solodovnikov <pa.solodovnikov@scylladb.com>
2022-03-18 09:54:25 +03:00
Pavel Solodovnikov
724ea7aa38 test: cql_test_env: enable features in a seastar::thread
Each feature can have an associated `when_enabled` callback
registered, which is assumed to run in the thread context,
so wrap the `enable()` call in a seastar thread.

Tests: unit(dev)

Signed-off-by: Pavel Solodovnikov <pa.solodovnikov@scylladb.com>
2022-03-18 09:54:15 +03:00
Avi Kivity
77f330f393 Merge "readers: retire v1 generating reader implementation" from Botond
"
The generating reader is a reader which converts a functor returning
mutation fragments to a mutation reader.
We currently have 2 generating reader implementations: one operating
with a v1 functor and one with a v2 one. This patch-set converts the v1
functor based one to a v2 reader, by adapting the v1 functor to a v2
functor and reusing the v2 reader implementation.
Tests are also added to both variants.

Tests: unit(dev)
"

* 'generating-reader-v2/v1' of https://github.com/denesb/scylla:
  test/boost: mutation_reader_test: add tests for generating reader
  test: export squash_mutations() into lib/mutation_source_test.hh
  readers: add next partition adaptor
  readers: implement generating_reader from v1 generator via adaptor
  readers: upgrade_to_v2(): reimplement in terms of upgrading_consumer
  readers: add upgrading_consumer
  readers: generating_reader: use noncopyable_function<>
  readers: merge generating.hh into generating_v2.hh
  readers/generating.hh: return v2 reader from make_generating_reader()
2022-03-17 12:19:25 +02:00
Botond Dénes
3ea1240fb9 test/boost: flat_mutation_reader_test: remove reader from fragments test 2022-03-17 11:03:16 +02:00
Botond Dénes
c450508954 Merge "Introduce sharded<system_keyspace> instance" from Pavel Emelyanov
"
Making the system-keyspace into a standard sharded instance will
help to fix several dependency knots.

First, the global qctx and local-cache both will be moved onto the
sys-ks, all their users will be patched to depend on system-keyspace.
Now it's not quite so, but we're moving towards this state.

Second, snitch instance now sits in the middle of another dependency
loop. To untie one the preferred ip and dc/rack info should be
moved onto system keyspace altogether (now it's scattered over several
places). The sys-ks thus needs to be a sharded service with some
state.

This set makes system-keyspace sharded instance, equipps it with all
the dependencies it needs and passes it as dependency into storage
service, migration manager and API. This helps eliminating a good
portion of global qctx/cache usage and prepares the ground for snitch
rework.

tests: unit(dev)
       v1: unit(debug), dtest.simple_boot_shutdown(dev)
"

* 'br-sharded-system-keyspace-instance-2' of https://github.com/xemul/scylla: (25 commits)
  system_keyspace: Make load_host_ids non-static
  system_keyspace: Make load_tokens non-static
  system_keyspace: Make remove_endpoint and update_tokens non-static
  system_keyspace: Coroutinize update_tokens
  system_keyspace: Coroutinize remove_endpoint
  system_keyspace: Make update_cached_values non-static
  system_keyspace: Coroutinuze update_peer_info
  system_keyspace: Make update_schema_version non-static
  schema_tables: Add sharded<system_keyspace> argument to update_schema_version_and_announce
  replica: Push sharded<system_keyspace> down to parse_system_tables
  api: Carry sharded<system_keyspace> reference along
  storage_service: Keep sharded<system_keyspace> reference
  migration_manager: Keep sharded<system_keyspace> reference
  system_keyspace: Remove temporary qp variable
  system_keyspace: Make get_preferred_ips non-static
  system_keyspace: Make cache_truncation_record non-static
  system_keyspace: Make check_health non-static
  system_keyspace: Make build_bootstrap_info non-static
  system_keyspace: Make build_dc_rack_info non-static
  system_keyspace: Make setup_version non-static
  ...
2022-03-17 08:16:29 +02:00
Botond Dénes
9f95042c2b test/boost: mutation_reader_test: add tests for generating reader 2022-03-17 08:08:01 +02:00
Botond Dénes
4243cd395d test: export squash_mutations() into lib/mutation_source_test.hh
This method used to be a static one in
boost/flat_mutation_reader_test.cc. Turns out it is useful for other
tests based on the mutation source test suite, so move it into the
header of the latter to make it accessible.
2022-03-17 08:08:01 +02:00
Botond Dénes
fcf15fda94 readers: generating_reader: use noncopyable_function<>
std::function<> requires the functor it wraps to be copyable, which is
an unnecessarily strict requirement. To relax this, we use
noncopyable_function<> instead. Since the former seems to lack some
disambiguation magic of the latter, we add `_v1` and `_v2` postfixes to
manually disambiguate.
2022-03-17 06:53:44 +02:00
Gleb Natapov
a1604aa388 raft: make raft requests abortable
This patch adds an ability to pass abort_source to raft request APIs (
add_entry, modify_config) to make them abortable. A request issuer not
always want to wait for a request to complete. For instance because a
client disconnected or because it no longer interested in waiting
because of a timeout. After this patch it can now abort waiting for such
requests through an abort source. Note that aborting a request only
aborts the wait for it to complete, it does not mean that the request
will not be eventually executed.

Message-Id: <YjHivLfIB9Xj5F4g@scylladb.com>
2022-03-16 18:38:01 +01:00
Botond Dénes
0ea6dddc00 test/boost/mutation_reader_test: remove unused puppet_reader
All users use puppet_reader_v2 now.

Signed-off-by: Botond Dénes <bdenes@scylladb.com>
Message-Id: <20220316135525.211753-1-bdenes@scylladb.com>
2022-03-16 18:57:00 +02:00
Botond Dénes
afc824a109 test/boost/flat_mutation_reader_test: test_flat_mutation_reader_consume_single_partition: make ckrange conditionally inclusive
Depending on the bound weight of the position of the last fragment we
expect to read. Currently the range is unconditionally exclusive, which
might lead to an artificial difference between the read and expected
data, due to a fragment being possibly omitted.

Fixes #10229.

Tests: unit(boost/flat_mutation_reader_test:test_flat_mutation_reader_consume_single_partition)
Signed-off-by: Botond Dénes <bdenes@scylladb.com>
Message-Id: <20220304133515.74586-1-bdenes@scylladb.com>
2022-03-16 18:57:00 +02:00
Avi Kivity
975b0c0b03 Merge "tools/scylla-sstable: add validate-checksums and decompress" from Botond
"
This patchset adds two new operations to scylla-sstable:
* validate-checksums - helps identifying whether an sstable is intact or
  not, but checking the digest and the per-chunk checksums against the
  data on disk.
* decompress - helps when one wants to manually examine the content of a
  compressed sstable.

Refs: #497

Tests: unit(dev)
"

* 'scylla-sstable-validate-checksums-decompress/v3' of https://github.com/denesb/scylla:
  tools/scylla-sstable: consume_sstables(): s/no_skips/use_crawling_reader/
  tools/scylla-sstable: add decompress operation
  tools/scylla-sstables: add validate-checksums operation
  sstables/sstable: add validate_checksums()
  sstables/sstable: add raw_stream option to data_stream()
  sstables/sstable: make data_stream() and data_read() public
  utils/exceptions: add maybe_rethrow_exception()
2022-03-16 18:56:48 +02:00
Avi Kivity
c4a992564b Merge "Assorted fixes for Fedora 36 build" from Pavel S
"
This mini-series contains a few trivial fixes to be able
to build scylla on Fedora 36 Pre-Release, which will
soon enter "Beta" state.

It's mostly fixes due to some changes to external dependencies,
e.g. boost.outcome and libfmt.

Tests: unit(dev)
"

* 'fc36_build_fixes_v1' of https://github.com/ManManson/scylla:
  schema: fix build issues with libstdc++ 12
  treewide: fix compilation issues with fmtlib 8.1.0+
  utils/result.hh: add missing header includes for boost.outcome
2022-03-16 18:56:02 +02:00
Pavel Emelyanov
009c449cc3 replica: Push sharded<system_keyspace> down to parse_system_tables
The method needs to call merge_schema() that will need system keyspace
instance at hand. The parse_s._t. method is boot-time one, pushing the
main-local instance through it is fine

Signed-off-by: Pavel Emelyanov <xemul@scylladb.com>
2022-03-16 14:24:40 +03:00
Pavel Emelyanov
f18a80852e storage_service: Keep sharded<system_keyspace> reference
Storage service uses system keyspace on boot heavily

Signed-off-by: Pavel Emelyanov <xemul@scylladb.com>
2022-03-16 14:24:40 +03:00
Pavel Emelyanov
42e733bdf7 migration_manager: Keep sharded<system_keyspace> reference
The main target here is system_keyspace::update_schema_version() which
is now static, but needs to have system_keyspace at "this". Migration
manager is one of the places that calls that method indirectly.

Signed-off-by: Pavel Emelyanov <xemul@scylladb.com>
2022-03-16 14:24:40 +03:00
Pavel Emelyanov
b761e558b5 system_keyspace: Keep local_cache reference on board
For now it's a reference, but all users of the cache will be
eventually switched into using system_keyspace.

In cql-test-env cache starting happens earlier than it was
before, but that's OK, it just initializes empty instances.
In main cache starts at the same time as before patching.

Signed-off-by: Pavel Emelyanov <xemul@scylladb.com>
2022-03-16 13:57:59 +03:00
Pavel Emelyanov
1bcb6c13a5 system_keyspace: Move minimal_setup into start
Start happens at exactly the same place. One thing to take care
of is that it happens on all shards.

Signed-off-by: Pavel Emelyanov <xemul@scylladb.com>
2022-03-16 13:57:59 +03:00
Pavel Emelyanov
7ef69b8189 system_keyspace: Make sharded object
The db::system_keyspace was made a class some time ago, time to create
a standard sharded<> object out of it. It needs query processor and
database. None of those depensencies is started early enough, so the
object for now starts in two steps -- early instances creation and
late start.

The instances will carry qctx and local_cache on board and all the
services that need those two will depend on system-keyspace. Its start
happens at exactly the same place where system_keyspace::setup happens
thus any service that will use system_keyspace will be on the same
safe side as it is now.

In the further future the system_keyspace will be equpped with its
own query processor backed by local replica database instance, instead
of the whole storage proxy as it is now.

Signed-off-by: Pavel Emelyanov <xemul@scylladb.com>
2022-03-16 13:57:59 +03:00
Pavel Solodovnikov
95c8d65949 treewide: fix compilation issues with fmtlib 8.1.0+
Due to fd62fba985
scoped enums are not automatically converted to integers anymore,
this is the intended behavior, according to the fmtlib devs.

A bit nicer solution would be to use `std::to_underlying`
instead of a direct `static_cast`, but it's not available until
C++23 and some compilers are still missing the support for it.

Tests: unit(dev)

Signed-off-by: Pavel Solodovnikov <pa.solodovnikov@scylladb.com>
2022-03-16 12:31:50 +03:00
Raphael S. Carvalho
58e520ab1d compaction: Move run_off_strategy_compaction() into compaction manager
Compaction manager is calling back the table to run off-strategy compaction,
but the logic clearly belongs to manager which should perform the
operation independently and only call table to update its state with the
result.

Signed-off-by: Raphael S. Carvalho <raphaelsc@scylladb.com>
Message-Id: <20220315174504.107926-2-raphaelsc@scylladb.com>
2022-03-16 09:55:52 +02:00
Mikołaj Sielużycki
a8cb7bf677 readers: Make result_collector use queue reader handle v2.
Transitively modifies streaming_virtual_table::as_mutation_source,
along with the tests.

Closes #10223
2022-03-15 17:02:28 +02:00
Botond Dénes
ddf9dee9d8 sstables/sstable: add validate_checksums()
Sstables have two kind of checksums: per-chunk checksums and
full-checksum (digest) calculated over the entire content of Data.db.

The full-checksum (digest) is stored in Digest.crc
(component_type::Digest).

When compression is used, the per-chunk checksum is stored directly
inside Data.db, after each compressed chunk. These are validated on
read, when decompressing the respective chunks.
When no compression is used, the per-chunk checksum is stored separately
in CRC.db (component_type::CRC). Chunk size is defined and stored in said
component as well.

In both compressed and uncompressed sstables, checksums are calculated
on the data that is actually written to disk, so in case of compressed
data, on the compressed data.

This method validates both the full checksum and the per-chunk checksum
for the entire Data.db.
2022-03-15 14:52:15 +02:00
Botond Dénes
61028ad718 evicatble_reader: avoid preemption pitfall around waiting for readmission
Permits have to wait for re-admission after having been evicted. This
happens via `reader_permit::maybe_wait_readmission()`. The user of this
method -- the evictable reader -- uses it to re-wait admission when the
underlying reader was evicted. There is one tricky scenario however,
when the underlying reader is created for the first time. When the
evictable reader is part of a multishard query stack, the created reader
might in fact be a resumed, saved one. These readers are kept in an
inactive state until actually resumed. The evictable reader shares it
permit with the to-be-resumed reader so it can check whether it has been
evicted while saved and needs to wait readmission before being resumed.
In this flow it is critical that there is no preemption point between
this check and actually resuming the reader, because if there is, the
reader might end up actually recreated, without having waited for
readmission first.
To help avoid this situation, the existing `maybe_wait_readmission()` is
split into two methods:
* `bool reader_permit::needs_readmission()`
* `future<> reader_permit::wait_for_readmission()`

The evictable reader can now ensure there is no preemption point between
`needs_readmission()` and resuming the reader.

Fixes: #10187

Tests: unit(release)
Signed-off-by: Botond Dénes <bdenes@scylladb.com>
Message-Id: <20220315105851.170364-1-bdenes@scylladb.com>
2022-03-15 14:37:22 +02:00
Avi Kivity
39504dea61 Merge "Convert result builders to v2" from Botond
"
Namely the query result writer and the reconcilable result builder, used
for building results for regular queries and mutation queries (used in
read repair) respectively.
With this, there are no users left for the v1 output of the compactor,
so we remove that, making the compactor v2 all-the-way (and simpler).
This means that for regular queries, a downgrade phase is eliminated
completely, as regular queries don't store range tombstone in their
result, so no need to convert them.

Tests: unit(dev, release, debug)
"

* 'result-builders-v2/v1' of https://github.com/denesb/scylla:
  reconcilable_result_builder: remove v1 support
  query_result_builder: remove v1 support
  mutation_compactor: drop v1 related code-paths
  mutation_compactor: drop v1 support altogether from the API
  tree: migrate to the v2 consumer APIs
  test/boost/mutation_test: remove v1 specific test code
  querier: switch to v2 compactor output
  reconcilable_result_builder: add v2 support
  query_result_writer: add v2 support
  query_result_builder: make consume(range_tombstone) noop
2022-03-15 14:32:58 +02:00
Mikołaj Sielużycki
7ce0d380d4 readers: Update tests to use make_queue_reader_v2.
Closes #10220
2022-03-15 13:56:50 +02:00
Piotr Dulikowski
5d7b2c6515 utils/result_try: prevent exceptions from being caught multiple times
The `result_try` and `result_futurize_try` are supposed to handle both
failed results and exceptions in a way similar to a try..catch block.
In order to catch exceptions, the metaprogramming machinery invokes the
fallible code inside a stack of try..catch blocks, each one of them
handling one exception. This is done instead of creating a single
try..catch block, as to my knowledge it is not possible to create
a try..catch block with the number of "catch" clauses depending on a
variadic template parameter pack.

Unfortunately, a "try" with multiple "catches" is not functionally
equivalent to a "try block stack". Consider the following code:

    try {
        try {
            return execute_try_block();
        } catch (const derived_exception&) {
            // 1
        }
    } catch (const base_exception&) {
        // 2
    }

If `execute_try_block` throws `derived_exception` and the (1) catch
handler rethrows this exception, it will also be handled in (2), which
is not the same behavior as if the try..catch stack was "flat".

This causes wrong behavior in `result_try` and `result_futurize_try`.
The following snippet has the same, wrong behavior as the previous one:

    return utils::result_try([&] {
        return execute_try_block();
    },  utils::result_catch<derived_exception>([&] (const auto&& ex) {
        // 1
    }), utils::result_catch<base_exception>([&] (const auto&& ex) {
        // 2
    });

This commit fixes the problem by adding a boolean flag which is set just
before a catch handler is executed. If another catch handler is
accidentally matched due to exception rethrow, the catch handler is
skipped and exception is automatically rethrown.

Tests: unit(dev, debug)

Fixes: #10211

Closes #10216
2022-03-15 11:42:42 +02:00
Benny Halevy
e5538cf52e test: mutation_write_test: test_timestamp_based_splitting_mutation_writer: no need to downgrade reader to v1
Signed-off-by: Benny Halevy <bhalevy@scylladb.com>
Message-Id: <20220315083425.2786228-2-bhalevy@scylladb.com>
2022-03-15 11:41:11 +02:00
Benny Halevy
90edddd7e3 everywhere: use make_flat_mutation_reader_from_mutations_v2
Rather than upgrade_to_v2(make_flat_mutation_reader_from_mutations)

Signed-off-by: Benny Halevy <bhalevy@scylladb.com>
Message-Id: <20220315083425.2786228-1-bhalevy@scylladb.com>
2022-03-15 11:41:10 +02:00
Nadav Har'El
189ff5414f test/cql-pytest: implement test_tools.py without run-script cooperation
In commit afab1a97c6, we added
test_tools.py - tests for the various tools embedded in the Scylla
executable. These tests need to know where the Scylla executable is,
and also where its sstables are stored. For this, the commit added two
test parameters - "--scylla-path" and "--workdir" - with which the
"run" script communicated this knowledge to the test.

However, that implementation meant that these tests only work if the
test was run via the test/cql-pytest/run script - they won't work if
the user ran Scylla/pytest manually, or through some other script not
passing these options.

This patch drops the "--scylla-path" and "--workdir" parameters, and
instead the test figures out this information on its own:

1. To find the Scylla executable, we begin by looking (using the
   local_process_id(cql) function from the previous patch) for a
   local process which listens to our CQL connection, and then find
   the executable's path using /proc.

2. To find the Scylla data directory (which is what we really need, not
   workdir which is just a shortcut to set all directories!), we
   retrieve this configuration from the system.config table through CQL.

I tested that test_tools.py now works not only through test/cql-pytest/run
but also if I run Scylla manually and then run "pytest test_tools.py"
without any extra parameters.

Fixes #10209

Signed-off-by: Nadav Har'El <nyh@scylladb.com>
Message-Id: <20220314151125.2737815-2-nyh@scylladb.com>
2022-03-14 20:25:22 +02:00
Nadav Har'El
8ed0909cc3 test/cql-pytest: add mechanism and example of testing Scylla log messages
Generally, cql-pytest tests do not, and *should not* rely on looking up
messages in the Scylla log file: Relying on such messages makes it
impossible to run the same test against Cassandra or even a remotely-
installed Scylla, and the tests tend to break when logging (which is not
considered part of our API) changes. Moreover, usually what our dtests
achieve by looking at the log - e.g., figuring out when some event has
happened - can be achieved through official CQL APIs, and this is what
normal users do anyway (users don't normally dig through the log to
figure out when their operation completed).

However, sometimes we do want to write a test to confirm that during a
certain operation, a certain log message gets written to Scylla's log.
A desire to do this was raised by @fruch and @soyacz, so in this patch
I provide a mechanism to do this, and a trivial example - which checks
that a "Creating ..." message appears on the log whenever a table is
created, and "Dropping ..." when the table is deleted.

As is explained in detail in patches in the comment, Scylla's log file
is found automatically, without relying on Scylla's runner (such as
the script test/cql-pytest/run) communicating to the test where the log
file is. If the log file can't be found - e.g., we're testing a remote
Scylla, or if this isn't Scylla, the tests are skipped.

I would like all logfile-testing tests to be in the same file,
test_logs.py. As I explained above, I think it is a mistake for general
tests to check the log file just because they can. I think that the only
tests that should use the log file are tests deliberately written to
check what gets logged - and those can be collected in the same file.

As part of this patch, we add the utility function local_process_id(cql)
to find (if we can) the local process which listens to the connection
"cql". This utility function will later be useful in more places - for
example test_tools.py needs to find Scylla's executable.

Signed-off-by: Nadav Har'El <nyh@scylladb.com>
Message-Id: <20220314151125.2737815-1-nyh@scylladb.com>
2022-03-14 20:25:20 +02:00
Lukasz Sojka
c65f1c3b47 test/cql-pytest: add warnings test
cql client should return warnings when batch exceedes certain size.
This test verifies if response contains them.

Test covers issue: https://github.com/scylladb/scylla/issues/10196

Signed-off-by: Lukasz Sojka <lukasz.sojka@scylladb.com>

Closes #10197
2022-03-14 19:49:06 +02:00
Raphael S. Carvalho
fce9d869b4 compaction: Move table::compact_sstables() into compaction manager
Table submits compaction request into manager, which in turn calls
back table to run the compaction when the time has come, i.e.:
table -> compaction manager -> table -> execute compaction

But manager should not rely on table to run compaction, as compaction
execution procedure sits one layer below the manager and should be
accessed directly by it, i.e:

table -> compaction manager -> execute compaction

This makes code easier to understand and update_compaction_history()
can now be noop for unit tests using table_state.

Signed-off-by: Raphael S. Carvalho <raphaelsc@scylladb.com>
Message-Id: <20220311023410.250149-1-raphaelsc@scylladb.com>
2022-03-14 15:39:23 +02:00
Botond Dénes
964d9e033d Merge "raft_group_registry: drain_on_shutdown" from Benny Halevy
"
This series hardens raft_group_registry::stop_servers
and uses it to drain_on_shutdown, called before
the database is stopped in cql_test_env.
(Not needed for main).

raft_group_registry deferred_stop is introduced right after
the service is started to make sure it's properly stopped
even if there's an exception at any point while starting.

Test: unit(dev)
"

* tag 'raft_group_registry-drain_on_shutdown-v1' of https://github.com/bhalevy/scylla:
  cql_test_env: raft_group_registry::drain_on_shutdown before stopping the database
  raft_group_registry: harden stop_servers
  raft_group_registry: delete unused _shutdown_gate
2022-03-14 14:22:46 +02:00
Mikołaj Sielużycki
1d84a254c0 flat_mutation_reader: Split readers by file and remove unnecessary includes.
The flat_mutation_reader files were conflated and contained multiple
readers, which were not strictly necessary. Splitting optimizes both
iterative compilation times, as touching rarely used readers doesn't
recompile large chunks of codebase. Total compilation times are also
improved, as the size of flat_mutation_reader.hh and
flat_mutation_reader_v2.hh have been reduced and those files are
included by many file in the codebase.

With changes

real	29m14.051s
user	168m39.071s
sys	5m13.443s

Without changes

real	30m36.203s
user	175m43.354s
sys	5m26.376s

Closes #10194
2022-03-14 13:20:25 +02:00
Benny Halevy
26b1be0b8f test: lib: random_mutation_generator: accept optional random seed
Provide an easy way to instrument a particular test case to use
a given random number seed (that's curretly already printed to
the test log).

Refs #5349

Signed-off-by: Benny Halevy <bhalevy@scylladb.com>
Message-Id: <20210907114537.3464004-1-bhalevy@scylladb.com>
2022-03-14 13:09:36 +02:00
Benny Halevy
8481852c91 cql_test_env: raft_group_registry::drain_on_shutdown before stopping the
database

We're currently stopping raft_gr before
shutting the database down, but we fail to do that if
anything goes wrong before that, e.g. if
distributed_loader::init_non_system_keyspaces fails.

This change splits drain_on_shutdown out of stop()
to stop the raft groups before the database is stopped
and does the rest in a deferred_stop placed right
after the rafr_gr registry is strated.

Signed-off-by: Benny Halevy <bhalevy@scylladb.com>
2022-03-14 11:49:44 +02:00
Nadav Har'El
383aa326cc cql-pytest: translate Cassandra's tests for BATCH operations
This is a translation of Cassandra's CQL unit test source file
validation/operations/BatchTest.java into our our cql-pytest framework.

This test file includes 13 tests for various types of BATCH operations.
All tests pass on Scylla - no known or new bugs were reproduced.

Two of the tests involve very slow testing of TTLs, so after verifying
they work I marked them "skip" for now (we can always turn them on later,
perhaps after reducing the length or number of the sleeps).

Signed-off-by: Nadav Har'El <nyh@scylladb.com>
Message-Id: <20220313121634.2611423-1-nyh@scylladb.com>
2022-03-14 09:43:02 +01:00
Botond Dénes
924ff6a503 mutation_compactor: drop v1 support altogether from the API
Fully mechanical change. Drop all v1 types, template types. Internal
code is left unchanged, will be made v2 only in the next patch.
2022-03-11 09:24:05 +02:00
Botond Dénes
eacdfb2cb7 test/boost/mutation_test: remove v1 specific test code
From test_compactor_range_tombstone_spanning_many_pages, preparing for
the retirement of the v1 output of the compactor.
2022-03-11 09:24:05 +02:00
Botond Dénes
0b5217052d querier: switch to v2 compactor output
The change is mostly mechanical: update all compactor instances to the
_v2 variant and update all call-sites, of which there is not that many.
As a consequence of this patch, queries -- both single-partition and
range-scans -- now do the v2->v1 conversion in the consumers, instead of
in the compactor.
2022-03-11 09:24:05 +02:00
Botond Dénes
7e0b51ff23 Merge 'Overhaul compaction_manager::task' from Benny Halevy
The series overhauls the compaction_manager::task design and implementation
by properly layering the functionality between the compaction_manager
that deals with generic task execution, and the per-task business logic that is defined
in a set of classes derived from the generic task class.

While at it, the series introduces `task::state` and a set of helper functions to manage it
to prevent leaks in the statistics, fixing #9974.

Two more stats counter were exposed: `completed_tasks` and a new `postponed_tasks`.

Test: sstable_compaction_test
Dtest: compaction_test.py compaction_additional_test.py

Fixes #9974

Closes #10122

* github.com:scylladb/scylla:
  compaction_manager: use coroutine::switch_to
  compaction_manager::task: drop _compaction_running
  compaction_manager: move per-type logic to derived task
  compaction_manager: task: add state enum
  compaction_manager: task: add maybe_retry
  compaction_manager: reevaluate_postponed_compactions: mark as noexcept
  compaction_manager: define derived task types
  compaction_manager: register_metrics: expose postponed_compactions
  compaction_manager: register_metrics: expose failed_compactions
  compaction_manager: register_metrics: expose _stats.completed_tasks
  compaction: add documentation for compaction_type to string conversions
  compaction: expose to_string(compaction_type)
  compaction_manager: task: standardize task description in log messages
  compaction_manager: refactor can_proceed
  compaction_manager: pass compaction_manager& to task ctor
  compaction_manager: use shared_ptr<task> rather than lw_shared_ptr
  compaction_manager: rewrite_sstables: acquire _maintenance_ops_sem once
  compaction_manager: use compaction_state::lock only to synchronize major and regular compaction
2022-03-10 13:33:56 +02:00
Benny Halevy
a2a5e530f0 compaction_manager: move per-type logic to derived task
Move the business logic into the task specific classes.
Separating initialization during task construction,
from the compaction_done task, moved into
a do_run() method, and in some cases moving
a lambda function that was called per table (as in
rewrite_sstables) into a private method of the
derived class.

Signed-off-by: Benny Halevy <bhalevy@scylladb.com>
2022-03-10 12:20:01 +02:00
Mikołaj Sielużycki
5920349357 row_cache: Make row_cache reader from sstables compacting.
Reading data from sstables without compacting first puts
unnecessary pressure on the cache. The mutation streams
need to be resolved anyway before passing to subsequent
consumers, so it's better to do it as close to the
source as possible.

Fixes: #3568

Closes #10188
2022-03-10 11:40:10 +02:00