Commit Graph

30453 Commits

Author SHA1 Message Date
Benny Halevy
5e1fda7e1d compaction_manager: use coroutine::switch_to
Saving an allocation for running the functor
as a task in the switched-to scheduling group.

Also, switch to the desired scheduling group at
the beginning of the task so that the higher level logic,
like getting the list of sstables to compact
will be performed under the desired scheduling group,
not only the compaction code itself.

Signed-off-by: Benny Halevy <bhalevy@scylladb.com>
2022-03-10 12:20:01 +02:00
Benny Halevy
8c66916652 compaction_manager::task: drop _compaction_running
Replace the _compaction_running boolean member
by calculating _state == state::active
now that setup_new_compaction switches state to
`active`

Signed-off-by: Benny Halevy <bhalevy@scylladb.com>
2022-03-10 12:20:01 +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
Benny Halevy
2e6ce43a97 compaction_manager: task: add state enum
Add an enum class representing the task state machine
and a switch_state function to transition between the states
and update the corresponding compaction_manager stats counters.

Refs #9974

Signed-off-by: Benny Halevy <bhalevy@scylladb.com>
2022-03-10 12:19:59 +02:00
Benny Halevy
9c59d66b7e compaction_manager: task: add maybe_retry
Replacing and combining compaction_manager methods:
maybe_stop_on_error and put_task_to_sleep.

Signed-off-by: Benny Halevy <bhalevy@scylladb.com>
2022-03-10 11:35:37 +02:00
Benny Halevy
ee32be3aa5 compaction_manager: reevaluate_postponed_compactions: mark as noexcept
To simplify error handling in following patches
that will coroutinize task logic.

Signed-off-by: Benny Halevy <bhalevy@scylladb.com>
2022-03-10 11:35:37 +02:00
Benny Halevy
72162ed653 compaction_manager: define derived task types
Turn task into a class, defining a clear hierarchy
of private, protected, and public methods.

Signed-off-by: Benny Halevy <bhalevy@scylladb.com>
2022-03-10 11:35:35 +02:00
Benny Halevy
37694422dc compaction_manager: register_metrics: expose postponed_compactions
Provide a metric counting the number of tables
with postponed compaction.

Signed-off-by: Benny Halevy <bhalevy@scylladb.com>
2022-03-10 08:39:18 +02:00
Benny Halevy
089d4442d8 compaction_manager: register_metrics: expose failed_compactions
Signed-off-by: Benny Halevy <bhalevy@scylladb.com>
2022-03-10 08:39:18 +02:00
Benny Halevy
8081f951d0 compaction_manager: register_metrics: expose _stats.completed_tasks
Signed-off-by: Benny Halevy <bhalevy@scylladb.com>
2022-03-10 08:39:18 +02:00
Benny Halevy
ffc314d506 compaction: add documentation for compaction_type to string conversions
Signed-off-by: Benny Halevy <bhalevy@scylladb.com>
2022-03-10 08:39:18 +02:00
Benny Halevy
28a74a2e90 compaction: expose to_string(compaction_type)
To be used in the next patch to generate
a string dscription from the compaction_type.

In theory, we could use compaction_name()
btu the latter returns the compaction type
in all-upper case and that is very different from
what we print to the log today.  The all-upper
strings are used for the api layer, e.g. to
stop tasks of a particular compaction type.

Signed-off-by: Benny Halevy <bhalevy@scylladb.com>
2022-03-10 08:39:18 +02:00
Benny Halevy
20a8609392 compaction_manager: task: standardize task description in log messages
Define task::describe and use it via operator<<
to print the task metadata to the log in a standard way.

Signed-off-by: Benny Halevy <bhalevy@scylladb.com>
2022-03-10 08:39:18 +02:00
Benny Halevy
59863b317f compaction_manager: refactor can_proceed
Move the task-internal parts of can_proceed
to a respective compaction_manager::task method,
preparing for turning it into a class with
a proper hierarchy of access to private members.

Signed-off-by: Benny Halevy <bhalevy@scylladb.com>
2022-03-10 08:39:17 +02:00
Benny Halevy
33b2731a4a compaction_manager: pass compaction_manager& to task ctor
And use it to get the compaction state of the
table to compact.

It will be used in a later patch
to manage the task state from task
methods.

Signed-off-by: Benny Halevy <bhalevy@scylladb.com>
2022-03-10 08:39:17 +02:00
Benny Halevy
20067b1050 compaction_manager: use shared_ptr<task> rather than lw_shared_ptr
Prepare for defining per compaction type tasks
derived from compaction_manager::task.

