Commit Graph

31268 Commits

Author SHA1 Message Date
Avi Kivity
21728dff6f Merge 'cql: Remove support for null inside lists of IN values' from Jan Ciołek
Currently we support queries like:
```cql
SELECT * FROM ks.tab WHERE p IN (1, 2, null, 4);
```
Nothing can be equal to null so this is equivalent to:
```cql
SELECT * FROM ks.tab WHERE p IN (1, 2, 4);
```

Cassandra doesn't support it at all.
```cql
> SELECT * FROM ks.tab WHERE p IN (1, 2, null, 4)
Error: DbError(Invalid, "Invalid null value in condition for column p")

> SELECT * FROM ks.tab WHERE p IN (1, 2, ?, 4) # ? is NULL
Error: DbError(Invalid, "Invalid null value in condition for column p")

> SELECT * FROM ks.tab WHERE p IN ? # ? is (1, 2, null, 4)
Error: DbError(Invalid, "Invalid null value in condition for column p")
```
It makes little sense to send a null inside list of IN values and supporting it is a bit cumbersome.

Supporting it causes trouble because internally the values are represented as a list, not a tuple, and lists can't contain nulls.
Because of that code requires exceptions because in this single case there can be a null inside of a collection.

This PR starts treating a llist of IN values the same as any other list and as result nulls are forbidden inside them.

In case of a null the message is the same as any other collection:
```
null is not supported inside collections
```
I'm not entirely happy about it - someone could be confused if they received this message after a query that didn't involve any collections.
The problem with making a prettier error message is that once again we would have to give `evaluate` additional information that it's now evaluating a list of IN values. And we would end up back with `evaluate_IN_list`

I think we could consider adding some kind of generic context to evaluate. The context would contain the whole expression and a mark on the part that we are currently evaluating. Then in case of error we could use this context and use it to create more helpful error messages, e.g. point to the part of the expression where a problem occured. But that's outside of the scope of this PR.

Fixes #10579

Closes #10620

* github.com:scylladb/scylla:
  cql: Add test for null in IN list
  cql: Forbid null in lists of IN values
2022-05-24 09:15:13 +03:00
Jan Ciolek
ff3205cf19 cql: Add test for null in IN list
Added tests that check for error
when null or unset appears in a list
of IN values.

Signed-off-by: Jan Ciolek <jan.ciolek@scylladb.com>
2022-05-24 00:17:54 +02:00
Jan Ciolek
f9b1fc0b69 cql: Forbid null in lists of IN values
We used to allow nulls in lists of IN values,
i.e. a query like this would be valid:
SELECT * FROM tab WHERE pk IN (1, null, 2);

This is an old feature that isn't really used
and is already forbidden in Cassandra.

Additionally the current implementation
doesn't allow for nulls inside the list
if it's sent as a bound value.
So something like:
SELECT * FROM tab WHERE pk IN ?;
would throw an error if ? was (1, null, 2).
This is inconsistent.

Allowing it made writing code cumbersome because
this was the only case where having a null
inside of a collection was allowed.
Because of it there needed to be
separate code paths to handle regular lists
and lists of NULL values.

Forbidding it makes the code nicer and consistent
at the cost of a feature that isn't really
important.

Signed-off-by: Jan Ciolek <jan.ciolek@scylladb.com>
2022-05-24 00:17:41 +02:00
Avi Kivity
e65b3ed50a Merge 'Allow trigger off strategy compaction early for node operations' from Asias He
This patch set adds two commits to allow trigger off strategy early for node operations.

*) repair: Repair table by table internally

This patch changes the way a repair job walks through tables and ranges
if multiple tables and ranges are requested by users.

Before:

```
for range in ranges
   for table in tables
       repair(range, table)
```

After:

```
for table in tables
    for range in ranges
       repair(range, table)
```

The motivation for this change is to allow off-strategy compaction to trigger
early, as soon as a table is finished. This allows to reduce the number of
temporary sstables on disk. For example, if there are 50 tables and 256 ranges
to repair, each range will generate one sstable. Before this change, there will
be 50 * 256 sstables on disk before off-strategy compaction triggers. After this
change, once a table is finished, off-strategy compaction can compact the 256
sstables. As a result, this would reduce the number of sstables by 50X.

This is very useful for repair based node operations since multiple ranges and
tables can be requested in a single repair job.

Refs: #10462

*) repair: Trigger off strategy compaction after all ranges of a table is repaired

When the repair reason is not repair, which means the repair reason is
node operations (bootstrap, replace and so on), a single repair job contains all
the ranges of a table that need to be repaired.

To trigger off strategy compaction early and reduce the number of
temporary sstable files on disk, we can trigger the compaction as soon
as a table is finished.

Refs: #10462

Closes #10551

* github.com:scylladb/scylla:
  repair: Trigger off strategy compaction after all ranges of a table is repaired
  repair: Repair table by table internally
