Commit Graph

27519 Commits

Author SHA1 Message Date
Pavel Emelyanov
5f99eeb35e transport: Keep lifecycle notifier sharded reference
It's needed to (un)subscribe server on it (next patch).

Signed-off-by: Pavel Emelyanov <xemul@scylladb.com>
2021-07-22 18:48:20 +03:00
Pavel Emelyanov
2a30cb1664 main: Use local lifecycle notifier to (un)subscribe listeners
The storage proxy and sl-manager get subscribed on lifecycle
events with the help of storage service. Now when the notifier
lives in main() they can use it directly.

Signed-off-by: Pavel Emelyanov <xemul@scylladb.com>
2021-07-22 18:47:15 +03:00
Pavel Emelyanov
8248bc9e33 main, tests: Push notifier through storage service
Now it's time to move the lifecycle notifier from storage
service to the main's scope. Next patches will remove the
$lifecycle-subscriber -> storage_service dependency.

Signed-off-by: Pavel Emelyanov <xemul@scylladb.com>
2021-07-22 18:45:51 +03:00
Pavel Emelyanov
6b3b01d9a6 storage_service: Move notification core into dedicated class
Introduce the endpoint_lifecycle_notifier class that's in
charge of keeping track of subscribers and notifying them.
The subscribers will thus be able to set and unset their
subscription without the need to mess with storage service
at all.

The storage_service for now keeps the notifier on board, but
this is going to change in the next patch.

Signed-off-by: Pavel Emelyanov <xemul@scylladb.com>
2021-07-22 18:44:02 +03:00
Pavel Emelyanov
7e8a032013 storage_service: Split lifecycle notification code
This prepares the ground for moving the notification engine
into own class like it was done for migration_notifier some
time ago.

Signed-off-by: Pavel Emelyanov <xemul@scylladb.com>
2021-07-22 18:43:14 +03:00
Pavel Emelyanov
c7b0b25494 transport, generic_server: Remove no longer used functionality
After subscription management was moved onto controller level
a bunch of code can be dropped:

- passing migration notifier beyond controller
- event_notifier's _stopped bit
- event_notifier .stop() method
- event_notifier empty constructor and destrictor
- generic_server's on_stop virtual method

Signed-off-by: Pavel Emelyanov <xemul@scylladb.com>
2021-07-22 18:41:32 +03:00
Pavel Emelyanov
1acef41626 transport: (Un)Subscribe cql_server::event_notifier from controller
There's a migration notifier that's carried through cql_server
_just_ to let event-notifier (un)subscribe on it. Also there's
a call for global storage-service in there which will need to
be replaced with yet another pass-through argument which is not
great.

It's easier to establish this subscription outside of cql_server
like it's currently done for proxy and sl-manager. In case of
cql_server the "outside" is the controller.

This patch just moves the subscription management from cql_server
to controller, next two patches will make more use of this change.

Signed-off-by: Pavel Emelyanov <xemul@scylladb.com>
2021-07-22 18:37:23 +03:00
Pavel Emelyanov
b57fb0aa9a tests: Remove storage service from manual gossiper test
It's not needed there, gossiper starts and works without it.

Signed-off-by: Pavel Emelyanov <xemul@scylladb.com>
2021-07-22 18:36:28 +03:00
Raphael S. Carvalho
e4eb7df1a1 table: Make correctness of concurrent sstable list update robust
Today, table relies on row_cache::invalidate() serialization for
concurrent sstable list updates to produce correct results.
That's very error prone because table is relying on an implementation
detail of invalidate() to get things right.
Instead, let's make table itself take care of serialization on
concurrent updates.
To achieve that, sstable_list_builder is introduced. Only one
builder can be alive for a given table, so serialization is guaranteed
as long as the builder is kept alive throughout the update procedure.

Signed-off-by: Raphael S. Carvalho <raphaelsc@scylladb.com>
Message-Id: <20210721001716.210281-1-raphaelsc@scylladb.com>
2021-07-21 16:45:30 +03:00
Botond Dénes
84c9bf2b63 tools/scylla-sstable-index: remove global reader concurrency semaphore
Use a local one instead and make sure to stop it before it is destroyed.

Message-Id: <20210721133754.356229-1-bdenes@scylladb.com>
2021-07-21 16:41:01 +03:00
Raphael S. Carvalho
aad72289e2 table: Kill load_sstable()
That function is dangerously used by distributed loader, as the latter
was responsible for invalidating cache for new sstable.
load_sstable() is an unsafe alternative to
add_sstable_and_update_cache() that should never have been used by
the outside world. Instead, let's kill it and make loader use
the safe alternative instead.
This will also make it easier to make sure that all concurrent updates
to sstable set are properly serialized.

Additionally, this may potentially reduce the amount of data evicted
from the cache, when the sstables being imported have a narrow range,
like high level sstables imported from a LCS table. Unlikely but
possible.

Signed-off-by: Raphael S. Carvalho <raphaelsc@scylladb.com>
Message-Id: <20210721131949.26899-1-raphaelsc@scylladb.com>
2021-07-21 16:21:42 +03:00
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