Signed-off-by: Benny Halevy <bhalevy@scylladb.com>
2022-03-10 08:39:17 +02:00
Benny Halevy
cb2403e917 compaction_manager: rewrite_sstables: acquire _maintenance_ops_sem once
Like all other maintenance operations, acquire the _maintenance_ops_sem
once for the whole task, rather than for each sstable.

Signed-off-by: Benny Halevy <bhalevy@scylladb.com>
2022-03-10 08:39:17 +02:00
Benny Halevy
d0f693a517 compaction_manager: use compaction_state::lock only to synchronize major and regular compaction
Maintenance operations like cleanup, upgrade, reshape, and reshard
are serialized serialized with major compaction using the _maintenance_ops_sem
and they need no further synchronization with regular compaction
by acquiring the per-table read lock..

Signed-off-by: Benny Halevy <bhalevy@scylladb.com>
2022-03-10 08:39:17 +02:00
Avi Kivity
a4756334ce Merge "tools/scylla-types: improve documentation" from Botond
"
Add per-action help content for each action. Main description now points
to these for more details.
"

* 'scylla-types-improvements/v1' of https://github.com/denesb/scylla:
  tools/types: update main description
  tools/scylla-types: per-action help content
  tools/scylla-types: description: remove -- from action listing
  tools/scylla-types: use fmt::print() instead of std::cout <<
2022-03-09 19:37:00 +02:00
Avi Kivity
e1c326a5ba Merge "Convert multishard writer to v2" from Botond
"
Also convert the foreign_reader used by it in the process.

Tests: unit(dev)
"

* 'multishard-writer-v2/v1' of https://github.com/denesb/scylla:
  mutation_writer/multishard_writer: remove now unused v1 factory overloads
  test/boost/mutation_writer_test: test the v2 variant of distribute_reader_and_consume_on_shards()
  flat_mutation_reader: add v2 variant of make_generating_reader()
  mutation_reader: multishard_writer: migrate implementation to v2
  mutation_reader: convert foreign_reader to v2
  streaming/consumer: convert to v2
  mutation_writer/multishard_writer: add v2 variant of distribute_reader_and_consume_on_shards()
2022-03-09 19:28:05 +02:00
Avi Kivity
fbb3904b67 Merge 'query: transform asserts into on_internal_error in forward_result::merge' from Michał Sala
It was a suggestion from @psarna, done to get more info about the abort from #10174.

Closes #10185

* github.com:scylladb/scylla:
  query: do not assert in `operator<<(ostream&, const forward_result::printer&)`
  query: transform asserts into on_internal_error in forward_result::merge
2022-03-09 16:30:32 +02:00
Tomasz Grabiec
8fa704972f loading_cache: Make invalidation take immediate effect
There are two issues with current implementation of remove/remove_if:

  1) If it happens concurrently with get_ptr(), the latter may still
  populate the cache using value obtained from before remove() was
  called. remove() is used to invalidate caches, e.g. the prepared
  statements cache, and the expected semantic is that values
  calculated from before remove() should not be present in the cache
  after invalidation.

  2) As long as there is any active pointer to the cached value
  (obtained by get_ptr()), the old value from before remove() will be
  still accessible and returned by get_ptr(). This can make remove()
  have no effect indefinitely if there is persistent use of the cache.

One of the user-perceived effects of this bug is that some prepared
statements may not get invalidated after a schema change and still use
the old schema (until next invalidation). If the schema change was
modifying UDT, this can cause statement execution failures. CQL
coordinator will try to interpret bound values using old set of
fields. If the driver uses the new schema, the coordinaotr will fail
to process the value with the following exception:

  User Defined Type value contained too many fields (expected 5, got 6)

The patch fixes the problem by making remove()/remove_if() erase old
entries from _loading_values immediately.

The predicate-based remove_if() variant has to also invalidate values
which are concurrently loading to be safe. The predicate cannot be
avaluated on values which are not ready. This may invalidate some
values unnecessarily, but I think it's fine.

Fixes #10117