2022-05-23 18:58:21 +03:00
Avi Kivity
3fadae74e7 Merge "Keep cluster-join code in one place" from Pavel E
"
There are several issues with it

- it's scattered between main() and storage_service methods
- yet another incarnation of it also sits in the cql-test-env
- the prepare_to_join() and join_token_ring() names are lying to readers,
  as sometimes node joins the ring in prepare- stage
- storage service has to carry several private fields to keep the state
  between prepare- and join- parts
- some storage service dependencies are only needed to satisfy joining,
  but since they cannot start early enough, they are pushed to storage
  service uninitialized "in the hope" that it won't use them until join

This patch puts joining steps in one place and enlightens storage service
not to carry unneeded dependencies/state onboard. And eliminates one more
usage of global proxy instance while at it.

branch: https://github.com/xemul/scylla/tree/br-merge-init-server-and-join-cluster
tests: https://jenkins.scylladb.com/job/releng/job/Scylla-CI/466/
refs: #2795
"

* 'br-merge-init-server-and-join-cluster' of https://github.com/xemul/scylla:
  storage_service: Remove global proxy call
  storage_service: Remove sys_dist_ks from storage_service dependencies
  storage_service: Remove cdc_gen_service from storage_service dependencies
  storage_service: Make _cdc_gen_id local variable
  storage_service: Make _bootstrap_tokens local variable
  storage_service: Merge prepare- and join- private members
  storage_service: Move some code up the file
  storage_service: Coroutinize join_token_ring
  storage_service: Fix indentation after previous patch
  storage_service: Execute its .bootstrap() into async()
  storage_service: Dont assume async context in mark_existing_views_as_built
  storage_service: Merge init-server and join-cluster
  main, storage_service: Move wait for gossip to settle
  main, storage_service: Move passive announce subscription
  main, storage_service: Move early group0 join call
2022-05-23 17:33:02 +03:00
Piotr Dulikowski
ead7bdd6f8 storage_proxy: remove unused overload of query_mutations_locally
An overload of storage_proxy::query_mutations_locally was declared in
a35136533d which takes a vector of
partition ranges as an argument, but it was never defined. This commit
removes the unused overload declaration.

