test_broadcast_kv_store does not use await or yield at all, so
there is no need to mark it with "asyncio" mark.
tested using
```
SCYLLA_HOME=$HOME/scylla build/cmake/scylla --overprovisioned --developer-mode=yes --consistent-cluster-management=true --experimental-features=broadcast-tables
...
pytest broadcast_tables/test_broadcast_tables.py
```
the test still passes.
Signed-off-by: Kefu Chai <kefu.chai@scylladb.com>
Closes#13006
Recently, we overhauled the error handling of UNSET_VALUE in various
places where it is not allowed. This patch adds two more regression
tests for this error handling. Both tests pass on Scylla today, pass
on Cassandra, but fail on earlier Scylla (e.g., I tested 5.1.5):
The first test does INSERT into clustering key UNSET_VALUE.
An UNSET_VALUE is designed to skip part of the write - not an entire
write - so this attempt should fail - not silently be skipped.
The write indeed fails with an error on Cassandra, and on recent
Scylla, but silently did nothing in older Scylla which leads this
test to fail there.
The second test does the same thing with LWT (adding an "IF NOT EXISTS")
added to the insert. Scylla's failure here was even more spectacular -
it crashed (as reported in issue #13001) instead of silently skipping
the right. The test passes on Scylla today and on Cassandra, which
both report the failure cleanly.
Refs #13001.
Signed-off-by: Nadav Har'El <nyh@scylladb.com>
Closes#13007
Cassandra is very strict in the CLUSTERING ORDER BY clause which it
allows when creating a materialized view - if it appears, it must
list all the clustering columns of the view. Scylla is less strict -
a subset of the clustering columns may be specified. But Scylla was
*too* lenient - a user could specify non-clustering columns and even
non-existent columns and Scylla would not fail the MV creation.
This patch fixes that - with it MV creation fails if anything besides
clustering columns are listed on CLUSTERING ORDER BY.
An xfailing test we had for this case no longer fails after this
patch so its xfail mark is removed. We also add a few more corner
cases to the tests.
This patch also fixs one C++ test which had exactly the error that this
patch detects - the test author tried to use the partition key, instead
of the clustering key, in CLUSTERING ORDER BY (this error had no effect
because the specified order, "asc", was the default anyway).
Fixes#10767
Signed-off-by: Nadav Har'El <nyh@scylladb.com>
Closes#12885
Said method can now throw `std::bad_alloc` since aab5954. All call-sites should have been adapted in the series introducing the throw, but some managed to slip through because the oom unit test didn't run in debug mode. This series fixes the remaining unpatched call-sites and makes sure the test runs in debug mode too, so leaks like this are detected.
Fixes: #12767Closes#12756
* github.com:scylladb/scylladb:
test/boost/reader_concurreny_semaphore_test: run oom protection tests in debug mode
treewide: adapt to throwing reader_concurrency_semaphore::consume()
since "token()" computes the token for a given partition key,
if we pass the key of the wrong type, it should reject.
in this change,
* we validate the keys before returning the "token()" function.
* drop the "xfail" decorator from two of the tests. they pass
now after this fix.
* change the tests which previously passed the wrong number of
arguments containing null to "token()" and expect it to return
null, so they verify that "token()" should reject these
arguments with the expected error message.
Fixes#10448
Signed-off-by: Kefu Chai <kefu.chai@scylladb.com>
Closes#12991
Aggregation query on counter column is failing because forward_service is looking for function with counter as an argument and such function doesn't exist. Instead the long type should be used.
Fixes: #12939Closes#12963
* github.com:scylladb/scylladb:
test:boost: counter column parallelized aggregation test
service:forward_service: use long type when column is counter
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
Task manager task implementation that covers the major keyspace
compaction which can be start through /storage_service/keyspace_compaction/
api.
Closes#12661
* github.com:scylladb/scylladb:
test: add test for major keyspace compaction tasks
compaction: create task manager's task for major keyspace compaction
compaction: copy run_on_existing_tables to task_manager_module.cc
compaction: add major_compaction_task_impl
compacition: add pure virtual compaction_task_impl
compaction: add compaction module getter to compaction manager
Some test cases can be made a bit more compact by using the sugar provided by the aforementioned sugar
Closes#12965
* github.com:scylladb/scylladb:
test: Make use of reusable_sst default format
tests: Use reusable_sst() where applicable
The sstable_test_env::reusable_sst() has default value for the format
argument. Patch the test cases that don't use one while at it.
Signed-off-by: Pavel Emelyanov <xemul@scylladb.com>
The reusable_sst() is intented to be used to load the pre-existing
sstable from the test/resources directory and .load() them. Some test
cases, however, still do it "by hand".
Signed-off-by: Pavel Emelyanov <xemul@scylladb.com>
it already includes the necessary bits used by test-perf, so let's
just link the latter to the former.
Signed-off-by: Kefu Chai <kefu.chai@scylladb.com>
BUILD_TESTING is an option exposed by CTest module, so let's
include CTest module, and check if BUILD_TESTING is enabled
before include boost based tests.
Signed-off-by: Kefu Chai <kefu.chai@scylladb.com>
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
Refs #11710
Allows reusing regex for segment matching (for opening left-over segments after crash).
Should remove any stalls caused by commitlog replay preparation.
v2: Add unit test for descriptor parsing
Closes#12112
Currently, applying a schema change on a replica works like this:
Collect all affected keyspaces from incoming mutations
Read current state of schema
Apply the mutations
Read new state of schema
The "Read ... state of schema" step reads all kinds of schema
objects. In particular, to read the "table" objects, it does the
following:
for every affected keyspace k:
read all mutations from system_schema.tables for k
extract all existing table names from those mutations
for every existing table:
read mutations from {tables, columns, indexes, view_virtual_columns, ...} for that table
As you can see, the number of reads performed is O(nr tables in a
keyspace), not O(nr tables in a change). This means that making a
sequence of schema changes, like adding a table, is quadratic.
Another aspect which magnifies this is that we don't read those tables
using a single scan, but issue individual queries for each table
separately.
This patch optimizes this by considering only affected tables when
reading schema for the purpose of diff calculation.
When mutations contain multi-table deletions, we still read the
set of tables, like before. This could be optimized by looking
at the database to get the list, but it's not part of the patch.
I tested this using a test case provided by Kamil (kbr-scylla@53fe154)
./test.py --mode debug test_many_schema_changes -s
The test bootstraps a cluster and then creates about 40 schema
changes. Then a new node is bootstrapped and replays those changes via
group0.
In debug mode, each change takes roughly 2s to process before the
patch, and 0.5s after the patch.
The whole replay is reduced to 56% of what was before:
Before (1m19s) :
INFO 2023-01-20 19:44:35,848 [shard 0] raft_group0 - setup_group0: ensuring that the cluster has fully upgraded to use Raft...
INFO 2023-01-20 19:45:54,844 [shard 0] raft_group0 - setup_group0: waiting for peers to synchronize state...
After (45s):
INFO 2023-01-20 22:02:51,869 [shard 0] raft_group0 - setup_group0: ensuring that the cluster has fully upgraded to use Raft...
INFO 2023-01-20 22:03:36,834 [shard 0] raft_group0 - setup_group0: waiting for peers to synchronize state...
Closes#12592Closes#12592
The test relies on exact request size, this doesn't work if compression is applied. The driver enables compression only if both the server and the client agree on the codec to use. If compression package
(e.g. lz4) is not installed, the compression
is not used.
The trick with locally_supported_compressions is needed since I couldn't find any standard means to disable compression other than the compression flag
on the cluster object, which seemed too broad.
fixes: #12836Closes#12854
* github.com:scylladb/scylladb:
test_shed_too_large_request: clarify the comments
test_shed_too_large_request: use smaller test string
test_shed_too_large_request fix: disable compression
In test_v2_apply_monotonically_is_monotonic_on_alloc_failures we
generate mutations with non-full continuity, so we should pass
is_evictable::yes to apply_monotonically(). Otherwise, it will assume
fully-continuous versions and not try to maintain continuity by
inserting sentinels.
This manifested in sporadic failures on continuity check.
Fixes#12882Closes#12921
* github.com:scylladb/scylladb:
test: mutation_test: Fix sporadic failure due to continuity mismatch
test: mutation_test: Fix copy-paste mistake in trace-level logging
This is a translation of Cassandra's CQL unit test source file
validation/operations/CompactStorageTest.java into our cql-pytest
framework.
This very large test file includes 86 tests for various types of
operations and corner cases of WITH COMPACT STORAGE tables.
All 86 tests pass on Cassandra (except one using a deprecated feature
that needs to be specially enabled). 30 of the tests fail on Scylla
reproducing 7 already-known Scylla issues and 7 previously-unknown issues:
Already known issues:
Refs #3882: Support "ALTER TABLE DROP COMPACT STORAGE"
Refs #4244: Add support for mixing token, multi- and single-column
restrictions
Refs #5361: LIMIT doesn't work when using GROUP BY
Refs #5362: LIMIT is not doing it right when using GROUP BY
Refs #5363: PER PARTITION LIMIT doesn't work right when using GROUP BY
Refs #7735: CQL parser missing support for Cassandra 3.10's new "+=" syntax
Refs #8627: Cleanly reject updates with indexed values where value > 64k
New issues:
Refs #12471: Range deletions on COMPACT STORAGE is not supported
Refs #12474: DELETE prints misleading error message suggesting
ALLOW FILTERING would work
Refs #12477: Combination of COUNT with GROUP BY is different from
Cassandra in case of no matches
Refs #12479: SELECT DISTINCT should refuse GROUP BY with clustering column
Refs #12526: Support filtering on COMPACT tables
Refs #12749: Unsupported empty clustering key in COMPACT table
Refs #12815: Hidden column "value" in compact table isn't completely hidden
Signed-off-by: Nadav Har'El <nyh@scylladb.com>
Closes#12816
they are part of the CQL type system, and are "closer" to types.
let's move them into "types" directory.
the building systems are updated accordingly.
the source files referencing `types.hh` were updated using following
command:
```
find . -name "*.{cc,hh}" -exec sed -i 's/\"types.hh\"/\"types\/types.hh\"/' {} +
```
the source files under sstables include "types.hh", which is
indeed the one located under "sstables", so include "sstables/types.hh"
instea, so it's more explicit.
Signed-off-by: Kefu Chai <kefu.chai@scylladb.com>
Closes#12926
because it has a member variable whose type is a reference. and a
reference cannot be reassigned. this silences following warning from Clang:
```
/home/kefu/dev/scylladb/test/boost/chunked_vector_test.cc:152:27: error: explicitly defaulted move assignment operator is implicitly deleted [-Werror,-Wdefaulted-function-deleted]
exception_safe_class& operator=(exception_safe_class&&) = default;
^
/home/kefu/dev/scylladb/test/boost/chunked_vector_test.cc:132:31: note: move assignment operator of 'exception_safe_class' is implicitly deleted because field '_esc' is of reference type 'exception_safety_checker &'
exception_safety_checker& _esc;
^
```
Signed-off-by: Kefu Chai <kefu.chai@scylladb.com>
In test_v2_apply_monotonically_is_monotonic_on_alloc_failures we
generate mutations with non-full continuity, so we should pass
is_evictable::yes to apply_monotonically(). Otherwise, it will assume
fully-continuous versions and not try to maintain continuity by
inserting sentinels.
This manifested in sporadic failures on continuity check.
Fixes#12882
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>
`int_range::make_singular()` accepts a single `int` as its parameter,
so there is no need to brace the paramter with `{}`. this helps to silence
the warning from Clang, like:
```
/home/kefu/dev/scylladb/test/perf/perf_fast_forward.cc:1396:63: error: braces around scalar initializer [-Werror,-Wbraced-scalar-init]
check_no_disk_reads(test(int_range::make_singular({100}))),
^~~~~
```
Signed-off-by: Kefu Chai <kefu.chai@scylladb.com>
Closes#12903
Task ttl can be set with task manager test api, which is disabled
in release mode.
Move get_and_update_ttl from task manager test api to task manager
api, so that it can be used in release mode.
Closes#12894
Said tests require on being run with a limited amount of memory to be
really useful. When the memory amount is unexpected, they silently exit.
Which is exactly what they did in debug mode too, where the amount of
memory available cannot be controlled.
Disable the check in debug mode.
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)?
This patch adds a reproducer for the bug described in issue #7964 -
The restriction `where k=1 and c=2` (when k,c are the key columns)
returns (at most) a single row so doesn't need ALLOW FILTERING,
but if we add a third restriction, say `v=2`, this still processes
at most a single row so doesn't need ALLOW FILTERING - and both
Scylla and Cassandra get it wrong - so it's marked with both xfail
and cassandra_bug.
The patch also adds another test that for longer partition slices,
e.g., `where k=1 and c>2`, although the slice itself doesn't need
filtering, if we add `v=2` here we suddenly do need ALLOW FILTERING,
because the slice itself may be a large number of rows, and adding
`v=2` may restrict it to just a few results. This test passes
on both Scylla and Cassandra.
Issue #7964 mentioned these scenarios and even had some example code,
but we never added it to the test suite, so we finally do it now.
Refs #7964
Signed-off-by: Nadav Har'El <nyh@scylladb.com>
Closes#12850
Tests of each module that is integrated with task manager use
calls to task manager api. Boilerplate to call, check status, and
get result may be reduced using functions.
task_manager_utils.py contains wrappers for task manager api
calls and helpers that may be reused by different tests.
Closes#12844
* github.com:scylladb/scylladb:
test: use functions from task_manager_utils.py in test_task_manager.py
test: add task_manager_utils.py
as an abstract base class `output_writer` is inherited by both
`json_output_writer` and `text_output_writer`. and `output_manager`
manages the lifecycles of used writers using
`std::unique_ptr<output_writer>`.
before this change, the dtor of `output_writer` is not marked as
virtual, so when its dtor is invoked, what gets called is the base
class's dtor. but the dtor of `json_output_writer` is non-trivial
in the sense that this class is aggregated by a bunch of member
variables. if we don't invoke its dtor when destroying this object,
leakage is expected.
so, in this change, the dtor of `output_writer` is marked as virtual,
this makes all of its derived classes' dtor virtual. and the right
dtor is always called.
test/perf is only designed for testing, and not used in production,
also, this feature was recently integrated into scylla executable in
228ccdc1c7.
so there is no need to backport this change.
change should also silence the warning from Clang 17:
```
/home/kefu/.local/bin/../lib/gcc/x86_64-pc-linux-gnu/13.0.1/../../../../include/c++/13.0.1/bits/unique_ptr.h:100:2: error: delete called on 'output_writer' that is abstract but has non-virtual destructor [-Werror,-Wdelete-abstract-non-virtual-dtor]
delete __ptr;
^
/home/kefu/.local/bin/../lib/gcc/x86_64-pc-linux-gnu/13.0.1/../../../../include/c++/13.0.1/bits/unique_ptr.h:405:4: note: in instantiation of member function 'std::default_delete<output_writer>::operator()' requested here
get_deleter()(std::move(__ptr));
^
/home/kefu/.local/bin/../lib/gcc/x86_64-pc-linux-gnu/13.0.1/../../../../include/c++/13.0.1/bits/stl_construct.h:88:15: note: in instantiation of member function 'std::unique_ptr<output_writer>::~unique_ptr' requested here
__location->~_Tp();
^
```
Signed-off-by: Kefu Chai <kefu.chai@scylladb.com>
Closes#12888
This patch adds a cql-pytest test for an old secondary-index bug
that was described three years ago in issue #5823. cql-pytest makes
it easy to run the same test against different versions of Scylla,
and it was used to check that the bug existed in Scylla 2.3.0 but
was gone by 2.3.5, and also not present in master or in 2021.1.
A bit about the bug itself:
A secondary index is useful for equality restrictions (a=2) but can't be
used for inequality restrictions (a>=2). In Scylla 3.2.0 we used to have a
bug that because the restriction a>=2 couldn't be used through the index,
it was ignored completely. This is of course a mistake.
Refs #5823
Signed-off-by: Nadav Har'El <nyh@scylladb.com>
Closes#12856
Since 5c0f9a8180 ("mutation_partition: Switch cache of
rows onto B-tree") it's no longer in use, except in some
performance test, so remove it.
Although scylla-gdb.py is sometimes used with older releases,
it's so outdated we can remove it from there too.
Closes#12868
There was a vague comment about CI using
larger limits for shedding. This turned out
to be false, and the real reason of different
limits is that Scylla handles the -m
command line option differently in
debug and release builds.
Debug builds use the default memory allocator
and the value of -m Scylla option
is given to each shard. In release builds
memory is evenly distributed between shards.
To accommodate for this we read the current
memory limit from Scylla metrics.
The helper class ScyllaMetrics was introduced to
handle metrics parsing logic. It can
potentially be reused for dealing with
metrics in other tests.
This patch adds a reproducer to a static-column index lookup bug
described in issue #12829: The restriction `where pk=0 and s=1 and c=3`
where pk,c are the primary key and s is an indexed static column,
results in an internal error: "clustering column id 2 >= 2".
Unfortunately, because on_internal_error() crashes Scylla in debug
mode, we need to mark this failing test with skip instead of xfail.
Refs #12829
Signed-off-by: Nadav Har'El <nyh@scylladb.com>
Closes#12852
Schema related files are moved there. This excludes schema files that
also interact with mutations, because the mutation module depends on
the schema. Those files will have to go into a separate module.
Closes#12858
The test relies on exact request size, this doesn't
work if compression is applied. The driver enables
compression only if both the server and the client
agree on the codec to use. If compression package
(e.g. lz4) is not installed, the compression
is not used.
The trick with locally_supported_compressions is needed
since I couldn't find any standard means to disable
compression other than the compression flag
on the cluster object, which seemed too broad.
Fixes#12836
In issue #12828 it was noted that Scylla requires ALLOW FILTERING
for `where b=1 and c=1` where b is an indexed static column and
c is a clustering key, and it was suggested that this is a bug.
This patch adds a test that confirms that both Scylla and Cassandra
require ALLOW FILTERING in this case. We explain in a comment that
this requirement is expected (i.e., it's not a bug), as the `b=1`
may match a huge number of rows, and the `c=1` may further match just
a few of those - i.e., it is filtering.
This test is virtually identical to the test we already had for
`where a=1 and c=1` - when `a` is an indexed regular column.
There too, the ALLOW FILTERING is required.
Closes#12828 as "not a bug".
Signed-off-by: Nadav Har'El <nyh@scylladb.com>
Closes#12848