Message-Id: <20220309135902.261734-1-tgrabiec@scylladb.com>
2022-03-09 16:13:07 +02:00
Michał Sala
538cff651e query: do not assert in operator<<(ostream&, const forward_result::printer&)
Printing invalid forward_result should not cause Scylla to stop.
2022-03-09 14:58:11 +01:00
Michał Sala
51362e4e5e query: transform asserts into on_internal_error in forward_result::merge
It was done to show more context in case of forward_result::merge
arguments size mismatch and also to prevent aborts caused by another
nodes sending malformed data.
2022-03-09 14:58:11 +01:00
Nadav Har'El
397dd64dea test/cql-pytest: avoid "run" warnings caused by pytest bug
This patch gets rid annoying pytest configuration warnings when running
test/cql-pytest/run. These started to happen after commit
afab1a97c6, due to a pytest bug:

In that commit, we added new "--scylla-path" and "--workdir" parameters
to our pytest tests, and test/cql-pytest/run started passing them,
and test/cql-pytest/run sometest runs pytest as:

    pytest --host something --workdir somedir --scylla-path somepath sometest

Pytest wants to find a configuration file (pytest.ini or tox.ini) in the
directory where the tests live, but its logic to find that directory is
buggy: It (_pytest/config/findpaths.py::determine_setup()) looks at
the command line for directory names, and looks for config files in
these directories or any of their parents. It ignores parameters
beginning with "-", but in our case the various arguments like
"--scylla-path" are each followed by another option, and this one is
not ignored! So instead of looking for the config file in sometest's
parent directories (and finding test/cql-pytest/pytest.ini), pytest
sees the directory given after "scylla-path", and finds the completely
irrelevant tox.ini there - and uses that, which (depending what you
have installed) can generate warnings.

The solution is to change the run script to use "--scylla-path=..."
as one parameter instead of "--scylla-path ..." as two parameters.
When it's just one parameter, the pytest determine_setup() logic skips
it entirely, and finds just the actual test directory.

Signed-off-by: Nadav Har'El <nyh@scylladb.com>
Message-Id: <20220309132726.2311721-1-nyh@scylladb.com>
2022-03-09 15:37:08 +02:00
Nadav Har'El
733672fc54 Merge 'types: fix is_string for reversed types' from Piotr Sarna
Checking if the type is string is subtly broken for reversed types,
and these types will not be recognized as strings, even though they are.
As a result, if somebody creates a column with DESC order and then
tries to use operator LIKE on it, it will fail because the type
would not be recognized as a string.

Fixes #10183

Closes #10181

* github.com:scylladb/scylla:
  test: add a case for LIKE operator on a descending order column
  types: fix is_string for reversed types
2022-03-09 10:04:42 +02:00
Piotr Sarna
05b66102e9 test: add a case for LIKE operator on a descending order column
This case is a regression test for issue #10181, where it turned out
that a clustering column with descending order is not properly
recognized as a string.
This test case used to fail with:
cassandra.InvalidRequest:
  Error from server: code=2200 [Invalid query]
  message="LIKE is allowed only on string types, which b is not"
...until it got fixed by the previous commit.
2022-03-09 08:56:22 +01:00
Piotr Sarna
0a068cddb1 types: fix is_string for reversed types
Checking if the type is string is subtly broken for reversed types,
and these types will not be recognized as strings, even though they are.
As a result, if somebody creates a column with DESC order and then
tries to use operator LIKE on it, it will fail because the type
would not be recognized as a string.
2022-03-09 08:18:33 +01:00
Benny Halevy
11ea2ffc3c compaction_manager: rewrite_sstables: do not acquire table write lock
Since regular compaction may run in parallel no lock
is required per-table.

We still acquire a read lock in this patch, for backporting
purposes, in case the branch doesn't contain
6737c88045.
But it can be removed entirely in master in a follow-up patch.

