Commit Graph

27508 Commits

Author SHA1 Message Date
Botond Dénes
a819f013f6 compaction/compaction: create_compaction_info(): take const compaction_descriptor&
Don't copy the descriptor.

Signed-off-by: Botond Dénes <bdenes@scylladb.com>
Message-Id: <20210721120219.326972-1-bdenes@scylladb.com>
2021-07-21 16:19:03 +03:00
Gleb Natapov
7261c2c93e raft: return a correct leader when leaving leader state
When a leader moves to a follower state it aborts all requests that are
waiting on an admission semaphore with not_a_leader exception. But
currently it specifies itself as a new leader since abortion happens
before the fsm state changes to a follower. The patch fixes this by
destroying leader state after fsm state already changed to be a
follower.

Message-Id: <YPbI++0z5ZPV9pKb@scylladb.com>
2021-07-21 00:42:39 +02:00
Nadav Har'El
c4f20f1641 Update seastar submodule
* seastar ef320940...388ee307 (4):
  > Merge 'Add a stall analyser tool' from Benny Halevy
  > compat: implement coroutine_handle<void> for <experimental/coroutine> header
  > Merge "Make app_template::run noexcept" from Pavel E
  > perftune.py: make RPS CPU set to be a full CPU set

The stall analyser tool was requested by the SCT team to help make
sense of Scylla's stall reports and find more stall bugs!
2021-07-21 00:47:11 +03:00
Tomasz Grabiec
dcd05f77b1 lsa: Avoid excessive eviction if region is not compactible
Introduced in d72b91053b.

If region was not compactible, for example because it has dense
segments, we would keep evicting even though the target for reclaimed
segments was met. In the worst case we may have to evict whole cache.

Refs #9038 (unlikely to be the cause though)
Message-Id: <20210720104039.463662-1-tgrabiec@scylladb.com>
2021-07-20 14:36:14 +03:00
dgarcia360
8d51482ffe docs: moved latest_version to conf.py
Related issues: scylladb/sphinx-scylladb-theme#87

All the variables related to the multiversion extension are now defined in conf.py instead of using the GitHub Actions file.

How to test this PR
Run make multiversionpreview on docs folder. When you open https://0.0.0.0:5500, the browser should render the documentation site.

Closes #7957
2021-07-20 14:31:46 +03:00
Avi Kivity
05fcf11557 Merge 'Coroutinize commit log' from Calle Wilund
No real refactoring, just move the various methods to coroutines. Because coroutines are neat.
Broken down into one method per change to make review easier. And hoping I get tipped per change.

Grand idea being that using coroutines will eventually make real refactoring easier.
Unit tests + relevant dtest.

As discussed below, simply coroutinizing the code will, at least in the fast path, cause the slightly naive
compiler to generate multiple unused coroutine frames, dropping raw performance a bit.
The last two patches in this series addresses this, by breaking the fast path into non-coroutine
subroutines (no futures involved) and one coroutine main loop.

Results, as collected by `perf_simple_query` are:
Master (before changes):
```
{
        "parameters" :
        {
                "concurrency" : 100,
                "concurrency,partitions,cpus,duration" : "100,10000,1,30",
                "cpus" : 1,
                "duration" : 30,
                "partitions" : 10000
        },
        "stats" :
        {
                "allocs_per_op" : 52.237303521776113,
                "instructions_per_op" : 47403.34422198555,
                "mad tps" : 670.12528706749436,
                "max tps" : 140817.0800358199,
                "median tps" : 139391.58369995767,
                "min tps" : 133663.0095463676,
                "tasks_per_op" : 13.189605506751203
        },
        "test_properties" :
        {
                "type" : "write"
        },
        "versions" :
        {
                "scylla-server" :
                {
                        "commit_id" : "1f51bc67fd",
                        "date" : "20210712",
                        "run_date_time" : "2021-07-13 10:26:46",
                        "version" : "4.6.dev"
                }
        }
}
```