Closes #10610
2022-05-23 16:20:51 +03:00
Avi Kivity
75e001fc6a cql3: grammar: unify production for bind variables
Since 9b49d27a8 ("cql3: expr: Remove shape_type from bind_variable"),
bind variables no longer remember their context (e.g. if they are
in a scalar or vector comparison, or if they are in an IN or
other relation. Exploit that my merging all of the productions that
generate a bind variables (that are now exactly equal) into a single
marker production.

Closes #10624
2022-05-23 17:41:16 +03:00
Pavel Emelyanov
d755fdc1f4 storage_service: Remove global proxy call
Storage service needs it to calculate schema version on join. The proxy
at this point can be passed as an argument to the joining helper.

Signed-off-by: Pavel Emelyanov <xemul@scylladb.com>
2022-05-23 12:55:30 +03:00
Pavel Emelyanov
bc051387c5 storage_service: Remove sys_dist_ks from storage_service dependencies
The service in question is only needed join_cluster-time, no need to
keep it in the dependencies list. This also solves the dependency
trouble -- the distributed keyspace is sharded::start-ed after it's
passed to storage_service initialization.

Signed-off-by: Pavel Emelyanov <xemul@scylladb.com>
2022-05-23 12:55:30 +03:00
Pavel Emelyanov
5a97ba7121 storage_service: Remove cdc_gen_service from storage_service dependencies
This service is only needed join-time, it's better to pass it as
argument to join_cluster(). This solves current reversed dependency
issuse -- the cdc_gen_svc is now started after it's passed to storage
service initialization.

Signed-off-by: Pavel Emelyanov <xemul@scylladb.com>
2022-05-23 12:55:30 +03:00
Pavel Emelyanov
0c40b69411 storage_service: Make _cdc_gen_id local variable
Same as with _bootstrap_tokens -- this variable is only needed
throughout a single function invocation, so it doesn't have to be a
class member.

Signed-off-by: Pavel Emelyanov <xemul@scylladb.com>
2022-05-23 12:55:30 +03:00
Pavel Emelyanov
80bd317292 storage_service: Make _bootstrap_tokens local variable
Now it's a member on storage_service, but it was such just to carry the
set of tokens between to subsequent calls. Now when all the joining
happens in one function, the set can become local variable.

Signed-off-by: Pavel Emelyanov <xemul@scylladb.com>
2022-05-23 12:55:30 +03:00
Pavel Emelyanov
3f6d3ea601 storage_service: Merge prepare- and join- private members
These two are the real code that does preparation and joining. They are
called in async() context by public storage_service methods that had
been merged recently, so this patch merges the internals.

Signed-off-by: Pavel Emelyanov <xemul@scylladb.com>
2022-05-23 12:55:30 +03:00
Pavel Emelyanov
7ac73bb87f storage_service: Move some code up the file
No logic change, this is to keep join_token_ring next to
prepare_to_join so that the patch merging them becomes clean and small.

Signed-off-by: Pavel Emelyanov <xemul@scylladb.com>
2022-05-23 12:55:30 +03:00
Pavel Emelyanov
f478c7f29c storage_service: Coroutinize join_token_ring
Next patch will merge this method with prepare_to_join() which is
already coroutinized. To make it happen -- coroutinize it in advance.

Signed-off-by: Pavel Emelyanov <xemul@scylladb.com>
2022-05-23 12:55:30 +03:00
Pavel Emelyanov
81e7de076e storage_service: Fix indentation after previous patch
Signed-off-by: Pavel Emelyanov <xemul@scylladb.com>
2022-05-23 12:55:30 +03:00
Pavel Emelyanov
3a16d3ee95 storage_service: Execute its .bootstrap() into async()
Next patches will coroutinize join_cluster(), so the .bootstrap() method
should return a future. It's worth coroutinizing it as well, but that's
a huge change, so for now -- keep it in its own explicit async().

Signed-off-by: Pavel Emelyanov <xemul@scylladb.com>
2022-05-23 12:55:30 +03:00
Pavel Emelyanov
4a6bf57e8f storage_service: Dont assume async context in mark_existing_views_as_built
Next patches will coroutinize join_cluster(), this is a preparation to
that change.

Signed-off-by: Pavel Emelyanov <xemul@scylladb.com>
2022-05-23 12:55:30 +03:00
Pavel Emelyanov
282cc070bc storage_service: Merge init-server and join-cluster
Now they always follow one another both in main and cql-test-env.
Also, despite the name, init-server does joins the cluster when it's
just a normal node restarting, so join-cluster is called when the
cluster is already joind. This merge make the function be named as
what it really does.

Signed-off-by: Pavel Emelyanov <xemul@scylladb.com>
2022-05-23 12:55:30 +03:00
Pavel Emelyanov
b2b86b0c83 main, storage_service: Move wait for gossip to settle
And make cql-test-env configure to skip it not to slow down tests in
vain. Another side effect is that cql-test-env would trigger features
enabling at this point, but that's OK, they are enabled anyway.

Signed-off-by: Pavel Emelyanov <xemul@scylladb.com>
2022-05-23 12:55:30 +03:00
Pavel Emelyanov
b842167bcc main, storage_service: Move passive announce subscription
Storage service already has a vector of random subscription scope
holders, this becomes yet another one. This partially reverts
e4f35e2139, which's half-step backwards, but so far I've no better
ideas where to track that scope guard.

Signed-off-by: Pavel Emelyanov <xemul@scylladb.com>
2022-05-23 12:55:30 +03:00
Pavel Emelyanov
89163a3be4 main, storage_service: Move early group0 join call
It happens right after the prepare to join, moving it at the end of the
latter call doesn't change the code logic. A side effect -- this removes
a silly join_group0() one-line helper.

Signed-off-by: Pavel Emelyanov <xemul@scylladb.com>
2022-05-23 12:55:30 +03:00
Nadav Har'El
02fb6f33fb test/*/run: improve signal handling in test runners
When a user runs a script and presses control-C, a SIGINT (signal 2)
gets sent to every process in the script's "process group". By default,
every subprocess started by a script joins the parent's process group.

Our test/*/run test-runner scripts typically start two processes: scylla
and pytest. If we keep them in the same process group, a control-C
would kill them in a random order and that is ugly - if Scylla is
killed before pytest, we'll see a few test failures before pytest
is finally killed. So the existing code put Scylla in its own process
group, and killed it on exit after killing pytest.

But there were a few inconsistencies in our implementation, leading
to some annoying behaviors:

1. Doing "kill -2" to the runner's process (not a control-C which sends
   a signal to the process group) caused scylla and pytest to be killed
   on exit. So far so good. But, we should kill their entire process
   groups, not just the one process. This is important when pytest starts
   its own subprocesses (as happens in cql-pytest/test_tools.py),
   otherwise they just remain running.
   We need to call pgkill() instead of kill(), but also we forgot
   to start a new process group for the pytest run - so this patch
   fixes it.

2. Our exit handler - which kills the subprocesses - only gets called
   on signals which Python catches, and this is only SIGINT. Killing
   the test runner with SIGTERM or SIGHUP before this patch caused
   the subprocesses to be left running. In this patch we also catch
   SIGTERM and SIGHUP, so our exit handler is also run in that case.

Signed-off-by: Nadav Har'El <nyh@scylladb.com>

Closes #10629
2022-05-23 12:24:19 +03:00
Piotr Sarna
84833e495e Merge 'Additional cql-pytest tests related to WHERE expressions' from Nadav Har'El
This small series includes a few more CQL tests in the cql-pytest
framework.