This should solve some of the slowness in cleanup compaction (and
likely in upgrade sstables seen in #10060, and
possibly #10166.

Fixes #10175

Signed-off-by: Benny Halevy <bhalevy@scylladb.com>

Closes #10177
2022-03-09 09:13:46 +02:00
Nadav Har'El
c8152e78d7 Merge 'CQL3: fromJson accepts string as bool' from Jadw1
The problem was incompatibility with cassandra, which accepts bool
as a string in `fromJson()` UDF. The difference between Cassandra and
Scylla now is Scylla accepts whitespaces around word in string,
Cassandra don't. Both are case insensitive.

Fixes: https://github.com/scylladb/scylla/issues/7915

Closes #10134

* github.com:scylladb/scylla:
  CQL3/pytest: Updating test_json
  CQL3: fromJson accepts string as bool
2022-03-08 16:27:40 +02:00
Avi Kivity
1622995900 Merge 'Allow empty partition keys in views' from Nadav Har'El
Cassandra generally does not allow empty strings as partition keys (note, by the way, that empty strings are allowed as clustering keys, as well as in individual components of a compound partition key).
However, Cassandra does allow empty strings in _regular_ columns - and those regular columns can be indexed by a secondary index, or become an empty partition-key column in a materialized view. As noted in issues #9375 and #9364 and verified in a few xfailing cql-pytest tests, Scylla didn't allow these cases - and this patch series fixes that.

Before the last patch in this series finally enables empty-string partition keys in materialized views, we first need to solve a couple of bugs in our code related to handling empty partition keys:

The first patch fixes issue #10178 - a bug in `key_view::tri_compare()`  where comparing two empty keys returned a random result instead of "equal".

The second patch fixes issue #9352: our tokenizer has an inconsistency where for an empty string key, two variants of the same function return different results:

1. One variant `murmur3_partitioner::get_token(bytes_view key)` returned `minimum_token()` for the empty string.
2. Another variant `murmur3_partitioner::get_token(const schema& s, partition_key_view key)` did not have this special case, and called the normal hash-function calculation on the empty string (the resulting token is 0).

Variant 2 was an unintentional bug, because Cassandra always does what variant does 1. So the "obvious" fix here would be to fix variant 2 to do what variant 1 does. Nevertheless, we decided to do the opposite: Change variant 1 to match variant 2. The reasoning is as follows:

The `minimum_token()` is `token{token::kind::before_all_keys, 0 }` - it's not a real token. Since we intend in this patch allow real data to exist with the empty key, we need this real data to have a real token. For example, this token needs to be located on the token ring (so the empty-key partition will have replicas) and also belong to one of the shards, and it's not clear that `minimum_token()` will be handled correctly in this context.

After changing the token of the empty string to 0, we note that some places in the code assume that `dht::decorated_key(dh
t::minimum_token(), partition_key::make_empty())` is a legal decorated key. However, as far as I can tell, none of these places actually assume that the partition-key part (the `make_empty()`) really matches the token - this decorated key is only used to start an iteration (ignoring this key itself) or to indicate a non-existent key (in modern code `std::optional` should be used for that).

While normally changing the token of a key is a big faux-pas, which can result in old data no longer being readable, in this case this change is safe because:

1. Scylla previously disallowed empty partition keys (in both base tables and views), so we cannot have had such a partition key saved in any sstable.
3. Cassandra does allow empty partition keys in _views_ and _secondary indexes_, but we do not support migrating sstables of those into Scylla - users are expected to only migrate the base table and then re-create the view or index. So however Cassandra writes those empty-key partitions, we don't care.

The third patch finally fixes the materialized views implementation to not drop view rows with an empty-string partition key (#9375). This means we basically revert commit ec8960df45 - which fixed #3262 by disallowing empty partition keys in views, whereas this patch fixes the same problem by handling the empty partition keys correctly.

The fix for the secondary index bug (#9364) comes "for free" because it is based on materialized views.

We already had xfailing test cases for empty strings in materialized views and indexes, and after this series they begin to pass so the "xfail" mark is removed. The series also adds additional test cases that validate additional corner cases discovered during the debugging.

Fixes #9352
Fixes #9364
Fixes #9375
Fixes #10178

Closes #10170

* github.com:scylladb/scylla:
  compound_compat.hh: add missing methods of iterator
  materialized views: allow empty strings in views and indexes
  murmur3: fix inconsistent token for empty partition key
  compound_compat.hh: fix bug iterating on empty singular key
2022-03-08 15:55:55 +02:00
Nadav Har'El
674d3a5a84 compound_compat.hh: add missing methods of iterator
While debugging legacy_compound_view, I noticed that it cannot be used
as a C++20 std::ranges::input_range because it is missing some trivial
methods. So let's fix this, and make the life of future developers a
little bit easier.

The two trivial methods we need to implement:

1. A postfix increment operator. We already had a prefix increment
   operator, but the C++20 concept weakly_iterable also needs postfix.

2. By mistake (this will be corrected in https://wg21.link/P2325R3),
   weakly_iterable also required the default_initialized concept, so
   our iterator type also needs a default constructor.
   We'll never actually use this silly constructor, and when this C++20
   standard mistake is corrected, we can remove this constructor.

After this patch, a legacy_compound_view is accepted for the C++20
ranges::input_range concept.

Signed-off-by: Nadav Har'El <nyh@scylladb.com>
2022-03-08 15:37:03 +02:00
Nadav Har'El
ef43531fb6 materialized views: allow empty strings in views and indexes
Although Cassandra generally does not allow empty strings as partition
keys (note they are allowed as clustering keys!), it *does* allow empty
strings in regular columns to be indexed by a secondary index, or to
become an empty partition-key column in a materialized view. As noted in
issues #9375 and #9364 and verified in a few xfailing cql-pytest tests,
Scylla didn't allow these cases - and this patch fixes that.

The patch mostly *removes* unnecessary code: In one place, code
prevented an sstable with an empty partition key from being written.
Another piece of removed code was a function is_partition_key_empty()
which the materialized-view code used to check whether the view's
row will end up with an empty partition key, which was supposedly
forbidden. But in fact, should have been allowed like they are allowed
in Cassandra and required for the secondary-index implementation, and
the entire function wasn't necessary.

Note that the removed function is_partition_key_empty() was *NOT* required
for the "IS NOT NULL" feature of materialized views - this continues to
work as expected after this patch, and we add another test to confirm it.
Being null and being an empty string are two different things.

This patch also removes a part of a unit test which enshrined the
wrong behavior.

After this patch we are left with one interesting difference from
Cassandra: Though Cassandra allows a user to create a view row with an
empty-string partition key, and this row is fully visible in when
scanning the view, this row can *not* be queried individually because
"WHERE v=''" is forbidden when v is the partition key (of the view).
Scylla does not reproduce this anomaly - and such point query does work
in Scylla after this patch. We add a new test to check this case, and mark
it "cassandra_bug", i.e., it's a Cassandra behavior which we consider
wrong and don't want to emulate.

This patch relies on #9352 and #10178 having been fixed in previous patches,
otherwise the WHERE v='' does not work when reading from sstables.
We add to the already existing tests we had for empty materialized-views
keys a lookup with WHERE v='' which failed before fixing those two issues.

Fixes #9364
Fixes #9375

Signed-off-by: Nadav Har'El <nyh@scylladb.com>
2022-03-08 15:34:26 +02:00
Nadav Har'El
bc4d0fd5ad murmur3: fix inconsistent token for empty partition key
Traditionally in Scylla and in Cassandra, an empty partition key is mapped
to minimum_token() instead of the empty key's usual hash function (0).
The reasons for this are unknown (to me), but one possibility is that
having one known key that maps to the minimal token is useful for
various iterations.

In murmur3_partitioner.cc we have two variants of the token calculation
function - the first is get_token(bytes_view) and the second is
get_token(schema, partition_key_view). The first includes that empty-
key special case, but the second was missing this special case!

As Kamil first noted in #9352, the second variant is used when looking
up partitions in the index file - so if a partition with an empty-string
key is saved under one token, it will be looked up under a different
token and not found. I reproduced exactly this problem when fixing
issues #9364 and #9375 (empty-string keys in materialized views and
indexes) - where a partition with an empty key was visible in a
full-table scan but couldn't be found by looking up its key because of
the wrong index lookup.

I also tried an alternative fix - changing both implementations to return
minimum_token (and not 0) for the empty key. But this is undesirable -
minimum_token is not supposed to be a valid token, so the tokenizer and
sharder may not return a valid replica or shard for it, so we shouldn't
store data under such token. We also have have code (such as an increasing-
key sanity check in the flat mutation reader) which assumes that
no real key in the data can be minimum_token, and our plan is to start
allowing data with an empty key (at least for materialized views).

This patch does not risk a backward-incompatible disk format changes
for two reasons:

1. In the current Scylla, there was no valid case where an empty partition
   key may appear. CQL and Thrift forbid such keys, and materialized-views
   and indexes also (incorrectly - see #9364, #9375) drop such rows.
2. Although Cassandra *does* allow empty partition keys, they is only
   allowed in materialized views and indexes - and we don't support reading
   materialized views generated by Cassandra (the user must re-generate
   them in Scylla).

When #9364 and #9375 will be fixed by the next patch, empty partition keys
will start appearing in Scylla (in materialized views and in the
materialized view backing a secondary index), and this fix will become
important.

Fixes #9352
Refs #9364
Refs #9375

Signed-off-by: Nadav Har'El <nyh@scylladb.com>
2022-03-08 14:15:03 +02:00
Nadav Har'El
f8807e24f4 compound_compat.hh: fix bug iterating on empty singular key
When iterating over a compound key with legacy_compound_view<>, when the
key is "singular" (i.e., a single column) we need to iterate over just the
component's actual bytes - without the two length bytes or end-of-component
byte. In particular, when the component is an *empty string*, the iteration
should return zero bytes. In other words, we should have begin() == end().

Unfortunately, this is not what happened - for an empty singular key, the
iterator returned for begin() was slightly different from end() - so
code using this iterator would not know there is nothing to iterate.
So in this patch we fix begin() and end() to return the same thing
if we have an empty singular key.

The bug in legacy_compound_view<> (which we fix here) caused a bug in
sstables::key_view::tri_compare(const schema& s, partition_key_view other),
causing it to return wrong results when comparing two empty keys. As a
result we were unable to retrieve a partition with an empty key from the
sstable index. So this patch is necessary to fix support for
empty-string keys in sstables (part of issue #9375).

This patch also includes a unit-test for this bug. We test it in the
context of sstables::key_view::tri_compare(), where it was first
discovered, and also test the legacy_compound_view itself. The included
test used to fail in both places before this patch, and pass after it.

Fixes #10178
Refs #9375

Signed-off-by: Nadav Har'El <nyh@scylladb.com>
2022-03-08 14:14:18 +02:00
Botond Dénes
f2060ac03b Merge "Remove sub-mode booleans from storage service" from Pavel Emelyanov
"
There's a _operation_mode enum sitting on storage_service that indicates the
top-level state of the scylla node. Next to it there's a bunch of booleans
that define (and duplicate) some sub-modes. These booleans just make the code
more obscure and complicated. This set removes all those booleans and patches
all the relevant checks/calls/methods to rely only on the operation mode.
Also, the switching between modes is simplified down to some bare minimum.

tests: unit(dev) dtest.simple_boot_shutdown(dev) manual(dev)

Manual test included start-stop, nodetool enablegossip, disablegosip and drain
commands, scylla-cly is_initialized and is_joined calls

As noticed in v2, this set changes the log messages that are checked by
dtests. The fix for dtest, that's compatible with both -- current scylla and
this patchset -- is already in dtest master.
"

* 'br-remove-bools-from-storage-service-3-rebase' of https://github.com/xemul/scylla:
  storage_service: Relax operation modes switch
  storage_service: Remove _ms_stopped
  storage_service: Remove _is_bootstrap_mode
  storage_service: Remove _initialized and is_initialized()
  storage_service: Remove _joined and is_joined()
  storage_service: Replace is_starting() with get_operation_mode()
  storage_service: Make get_operation_mode() return mode itself
  storage_service: Relax repeating set_mode-s
2022-03-07 15:27:03 +02:00
Avi Kivity
e2f3e9791b Update seastar submodule
* seastar 1d81c8e5aa...4e42a60199 (11):
  > condition_variable: Add coroutine-only "when" operation folding waiter into parent frame
  > condition_variable: Add simple test
  > condition_variable: Make std::chrono timeout operations templated
  > condition_variable: Remove semaphore usage, keep internal wait queue
  > condition_variable: Add concept checks to predicated wait methods
  > io_queue: Don't let preemption overlap requests
  > io_queue: Pending needs to keep capacity instead of ticket
  > io_queue: Extend grab_capacity() return codes
  > scattered_message: allow appending temporary buffers directly
  > util: file: include reactor.hh
  > tests: coroutines_test: Check scheduling group set with switch_to() is inherited and restored
2022-03-07 14:30:52 +02:00
Avi Kivity
8ab20bae68 Merge 'prepared_statements: Invalidate batch statement too' from Eliran Sinvani
It seams that batch prepared statements always return false for
depends_on_keyspace and depends_on_column_family, this in turn
renders the removal criteria from the cache to always be false
which result by the queries not being evicted.
Here we change the functions to return the true state meaning,
they will return true if any of the sub queries is dependant upon
the keyspace or column family.

In this fix we first make the API more coherent and then use this new API to implement
the batch statement's dependency test.
Fixes #10129

Signed-off-by: Eliran Sinvani <eliransin@scylladb.com>

Closes #10132

* github.com:scylladb/scylla:
  prepared_statements: Invalidate batch statement too
  cql3 statements: Change dependency test API to express better it's purpose
2022-03-07 14:00:05 +02:00
Pavel Emelyanov
190385551c storage_service: Relax operation modes switch
The set_mode() tries to combine mode switching and extended logging,
but there are no places left that do need this flexibility. It's
simpler and nicer to make set_mode() _just_ switch the mode and
log some generic "entering ... mode" message.

Signed-off-by: Pavel Emelyanov <xemul@scylladb.com>
2022-03-07 13:29:47 +03:00
Pavel Emelyanov
0941098b39 storage_service: Remove _ms_stopped
This boolean protects do_stop_ms from re-entrability. However, this
method is only called from stop_transport() which handles re-entring
itself, so the _ms_stopped can be just removed.

Signed-off-by: Pavel Emelyanov <xemul@scylladb.com>
2022-03-07 13:29:47 +03:00
Pavel Emelyanov
74212286f8 storage_service: Remove _is_bootstrap_mode
This "state" is the sub-state of the STARTING mode that's activated
when the storage_service::bootstrap() is called. Instead of the
separate boolean the new mode can be used. To stop it from reverting
the BOOTSTRAP mode back to JOINING some calls to set_mode() should
be converted into regular logging.

Signed-off-by: Pavel Emelyanov <xemul@scylladb.com>
2022-03-07 13:29:47 +03:00
Pavel Emelyanov
dbaca825ec storage_service: Remove _initialized and is_initialized()
This bit is hairy. First, it indicates that the storage service
entered the init_server() method. But, once the node is up and
running it also indicates whether the gossiper is enabled or not
via the APi call.

To rely on the operation mode, first, the NONE mode is introduced
at which the server starts. Then in init_server() is switches to
STARTING.

Second change is to stop using the bit in enable/disable gossiper
API call, instead -- check the gossiper.is_enabled() itself.

To keep the is_initialized API call compatible, when the operation
mode is NORMAL it would return true/false according to the status
of the gossiper. This change is simple because storage service API
handlers already have the gossiper instance hanging around.

Signed-off-by: Pavel Emelyanov <xemul@scylladb.com>
2022-03-07 13:29:47 +03:00
Pavel Emelyanov
ffbfa3b542 storage_service: Remove _joined and is_joined()
The is_joined() status can be get with get_operation_mode(). Since
it indicates that the operation mode is JOINING, NORMAL or anything
above, the operation mode the enum class should be shuffled to get
the simple >= comparison.

Another needed change is to set mode few steps earlier than it
happens now to cover the non-bootstrap startup case.

And the third change is to partially revert the d49aa7ab that made
the .is_joined() method be future-less. Nowadays the is_joined() is
called only from the API which is happy with being future-full in
all other storage service state checks.

Signed-off-by: Pavel Emelyanov <xemul@scylladb.com>
2022-03-07 13:29:47 +03:00
Pavel Emelyanov
ca03fd3145 storage_service: Replace is_starting() with get_operation_mode()
This is trivial change, since the only user is in API and the
get_operation_mode + mode values are at hand.

One thing to pay attention to -- the new method checks the mode to
be <= STARTING, not for equality. Now this is equivalent change,
but next patch will introduce NONE mode that should be reported
as is_starting() too.

Signed-off-by: Pavel Emelyanov <xemul@scylladb.com>
2022-03-07 13:29:47 +03:00
Pavel Emelyanov
c385fe7d79 storage_service: Make get_operation_mode() return mode itself
Now it reports back formatted mode. For future convenience it's
needed to return the raw value, all the more so the mode enum class
is already public.

Signed-off-by: Pavel Emelyanov <xemul@scylladb.com>
2022-03-07 13:29:47 +03:00
Pavel Emelyanov
968b07052d storage_service: Relax repeating set_mode-s
In several places the call to set_mode(...) is used as a (format-less)
replecement for regular logging. Mode doesn't really change there, because
it had been changed before. Patch all those places to use regular logging,
next patches will make full use of it.

Signed-off-by: Pavel Emelyanov <xemul@scylladb.com>
2022-03-07 13:29:47 +03:00
Benny Halevy
c7de2e0682 compaction: log info message when interrupting compaction
Info messages are logged when compaction jobs start and finish
but there is no message logged when the job is interrupted, e.g.
when stopped by the compaction_manager.

Refs scylladb/scylla-dtest#2468

Signed-off-by: Benny Halevy <bhalevy@scylladb.com>
2022-03-07 11:43:58 +02:00
Botond Dénes
f1b2ff1722 Merge 'service: storage_service: announce new CDC generation immediately with RBNO' from Kamil Braun
When a new CDC generation is created (during bootstrap or otherwise), it
is assigned a timestamp. The timestamp must be propagated as soon as
possible, so all live nodes can learn about the generation before their
clocks reach the generation's timestamp. The propagation mechanism for
generation timestamps is gossip.

When bootstrap RBNO was enabled this was not the case: the generation
timestamp was inserted into gossiper state too late, after the repair
phase finished. Fix this.

Also remove an obsolete comment.

Fixes https://github.com/scylladb/scylla/issues/10149.

Closes #10154

* github.com:scylladb/scylla:
  service: storage_service: announce new CDC generation immediately with RBNO
  service: storage_service: fix indentation
2022-03-07 11:28:00 +02:00
Benny Halevy
a085ef74ff atomic_cell: compare_atomic_cell_for_merge: compare ttl if expiry is equal
Following up on a57c087c89,
compare_atomic_cell_for_merge should compare the ttl value in the
reverse order since, when comparing two cells that are identical
in all attributes but their ttl, we want to keep the cell with the
smaller ttl value rather than the larger ttl, since it was written
at a later (wall-clock) time, and so would remain longer after it
expires, until purged after gc_grace seconds.

Fixes #10173

Test: mutation_test.test_cell_ordering, unit(dev)

Signed-off-by: Benny Halevy <bhalevy@scylladb.com>
Message-Id: <20220302154328.2400717-1-bhalevy@scylladb.com>
Signed-off-by: Benny Halevy <bhalevy@scylladb.com>
Message-Id: <20220306091913.106508-1-bhalevy@scylladb.com>
2022-03-07 11:05:30 +02:00
Avi Kivity
9359a2caad Merge 'cql3: expr: Replace column_value::sub with subscript struct in expression' from Jan Ciołek
Currently a subscripted column is expressed using the struct `column_value`:
```c++
/// A column, optionally subscripted by a value (eg, c1 or c2['abc']).
struct column_value {
    const column_definition* col;
    std::optional<expression> sub; ///< If present, this LHS is col[sub], otherwise just col.
}
 ```

 It would be better to have a generic AST node for expressing arbitrary subscripted values:
 ```c++
 /// A subscripted value, eg list_colum[2], val[sub]
struct subscript {
    expression val;
    expression sub;
};
```

The `subscript` struct would allow us to express more, for example:
* subscripted `column_identifier`, not only `column_definition` (needed to get rid of `relation` class)
* nested subscripts: `col[1][2]`

Adding `subscript` to `expression` variant immediately would require to implement all `expr::visit` handlers immediately in the same commit, so I took a different approach. At first the struct is just there and visit handlers are implemented one by one in advance, then at the end `subscript` is added to the `expression`. This way all the new code can be neatly divided into commits and everything is still bisectable.

There were a few cases where the existing behaviour seemed to make little sense, but I didn't change it to keep the PR focused on refactoring. I left a `FIXME` comments there and I will submit separate patches to fix them.

Closes #10139

* github.com:scylladb/scylla:
  cql3: expr: Remove sub from column_value
  cql3: Create a subscript in single_column_relation
  cql3: expr: Add subscript to expression
  cql3: Handle subscript in multi_column_range_accumulator
  cql3: Handle subscript in selectable_process_selection
  cql3: expr: Handle subscript in test_assignment
  cql3: expr: Handle subscript in prepare_expression
  cql3: Handle subscript in prepare_selectable
  cql3: expr: Handle subscript in extract_clustering_prefix_restrictions
  cql3: expr: Handle subscript in extract_partition_range
  cql3: expr: Handle subscript in fill_prepare_context
  cql3: expr: Handle subscript in evaluate
  cql3: expr: Handle subscript in extract_single_column_restrictions_for_column
  cql3: expr: Handle subscript in search_and_replace
  cql3: expr: Handle subscript in recurse_until
  cql3: expr: Implement operator<< for subscript
  cql3: expr: Handle subscript in possible_lhs_values
  cql3: expr: Handle subscript in is_supported_by
  cql3: expr: Handle subscript in is_satisifed_by
  cql3: expr: Remove unused attribute
  cql3: expr: Use column_maybe_subscripted in is_one_of()
  cql3: expr: Use column_maybe_subscripted in limits()
  cql3: expr: Use column_maybe_subscripted in equal()
  cql3: expr: add get_subscripted_column(column_maybe_subscripted)
  cql3: expr: Add as_column_maybe_subscripted
  cql3: expr: Make get_value_comparator work with column_maybe_subscripted
  cql3: expr: Make get_value work with column_maybe_subscripted
  cql3: expr: Add column_maybe_subscripted
  cql3: expr: Add get_subscripted_column
  cql3: expr: Add subscript struct
2022-03-06 19:03:38 +02:00