This PR (coroutines + fast path optimization patches):
```
{
        "parameters" :
        {
                "concurrency" : 100,
                "concurrency,partitions,cpus,duration" : "100,10000,1,30",
                "cpus" : 1,
                "duration" : 30,
                "partitions" : 10000
        },
        "stats" :
        {
                "allocs_per_op" : 52.208628061750559,
                "instructions_per_op" : 47300.501878330339,
                "mad tps" : 707.70233700674726,
                "max tps" : 139618.0661493362,
                "median tps" : 137891.11290420164,
                "min tps" : 127551.83433347062,
                "tasks_per_op" : 13.172121395660733
        },
        "test_properties" :
        {
                "type" : "write"
        },
        "versions" :
        {
                "scylla-server" :
                {
                        "commit_id" : "1d4b6f50bd",
                        "date" : "20210719",
                        "run_date_time" : "2021-07-19 09:27:09",
                        "version" : "4.6.dev"
                }
        }
}
```

I.e. both allocations/op and instruction count seem to be on par.

Closes #8954

* github.com:scylladb/scylla:
  commitlog: Make allocate_when_possible a template
  commitlog: break fast path alloc into non-fut/corout + outer loop
  commitlog: Drop stream/subscription from replayer
  commitlog: coroutinize commitlog::read_log_file
  commitlog: coroutinize commitlog::create_commitlog
  commitlog: coroutinize commitlog::add_entries
  commitlog: coroutinize commitlog::add_entry
  commitlog: coroutinize commitlog::add
  commitlog: change entry_writer usage to reference
  commitlog: coroutinize segment_manager::clear
  commitlog: coroutinize segment_manager::do_pending_deletes
  commitlog: coroutinize segment_manager::delete_file
  commitlog: coroutinize segment_manager::shutdown
  commitlog: coroutinize segment_manager::shutdown_all_segments
  commitlog: coroutinize segment_manager::sync_all_segments
  commitlog: coroutinize segment_manager::clear_reserve_segments
  commitlog: coroutinize segment_manager::active_segment
  commitlog: coroutinize segment_manager::new_segment
  commitlog: coroutinize segment_manager::allocate_segment
  commitlog: coroutinize segment_manager::rename_file
  commitlog: coroutinize segment_manager::init
  commitlog: coroutinize segment_manager::list_descriptors
  commitlog: coroutinize segment_manager::replenish_reserve
  commitlog: coroutinize segment::shutdown
  commitlog: coroutinize segment::close
  commitlog: coroutinize segment::batch_cycle
  commitlog: coroutinize segment::do_flush
  commitlog: coroutinize segment::flush
  commitlog: coroutinize segment::cycle
  commitlog: coroutinize allocate_when_possible
  commitlog: coroutinize segment::allocate
2021-07-20 14:14:13 +03:00
Tomasz Grabiec
50ec3ea295 lsa: Fix misaccunting of used space when allocating lsa_buffers
lsa_buffer allocations are aligned to 4K. If smaller size is
requested, whole 4K is used. However, only requested size was used in
accounting segment occupancy. This can confuse reclaimer which may
think the segment is sparse while it is actually dense, and compacting
it will yield no or little gain. This can cause inefficient memory
reclamation or lack of progress.

Refs #9038
Message-Id: <20210720104110.463812-1-tgrabiec@scylladb.com>
2021-07-20 14:08:06 +03:00
Pavel Solodovnikov
d2b53bc0ca configure: simplify raft tests dependencies management
There's no need for extended `scylla_raft_dependencies`,
which includes the entire `scylla_core` target list.

Revert the tests which don't need the extended list to use
a minimal set of dependencies and switch to using
`scylla_core` as a dependency for
`raft_sys_table_storage_test` and `raft_address_map_test`.

Signed-off-by: Pavel Solodovnikov <pa.solodovnikov@scylladb.com>
Message-Id: <20210705104712.295499-1-pa.solodovnikov@scylladb.com>
2021-07-20 12:32:10 +03:00
Botond Dénes
8fc55fa5bf reader_concurrency_semaphore: get rid of struct permit_list
struct permit_list exists so the intrusive list declaration which needs
the definition of reader_permit can be hidden in the .cc. But it turns
out that if the hook type is fully spelled out, the intrusive list
declaration doesn't need T to be defined. Exploit this to get rid of
this extra indirection.

Signed-off-by: Botond Dénes <bdenes@scylladb.com>
Message-Id: <20210720073121.63027-2-bdenes@scylladb.com>
2021-07-20 10:35:12 +03:00
Botond Dénes
11b39cbc23 reader_concurrency_semaphore: merge permit_stats into stats
If there was any reason to have them separate when permit_stats was
conceived, it is gone now, so merge the two.