The main patch is a translation of a unit test from Cassandra that
checks the behavior of restrictions (WHERE expressions, filtering) in
different cases.  It turns out that Cassandra didn't implement some
cases - for example filtering on unfrozen UDTs - but Scylla does
implement them. So in the translated test, the
checks-that-these-features-generate-an-error from Cassandra are
commented out, and this series also includes separate tests for these
Scylla-unique features to check that they actually work correctly and
not just that they exist.

Closes #10611

* github.com:scylladb/scylla:
  cql-pytest: translate Cassandra's tests for relations
  test/cql-pytest: add test for filtering UDTs
  test/cql-pytest: tests for IN restrictions and filtering
  test/cql-pytest: test more cases of overlapping restrictions
2022-05-23 11:10:24 +02:00
Avi Kivity
6a3494442e Update abseil submodule
* abseil f70eadad...9e408e05 (109):
  > Cord: workaround a GCC 12.1 bug that triggers a spurious warning
  > Change workaround for MSVC bug regarding compile-time initialization to trigger from MSC_VER 1910 to 1930. 1929 is the last _MSC_VER for Visual Studio 2019.
  > Don't default to the unscaled cycle clock on any Apple targets.
  > Use SSE instructions for prefetch when __builtin_prefetch is unavailable
  > Replace direct uses of __builtin_prefetch from SwissTable with the wrapper functions.
  > Cast away an unused variable to play nice with -Wunused-but-set-variable.
  > Use NullSafeStringView for const char* args to absl::StrCat, treating null pointers as "" Fixes #1167
  > raw_logging: Extract the inlined no-hook-registered behavior for LogPrefixHook to a default implementation.
  > absl: fix use-after-free in Mutex/CondVar
  > absl: fix live-lock in CondVar
  > Add a stress test for base_internal::ThreadIdentity reuse.
  > Improve compiler errors for mismatched ParsedFormat inputs.
  > Internal change
  > Fix an msan warning in cord_ringbuffer_test
  > Fix spelling error "charachter"
  > Document that Consume(Prefix|Suffix)() don't modify the input on failure
  > Fixes for C++20 support when not using std::optional.
  > raw_logging: Document that AbortHook's buffers live for as long as the process remains alive.
  > raw_logging: Rename SafeWriteToStderr to indicate what about it is safe (answer: it's async-signal-safe).
  > Correct the comment about the probe sequence.  It's (i/2 + i)/2 not (i/2 - i)/2.
  > Improve analysis of the number of extra `==` operations, which was overly complicated, slightly incorrect.
  > In btree, move rightmost_ into the CompressedTuple instead of root_.
  > raw_logging: Rename LogPrefixHook to reflect the other half of it's job (filtering by severity).
  > Don't construct/destroy object twice
  > Rename function_ref_benchmark.cc into more generic function_type_benchmark.cc, add missing includes
  > Fixed typo in `try_emplace` comment.
  > Fix a typo in a comment.
  > Adds ABSL_CONST_INIT to initializing declarations where it is missing
  > Automated visibility attribute cleanup.
  > Fix typo in absl/time/time.h
  > Fix typo: "a the condition" -> "a condition".
  > Export of internal Abseil changes
  > Export of internal Abseil changes
  > Export of internal Abseil changes
  > Export of internal Abseil changes
  > Export of internal Abseil changes
  > Export of internal Abseil changes
  > Export of internal Abseil changes
  > Export of internal Abseil changes
  > Export of internal Abseil changes
  > Fix build with uclibc-ng (#1145)
  > Export of internal Abseil changes
  > Export of internal Abseil changes
  > Replace the implementation of the Mix function in arm64 back to 128bit multiplication (#1094)
  > Support for QNX (#1147)
  > Export of internal Abseil changes
  > Export of internal Abseil changes
  > Export of internal Abseil changes
  > Exclude unsupported x64 intrinsics from ARM64EC (#1135)
  > Export of internal Abseil changes
  > Export of internal Abseil changes
  > Export of internal Abseil changes
  > Export of internal Abseil changes
  > Export of internal Abseil changes
  > Add NetBSD support (#1121)
  > Export of internal Abseil changes
  > Export of internal Abseil changes
  > Export of internal Abseil changes
  > Some trivial OpenBSD-related fixes (#1113)
  > Export of internal Abseil changes
  > Export of internal Abseil changes
  > Export of internal Abseil changes
  > Add support of loongarch64 (#1110)
  > Export of internal Abseil changes
  > Export of internal Abseil changes
  > Export of internal Abseil changes
  > Export of internal Abseil changes
  > Export of internal Abseil changes
  > Export of internal Abseil changes
  > Export of internal Abseil changes
  > Export of internal Abseil changes
  > Export of internal Abseil changes
  > Disable ABSL_INTERNAL_ENABLE_FORMAT_CHECKER under VsCode/Intellisense (#1097)
  > Export of internal Abseil changes
  > Export of internal Abseil changes
  > Export of internal Abseil changes
  > Export of internal Abseil changes
  > Export of internal Abseil changes
  > Export of internal Abseil changes
  > macos: support Apple Universal 2 builds (#1086)
  > cmake: make `random_mocking_bit_gen` library public. (#1084)
  > cmake: use target aliases from local Google Test checkout. (#1083)
  > Export of internal Abseil changes
  > Export of internal Abseil changes
  > Export of internal Abseil changes
  > Export of internal Abseil changes
  > Export of internal Abseil changes
  > Export of internal Abseil changes
  > Export of internal Abseil changes
  > cmake: add ABSL_BUILD_TESTING option (#1057)
  > Export of internal Abseil changes
  > Export of internal Abseil changes
  > Export of internal Abseil changes
  > Export of internal Abseil changes
  > Export of internal Abseil changes
  > Export of internal Abseil changes
  > Export of internal Abseil changes
  > Export of internal Abseil changes
  > Fix googletest URL in CMakeLists.txt (#1062)
  > Export of internal Abseil changes
  > Export of internal Abseil changes
  > Export of internal Abseil changes
  > Export of internal Abseil changes
  > Export of internal Abseil changes
  > Export of internal Abseil changes
  > Export of internal Abseil changes
  > Export of internal Abseil changes
  > Fix Randen and PCG on Big Endian platforms (#1031)
  > Export of internal Abseil changes

Closes #10630
2022-05-22 23:46:33 +03:00
Avi Kivity
4afe2a24b9 cql3: grammar: make 'relation' return an expression rather than append to a vector
The 'relation' production is self-contained, except for its interface
to the rest of the grammar where it appends to a vector of expressions
(that happens to represent a conjunction of relations). Make it stand-
alone by returning an expression, and move the responsibility for
appending to an expression vector to the whereClause production (later
we can make it build a conjunction expression rather than a vector
of expressions, paving the way for more boolean operators).

Closes #10623
2022-05-22 22:16:12 +03:00
Avi Kivity
a6b554409b storage_proxy: stop using deprecated std::not1 and std::bind1st in cas(), get_paxos_participants(), and create_write_response_handler_helper()
Use equivalent std::not_fn and std::bind_front instead.

Closes #10622
2022-05-22 22:13:12 +03:00
Nadav Har'El
ac393a62a1 cql-pytest: translate Cassandra's tests for relations
This is a translation of Cassandra's CQL unit test source file
validation/operations/SelectSingleColumnRelationTest.java into our
cql-pytest framework.

This test file includes 23 tests for various types of SELECT operations
which involve relations, a.k.a expressions (i.e., WHERE).

All 23 tests pass on Cassandra. 3 of the tests fail on Scylla
reproducing 2 already known Scylla issues and three minor
previously-unknown issues:

Previously known issues:

Refs #2962:  Collection column indexing
Refs #10358: Comparison with UNSET_VALUE should produce an error

Three new (and minor) issue:

Refs #10577: Is max-clustering-key-restrictions-per-query too low?
Refs #10631: Invalid IN restriction is reported as a '=' restriction
Refs #10632: Column name printed in a strange way in error message

NOTE: Scylla supports some expressions which Cassandra does not. In some
cases the Cassandra unit test had checks that certain constructs are not
allowed, and I had to comment out such checks when the expression *does*
work in Scylla. But of course, in such cases, it is not enough to comment
out a check - we also need to verify that Scylla's unique behavior
is the correct one. For that, we will have separate cql-pytest test for
those features - they won't be in the translated Cassandra unit tests
(of course). For example, in this test I had to comment out a check
that filtering on *non*-frozen UDTs is not allowed. In a separate
patch which I'm sending in parallel, I added a new test -
test_filter_UDT_restriction_nonfrozen - which will verify that what
Scylla does in that case is the correct behavior.

Signed-off-by: Nadav Har'El <nyh@scylladb.com>
2022-05-22 20:49:04 +03:00
Nadav Har'El
8af2c4aced test/cql-pytest: add test for filtering UDTs
Both Scylla and Cassandra support filtering on frozen UDTs, which are
compared using lexicographical order. This patch adds a test to verify
that the behavior here is the same - and indeed it is.

For *non*-frozen UDTs, Cassandra does not allow filtering on them (this
was decided in CASSANDRA-13247), but Scylla does. So we also add a test
on how non-frozen UDTs work - that passes on Scylla (and of course not
in Cassandra).

The two tests here - for frozen and non-frozen UDTs - are identical
(they just call the same function) - to ensure these two cases work the
same. This is important because we can't judge the correctness of the
non-frozen test by comparison to Cassandra - because it can't run there.

Signed-off-by: Nadav Har'El <nyh@scylladb.com>
2022-05-22 20:20:15 +03:00
Nadav Har'El
1c355d773a test/cql-pytest: tests for IN restrictions and filtering
Cassandra only allows IN restrictions on primary key columns.
With filtering (ALLOW FILTERING), this limitation makes little
sense, and it turns out that Scylla does not have limitation.
So this patch adds a test that we support such queries *correctly*
(we can't compare to Cassandra because it doesn't implement this).

Another test checks IN restrictions on an *indexed* column
(without ALLOW FILTERING). We could have implemented this - the
indexed column behaves like a partition key - but we didn't,
so this test xfails. It also fails on Cassandra because as mentioned
above, Cassandra didn't implement IN except for primary key columns.

Signed-off-by: Nadav Har'El <nyh@scylladb.com>
2022-05-22 20:20:15 +03:00
Nadav Har'El
bd71e6d962 test/cql-pytest: test more cases of overlapping restrictions
As noted by test_filtering.py::test_multiple_restrictions_on_same_column
Cassandra WHERE does not allow specifying two restrictions on the the
same column, but Scylla does allow it, and this test verifies that the
results are correct (conflicting restrictions would lead to no results,
but overlapping restrictions can return some results).

In this patch we add yet another example of multiple restrictions
on the same column that was seen in a Cassandra unit test - this time
one of the restrictions involves a IN. These patch helps confirm that
the expression evaluation is done correct (and, again, differently from
Cassandra - Cassandra results in an error in this case).

Signed-off-by: Nadav Har'El <nyh@scylladb.com>
2022-05-22 20:20:15 +03:00
Alejo Sanchez
3904d3b96e install-dependencies.sh: add python3-pytest-asyncio
Add package pytest-asyncio for async pytest support.

Signed-off-by: Alejo Sanchez <alejo.sanchez@scylladb.com>

Closes #10616

[avi: regenerate frozen toolchain]
2022-05-22 17:46:56 +03:00
Botond Dénes
f873806c7c Merge 'Fix use-after-free when queue reader is destroyed before its handle' from Benny Halevy
The handle must not point at this reader implementation after it's destroyed.

This fixes use-after-free when the queue_reader_v2
is destroyed first as repair_writer_impl::_queue_reader,
before repair_writer_impl::_mq is destroyed.

The issue was introduced in 39205917a8
in the definition of `repair_writer_impl`.

Fixes #10528

While at it, fix also an ignored exceptional future seen in the test:
`repair_additional_test.py::TestRepairAdditional::test_repair_kill_3`

Closes #10591

* github.com:scylladb/scylla:
  mutation_readers: queue_reader_v2: detach from handle when destroyed
  messaging_service: do_make_sink_source: handle failed source future
2022-05-19 21:51:41 +03:00
Raphael S. Carvalho
b120cacdd1 compaction_manager: Allow off-strategy to proceed in parallel to in-strategy compactions
Off-strategy works on maintenance sstable set using maintenance
scheduling group, whereas "in-strategy" works on main sstable set
and uses compaction group.

Today, it can happen that off-strategy has to wait for an "in-strategy"
maintenance compaction, e.g. cleanup, to complete before getting
a chance to run. But that's not desired behavior as off-strategy uses
maintenance group, and its candidates don't add to the backlog that
influences "in-strategy" bandwidth. Therefore, "in-strategy" and
off-strategy should be decoupled, with off-strategy having its own
semaphore for guaranteeing serialization across tables.

Signed-off-by: Raphael S. Carvalho <raphaelsc@scylladb.com>

Closes #10595
2022-05-19 17:37:11 +03:00
Takuya ASADA
b6003989f9 scylla_setup: stop using sudo -u, use user/group parameter on subprocess module
To run scylla-housekeeping we currently use "sudo -u scylla <cmd>" to switch
scylla user, but it fails on some environment.
Since recent version of Python 3 supports to switch user on subprocess module,
let's use python native way and drop sudo.

Fixes #10483

Closes #10538
2022-05-19 17:21:35 +03:00
Avi Kivity
fcf292b5d1 Merge 'Add range deserializer' from Benny Halevy
Currently, the idl-generated deserialization code, e.g. mutation_partition_view::rows() deserializes and returns a complete utils::chunked_vector<deletable_row_view> . And that could be arbitrarily long.

To consume it gently, we don't need the whole vector in advance, but rather we can consume it one element at a time (and in a nested way for cells in a row in the future).

Use `range_deserializer` to consume range tombstones and rows one item at a time.

We may consider in the future also gently iterating over cells
in a row and then dipping into collection cells that might also
contain a large number of items.

Fixes #10558

Closes #10566

* github.com:scylladb/scylla:
  ser: use vector_deserializer by default for all idl vectors
  mutation_partition_view: do_accept_gently: use the range based deserializers
  idl-compiler: generate *_range methods using vector_deserializer
  serializer_impl: add vector_deserializer
  test: frozen_mutation_test: test_writing_and_reading_gently: log detailed error
2022-05-19 17:21:35 +03:00
Avi Kivity
5285ccbb12 Merge 'Add prune ghost rows statement' from Piotr Sarna
This series is split from another, bigger RFC series which provides
manual remedies to deal with inconsistencies between the base table
and its views. This part deals with ghost rows by providing a statement
which fetches view rows from a given range, then reads its corresponding
rows from the base table (cl=ALL), and finally removes rows which were
not present in the base table at all, qualifying them as ghost rows.
Motivations for introducing such a statement:
 * in case of detected inconsistencies, it can be used to fix
   materialized views without recreating them from scratch, which can
   take days and generates lots of throughput
 * a tool which periodically scrubs a materialized view can be easily
   created on top of this statement, especially that it's possible
   to remove ghost rows from a user-defined view token range;

This series comes with a unit test.

The reason for digging up this series is because it's still possible to end up with ghost rows in certain rather improbable scenarios, and we lack a way of fixing them without rebuilding the whole view. For instance, in case of a failed synchronous update to a local view, the user will be notified that the query failed, but a ghost row can be created nonetheless. The pruning statement introduced in this series would allow healing the failure locally, without rebuilding the whole view.

Tests: unit(dev)

Closes #10426

* github.com:scylladb/scylla:
  docs: add a paragraph on PRUNE MATERIALIZED VIEW statement
  service,test: add a test case for error during pruning
  tests: add ghost row deletion test case
  cql3: enable ghost row deletion via CQL
  cql3: add a statement for deleting ghost rows
  cql3: convert is_json statement parameter to enum
  pager: add ghost row deleting pager
  db,view: add delete ghost rows visitor
2022-05-19 17:21:35 +03:00
Gleb Natapov
c2ef390a52 service: raft: move group0 write path into a separate file
Writing into the group0 raft group on a client side involves locking
the state machine, choosing a state id and checking for its presence
after operation completes. The code that does it resides now in the
migration manager since the currently it is the only user of group0. In
the near future we will have more client for group0 and they all will
have to have the same logic, so the patch moves it to a separate class
raft_group0_client that any future user of group0 can use to write
into it.

Message-Id: <YoYAJwdTdbX+iCUn@scylladb.com>
2022-05-19 17:21:35 +03:00
Avi Kivity
78eccd8763 Merge "Remove sstable_format_slector::sync()" from Pavel E
"
There's an explicit barrier in main that waits for the sstable format
selector to finish selecting it by the time node start to join a cluter.
(Actually -- not quite, when restarting a normal node it joins cluster
in prepare_to_join()).

This explicit barrier is not needed, the sync point already exists in
the way features are enabled, the format-selector just needs to use it.

branch: https://github.com/xemul/scylla/tree/br-format-selector-sync
tests: https://jenkins.scylladb.com/job/releng/job/Scylla-CI/351/
refs: #2795
"

* 'br-format-selector-sync' of https://github.com/xemul/scylla:
  format-selector: Remove .sync() point
  format-selector: Coroutinize maybe_select_format()
  format-selector: Coroutinize simple methods
2022-05-19 17:21:35 +03:00
Avi Kivity
08ed4d7405 Merge 'scylla-gdb.py: add commands to dump sstables summary and index-cache' from Botond Dénes
This series adds two commands:
* scylla sstable-summary
* scylla sstable-index-cache

The former dumps the content of the sstable summary. This component is kept in memory in its entirety, so this can be easily done. The latter command dumps the content of the sstable index cache. This contains all the index-pages that are currently cached. The promoted index is not dumped yet and there is no indication of whether a given entry is in the LRU or not, but this already allows at seeing what pages are in the cache and what aren't.

Closes #10546

* github.com:scylladb/scylla:
  scylla-gdb.py: add scylla sstable-index-cache command
  scylla-gdb.py: add scylla sstable-summary command
  test/scylla-gdb: add sstable fixture
  scylla-gdb.py: make chunked_vector a proper container wrapper"
  scylla-gdb.py: make small_vector a proper container wrapper"
  scylla-gdb.py: add sstring container wrapper
  scylla-gdb.py: add chunked_managed_vector container wrapper
  scylla-gdb.py: add managed_vector container wrapper
  scylla-gdb.py: std_variant: add workaround for clang template bug
  scylla-gdb.py: add bplus_tree container wrapper
2022-05-19 17:21:35 +03:00
Pavel Emelyanov
3e53a0965c scylla-gdb: Handle new seastar/fair_queue layout
Signed-off-by: Pavel Emelyanov <xemul@scylladb.com>
Message-Id: <20220519094452.14538-1-xemul@scylladb.com>
2022-05-19 13:29:03 +03:00
Benny Halevy
9c5feb2781 mutation_readers: queue_reader_v2: detach from handle when destroyed
The handle must not point at this reader implementation
after it's destroyed.

This fixes use-after-free when the queue_reader_v2
is destroyed first as repair_writer_impl::_queue_reader,
before repair_writer_impl::_mq is destroyed.

The issue was introduced in 39205917a8
in the definition of `repair_writer_impl`.

Fixes #10528

Signed-off-by: Benny Halevy <bhalevy@scylladb.com>
2022-05-19 11:48:03 +03:00
Benny Halevy
1308b45c58 messaging_service: do_make_sink_source: handle failed source future
I've stumbled upon this with version a2901a376d in debug mode
when testing repair_additional_test.py::TestRepairAdditional::test_repair_kill_3:

WARN  2022-05-17 07:26:12,581 [shard 0] seastar - Exceptional future ignored: seastar::rpc::closed_error (connection is closed), backtrace: 0x137c33d0 0x1ad14d0d 0x1ad149cd 0x1ad16fc3 0x1ad17e52 0x19d8a809 0x19d8ab6a 0x139165a9 0x17be0d21 0x17bdcfb0 0x17bf3611 0x17bf39f0 0x17bf3c62 0x17bf3958 0x17bf57d8 0x17bf5468 0x19efe44e 0x19f04ac6 0x19f09732 0x19f072a1 0x19cca281 0x19cc7de5 0x13859cbf 0x13d309d6 0x13d3090b 0x13d30775 0x1391364d 0x13858521 /lib64/libc.so.6+0x27b74 0x137774ad

Decode:
```
seastar::report_failed_future(seastar::future_state_base::any&&) at //./seastar/src/core/future.cc:218
seastar::future_state_base::any::check_failure() at //./seastar/include/seastar/core/future.hh:573
seastar::future_state<seastar::rpc::source<repair_row_on_wire_with_cmd> >::clear() at ././seastar/include/seastar/core/future.hh:615
~future_state at ././seastar/include/seastar/core/future.hh:620
 (inlined by) ~future at ././seastar/include/seastar/core/future.hh:1343
~ at ./message/messaging_service.cc:841
```

Looks like if sink.close() fails after source.failed() then source gets abandoned.

Signed-off-by: Benny Halevy <bhalevy@scylladb.com>
2022-05-19 11:47:38 +03:00
Avi Kivity
5c481973a3 sstables/processing_result_generator.hh: refine check for coroutine standard
We have a check for whether we can use standard coroutines (in namespace
std) or the technical specification (in std::experimental), but it doesn't
work since Clang doesn't report the correct standard version. Use a
compiler versionspecific check, inspired by Seastar's check.

This allows building with clang 14.

Closes #10603
2022-05-19 11:31:40 +03:00
Nadav Har'El
0040e9e7f4 Merge 'cql: Add proper validation for null and unset inside collections send as bound values' from Jan Ciołek
Let's say we have a query like:
```cql
INSERT INTO ks.t (list_column) VALUES (?);
```
And the driver sends a list with null inside as the bound value, something like `[1, 2, null, 4]`.

In such case we should throw `invalid_request_exception` because `nulls` are not allowed inside collections.

Currently when a query like this gets executed Scylla throws an ugly marshalling error.
This is because the validation code reads size of the next element, interprets it as an unsigned integer and tries to read this much.
In case of `null` element the size is `-1`, which when converted to unsigned `size_t` gives 18446744073709551615 and it fails to read this much.

This PR adds proper validation checks to make the error message better.

I also added some tests.
I originally tried to write them in python, but python driver really doesn't like sending invalid values.
Trying to send `[1, None, 2]` results in a list with empty value instead of null.
Trying to send `[1, UNSET_VALUE, 2]` Fails before query even leaves the driver.

Fixes #10580

Closes #10599

* github.com:scylladb/scylla:
  cql3: Add tests for null and unset inside collections
  cql3: Add null and unset checks in collection validation
2022-05-19 11:25:24 +03:00
Piotr Sarna
e54a4ebdcb docs: add a paragraph on PRUNE MATERIALIZED VIEW statement 2022-05-19 10:16:04 +02:00
Piotr Sarna
b8a36ff253 service,test: add a test case for error during pruning
The test case checks that errors which occur during materialized
view pruning are properly propagated back to the user.
2022-05-19 10:16:04 +02:00
Piotr Sarna
995468520e tests: add ghost row deletion test case
The tests checks if manually injected ghost rows are properly deleted
by the ghost row delete statement - and, that non-ghost regular rows
are left intact.
2022-05-19 10:16:03 +02:00
Piotr Sarna
be2ef862bd cql3: enable ghost row deletion via CQL
This commit allows accepting a CQL request to clear ghost rows
from a given view partition range. Currently its syntax is a purposely
convoluted mix of existing keywords, which makes sure that the statement
is never issued by mistake. Example runs:

 -- try deleting all ghost rows, effectively performs a paged full scan
PRUNE MATERIALIZED VIEW my_mv;

 -- try deleting ghost rows from a single view partition
PRUNE MATERIALIZED VIEW my_mv WHERE mv_pk = 3;

 -- try deleting ghost rows from a token range (effective full scans)
PRUNE MATERIALIZED VIEW my_mv WHERE TOKEN(mv_pk) > 7 AND TOKEN(mv_pk) < 42
2022-05-19 10:11:50 +02:00
Piotr Sarna
ec0a3bbbd4 cql3: add a statement for deleting ghost rows
In order to expose the API for deleting ghost rows from a view,
a CQL statement is created. It is loosely based on select_statement,
as its first step is to select view table rows.
2022-05-19 10:11:50 +02:00