Signed-off-by: Botond Dénes <bdenes@scylladb.com>
Message-Id: <20210720073121.63027-1-bdenes@scylladb.com>
2021-07-20 10:35:12 +03:00
Tomasz Grabiec
a8528cb24d lsa: Fix uninitialized field access resulting in hangs during segment compaction
_free_space may be initialized with garbage so kind() getter should
only look at the bit which corresponds to the kind. Misclasification
of segment as being of different kind may result in a hang during
segment compaction.

Surfaced in debug mode build where the field is filled with 0xbebebebe.

Introduced in b5ca0eb2a2.

Fixes #9057
Message-Id: <20210719232734.443964-1-tgrabiec@scylladb.com>
2021-07-20 02:33:21 +03:00
Tomasz Grabiec
393b90112f gdb: segment-descs: Support debug mode builds
Debug mode builds have a different implementation of segment_store in LSA.
Message-Id: <20210719232125.442458-1-tgrabiec@scylladb.com>
2021-07-20 02:33:18 +03:00
Gleb Natapov
aa8c6b85fb raft: do not apply empty command list
Do not call user's state machine apply() if there is nothing to apply.

Message-Id: <YO1dMitXnZhZlmra@scylladb.com>
2021-07-19 18:26:18 +02:00
Nadav Har'El
36ec1d792e Merge 'cql-pytest: Test selecting from indexed table using only clustering key' from Jan Ciołek
Add examples from issue #8991 to tests
Both of these tests pass on `cassandra 4.0` but fail on `scylla 4.4.3`

First test tests that selecting values from indexed table using only clustering key returns correct values.
The second test tests that performing this operation requires filtering.

The filtering test looks similar to [the one for #7608](1924e8d2b6/test/cql-pytest/test_allow_filtering.py (L124)) but there are some differences - here the table has two clustering columns and an index, so it could test different code paths.

Contains a quick fix for the `needs_filtering()` function to make these tests pass.
It returns `true` for this case and the one described in #7708.

This implementation is a bit conservative - it might sometimes return `true` where filtering isn't actually needed, but at least it prevents scylla from returning incorrect results.

Fixes #8991.
Fixes #7708.

Closes #8994

* github.com:scylladb/scylla:
  cql3: Fix need_filtering on indexed table
  cql-pytest: Test selecting using only clustering key requires filtering
  cql-pytest: Test selecting from indexed table using clustering key
2021-07-19 18:23:08 +03:00
Tomasz Grabiec
049a1ef729 Merge 'flat_mutation_reader: downgrade_to_v1 - reset state of rt_assembler' from enedil
The downgrade_to_v1 didn't reset the state of range tombstone assembler
in case of the calls to next_partition or fast_forward_to, which caused
a situation where the closing range tombstone change is cleared from the
buffer before being emitted, without notifying the assembler. This patch
fixes the behaviour in fast_forward_to as well.

Fixes #9022

Closes #9023

* github.com:scylladb/scylla:
  flat_mutation_reader: downgrade_to_v1 - reset state of rt_assembler
  flat_mutation_reader: introduce public method returning the default size of internal buffer.
2021-07-19 17:10:23 +02:00
Jan Ciolek
54149242b4 cql3: Fix need_filtering on indexed table
There were cases where a query on an indexed table
needed filtering but need_filtering returned false.

This is fixed by using new conditions in cases where
we are using an index.

Fixes #8991.
Fixes #7708.

For now this is an overly conservative implementation
that returns true in some cases where filtering
is not needed.

Signed-off-by: Jan Ciolek <jan.ciolek@scylladb.com>
2021-07-19 16:22:17 +02:00
Michał Radwański
67d99e02a7 flat_mutation_reader: downgrade_to_v1 - reset state of rt_assembler
The downgrade_to_v1 didn't reset the state of range tombstone assembler
in case of the calls to next_partition or fast_forward_to, which caused
a situation where the closing range tombstone change is cleared from the
buffer before being emitted, without notifying the assembler. This patch
fixes the behaviour in fast_forward_to as well.

Fixes #9022
2021-07-19 15:54:26 +02:00
Michał Radwański
c4089007a2 flat_mutation_reader: introduce public method returning the default size
of internal buffer.

This method is useful in tests that examine behaviour after the buffer
has been filled up.
2021-07-19 15:54:13 +02:00
Nadav Har'El
4c6dc5fce2 Merge 'continuous_data_consumer: properly skip bytes at the end of a range' from Wojciech Mitros
When skipping bytes at the end of a continuous_data_consumer range,
the position of the consumer is moved after the skipped bytes, but
the position of the underlying input_stream is not.

This patch adds skipping of the underlying input_stream, to make
its position consistent with the position of the consumer.

Fixes #9024

Signed-off-by: Wojciech Mitros <wojciech.mitros@scylladb.com>

Closes #9039

* github.com:scylladb/scylla:
  tests: add test for skipping bytes at end of consumer
  continuous_data_consumer: properly skip bytes at the end of a range
2021-07-19 15:57:26 +03:00
Botond Dénes
27fbca84f6 reader_concurrency_semaphore: remove prethrow_action
The semaphore accepts a functor as in its constructor which is run just
before throwing on wait queue overload. This is used exclusively to bump
a counter in the database::stats, which counts queue overloads. However,
there is now an identical counter in
reader_concurrency_semaphore::stats, so the database can just use that
directly and we can retire the now unused prethrow action.

Signed-off-by: Botond Dénes <bdenes@scylladb.com>
Message-Id: <20210716111105.237492-1-bdenes@scylladb.com>
2021-07-19 15:47:37 +03:00
Wojciech Mitros
507bdfc36a tests: add test for skipping bytes at end of consumer
The new tests confirms that the regression issue, where
we didn't correctly skip bytes at the end of a
continuous_data_consumer range, is fixed.

Signed-off-by: Wojciech Mitros <wojciech.mitros@scylladb.com>
2021-07-19 14:42:38 +02:00
Wojciech Mitros
7107e32390 continuous_data_consumer: properly skip bytes at the end of a range
When skipping bytes at the end of a continuous_data_consumer range,
the position of the consumer is moved after the skipped bytes, but
the position of the underlying input_stream is not.

This patch adds skipping of the underlying input_stream, to make
its position consistent with the position of the consumer.

Fixes #9024

Signed-off-by: Wojciech Mitros <wojciech.mitros@scylladb.com>
2021-07-19 11:43:30 +02:00
Piotr Sarna
38afef71b9 Merge 'Service Level Controller: Stop polling distributed data..
... when decommissioned (reworked)' from Eliran Sinvani

This is a rework of #8916 The polling loop of the service level
controller queries a distributed table in order to detect configuration
changes. If a node gets decommissioned, this loop continues to run until
shutdown, if a node stays in the decommissioned mode without being shut
down, the loop will fail to query the table and this will result in
warnings and eventually errors in the log. This is not really harmful
but it adds unnecessary noise to the log.  The series below lays the
infrastructure for observing storage service state changes, which
eventually being used to break the loop upon preparation for
decommissioning.  Tests: Unit test (dev) Failing tests in jenkins.

Fixes #8836

The previous merge (possibly due to conflict resolution) contained a
misplaced get that caused an abort on shutdown.

Closes #9035

* github.com:scylladb/scylla:
  Service Level Controller: Stop configuration polling loop upon leaving the cluster
  main: Stop using get_local_storage_service in main
2021-07-19 10:52:42 +02:00
Benny Halevy
3700702e90 cmake: update compaction source files location
Signed-off-by: Benny Halevy <bhalevy@scylladb.com>
Message-Id: <20210718120906.701185-1-bhalevy@scylladb.com>
2021-07-19 11:47:35 +03:00
Botond Dénes
5aa733f933 sstables/mx/writer: initialize _range_tombstones at the end of the ctor
We need a permit to initialize said object which makes the semaphore
used and hence trigger an error if an exception is thrown in the
constructor. Move the initialization to the end of the constructor to
prevent this.

Signed-off-by: Botond Dénes <bdenes@scylladb.com>
Message-Id: <20210719040449.9202-1-bdenes@scylladb.com>
2021-07-19 11:43:00 +03:00
Calle Wilund
4990ba2769 commitlog: Make allocate_when_possible a template
And call it by-value with the polymorphic writers. This
eliminates outer coroutine frame and ensures we use only one
for fast-case allocation.
2021-07-19 08:27:30 +00:00
Calle Wilund
69ead0e658 commitlog: break fast path alloc into non-fut/corout + outer loop
Removes 2 coroutine frames in fast path (as long as segment + space is
avail). Puts IPS back on track with master.
2021-07-19 08:27:30 +00:00
Calle Wilund
62acc84e58 commitlog: Drop stream/subscription from replayer
Change args to values so stays on coroutine frame.
Remove pointless subscription/stream usage, just iterate.
2021-07-19 08:27:30 +00:00
Calle Wilund
5e8af28da7 commitlog: coroutinize commitlog::read_log_file 2021-07-19 08:27:30 +00:00
Calle Wilund
b3c35f9ec0 commitlog: coroutinize commitlog::create_commitlog 2021-07-19 08:27:30 +00:00
Calle Wilund
ef471d0a93 commitlog: coroutinize commitlog::add_entries 2021-07-19 08:27:30 +00:00
Calle Wilund
96434b1b12 commitlog: coroutinize commitlog::add_entry 2021-07-19 08:27:30 +00:00
Calle Wilund
e16cff6952 commitlog: coroutinize commitlog::add 2021-07-19 08:27:30 +00:00
Calle Wilund
da360fb841 commitlog: change entry_writer usage to reference
Calling frames keeps object alive in all paths. Use references in
allocate()/allocate_when_possible()
2021-07-19 08:27:30 +00:00
Calle Wilund
42bfae513a commitlog: coroutinize segment_manager::clear 2021-07-19 08:27:30 +00:00
Calle Wilund
554a09baab commitlog: coroutinize segment_manager::do_pending_deletes 2021-07-19 08:27:30 +00:00
Calle Wilund
9e18cf3f5f commitlog: coroutinize segment_manager::delete_file 2021-07-19 08:27:30 +00:00
Calle Wilund
ca65387c53 commitlog: coroutinize segment_manager::shutdown 2021-07-19 08:27:30 +00:00
Calle Wilund
4678d1fbec commitlog: coroutinize segment_manager::shutdown_all_segments 2021-07-19 08:27:30 +00:00
Calle Wilund
2f048e658b commitlog: coroutinize segment_manager::sync_all_segments 2021-07-19 08:27:30 +00:00
Calle Wilund
ad4e4e9ee4 commitlog: coroutinize segment_manager::clear_reserve_segments 2021-07-19 08:27:30 +00:00
Calle Wilund
ec430807fc commitlog: coroutinize segment_manager::active_segment 2021-07-19 08:27:30 +00:00
Calle Wilund
13bba1ef39 commitlog: coroutinize segment_manager::new_segment 2021-07-19 08:27:30 +00:00
Calle Wilund
ccd34203dc commitlog: coroutinize segment_manager::allocate_segment 2021-07-19 08:27:30 +00:00
Calle Wilund
f5de830f0c commitlog: coroutinize segment_manager::rename_file 2021-07-19 08:27:30 +00:00
Calle Wilund
011bc68209 commitlog: coroutinize segment_manager::init 2021-07-19 08:27:30 +00:00
Calle Wilund
04c725b29c commitlog: coroutinize segment_manager::list_descriptors 2021-07-19 08:27:30 +00:00
Calle Wilund
d514fc5822 commitlog: coroutinize segment_manager::replenish_reserve 2021-07-19 08:27:30 +00:00
Jan Ciolek
9bd62a07c9 cql-pytest: Test selecting using only clustering key requires filtering
Adds test that creates a table with primary key (p, c1, c2)
with a global index on c2 and then selects where c1 = 1 and c2 = 1.

This should require filtering, but doesn't.
Refs #8991.

Signed-off-by: Jan Ciolek <jan.ciolek@scylladb.com>
2021-07-19 10:24:48 +02:00
Jan Ciolek
a041767aa3 cql-pytest: Test selecting from indexed table using clustering key
Adds test that creates a table with primary key (p, c1, c2)
with a global index on c2 and then selects where c1 = 1 and c2 = 1.

This currently fails.
Refs #8991.

Signed-off-by: Jan Ciolek <jan.ciolek@scylladb.com>
2021-07-19 10:24:46 +02:00