Add test for currently implemented raft features. replication_test
tests replication functionality with various initial log configurations.
raft_fsm_test test voting state machine functionality.
"
The last major untracked area of the reader pipeline is the reader
buffers. These scale with the number of readers as well as with the size
and shape of data, so their memory consumption is unpredictable varies
wildly. For example many small rows will trigger larger buffers
allocated within the `circular_buffer<mutation_fragment>`, while few
larger rows will consume a lot of external memory.
This series covers this area by tracking the memory consumption of both
the buffer and its content. This is achieved by passing a tracking
allocator to `circular_buffer<mutation_fragment>` so that each
allocation it makes is tracked. Additionally, we now track the memory
consumption of each and every mutation fragment through its whole
lifetime. Initially I contemplated just tracking the `_buffer_size` of
`flat_mutation_reader::impl`, but concluded that as our reader trees are
typically quite deep, this would result in a lot of unnecessary
`signal()`/`consume()` calls, that scales with the number of mutation
fragments and hence adds to the already considerable per mutation
fragment overhead. The solution chosen in this series is to instead
track the memory consumption of the individual mutation fragments, with
the observation that these are typically always moved and very rarely
copied, so the number of `signal()`/`consume()` calls will be minimal.
This additional tracking introduces an interesting dilemma however:
readers will now have significant memory on their account even before
being admitted. So it may happen that they can prevent their own
admission via this memory consumption. To prevent this, memory
consumption is only forwarded to the semaphore upon admission. This
might be solved when the semaphore is moved to the front -- before the
cache.
Another consequence of this additional, more complete tracking is that
evictable readers now consume memory even when the underlying reader is
evicted. So it may happen that even though no reader is currently
admitted, all memory is consumed from the semaphore. To prevent any such
deadlocks, the semaphore now admits a reader unconditionally if no
reader is admitted -- that is if all count resources all available.
Refs: #4176
Tests: unit(dev, debug, release)
"
* 'track-reader-buffers/v2' of https://github.com/denesb/scylla: (37 commits)
test/manual/sstable_scan_footprint_test: run test body in statement sched group
test/manual/sstable_scan_footprint_test: move test main code into separate function
test/manual/sstable_scan_footprint_test: sprinkle some thread::maybe_yield():s
test/manual/sstable_scan_footprint_test: make clustering row size configurable
test/manual/sstable_scan_footprint_test: document sstable related command line arguments
mutation_fragment_test: add exception safety test for mutation_fragment::mutate_as_*()
test: simple_schema: add make_static_row()
reader_permit: reader_resources: add operator==
mutation_fragment: memory_usage(): remove unused schema parameter
mutation_fragment: track memory usage through the reader_permit
reader_permit: resource_units: add permit() and resources() accessors
mutation_fragment: add schema and permit
partition_snapshot_row_cursor: row(): return clustering_row instead of mutation_fragment
mutation_fragment: remove as_mutable_end_of_partition()
mutation_fragment: s/as_mutable_partition_start/mutate_as_partition_start/
mutation_fragment: s/as_mutable_range_tombstone/mutate_as_range_tombstone/
mutation_fragment: s/as_mutable_clustering_row/mutate_as_clustering_row/
mutation_fragment: s/as_mutable_static_row/mutation_as_static_row/
flat_mutation_reader: make _buffer a tracked buffer
mutation_reader: extract the two fill_buffer_result into a single one
...
After cleaning up old cluster features (253a7640e3)
the code for special handling of 1.7.4 counter order was effectively
only used in its own tests, so it can be safely removed.
Closes#7289
The memory usage is now maintained and updated on each change to the
mutation fragment, so it needs not be recalculated on a call to
`memory_usage()`, hence the schema parameter is unused and can be
removed.
We want to start tracking the memory consumption of mutation fragments.
For this we need schema and permit during construction, and on each
modification, so the memory consumption can be recalculated and pass to
the permit.
In this patch we just add the new parameters and go through the insane
churn of updating all call sites. They will be used in the next patch.
We will soon want to update the memory consumption of mutation fragment
after each modification done to it, to do that safely we have to forbid
direct access to the underlying data and instead have callers pass a
lambda doing their modifications.
Uses where this method was just used to move the fragment away are
converted to use `as_clustering_row() &&`.
Not used yet, this patch does all the churn of propagating a permit
to each impl.
In the next patch we will use it to track to track the memory
consumption of `_buffer`.
Current code uses a single counter to produce multiple buffer worth of
data. This uses carry-on from on buffer to the other, which happens to
work with the current memory accounting but is very fragile. Account
each buffer separately, resetting the counter between them.
The test consumes all resources off the semaphore, leaving just enough
to admit a single reader. However this amount is calculated based on the
base cost of readers, but as we are going to track reader buffers as
well, the amount of memory consumed will be much less predictable.
So to make sure background readers can finish during shutdown, release
all the consumed resources before leaving scope.
No point in continuing processing the entire buffer once a failure was
found. Especially that an early failure might introduce conditions that
are not handled in the normal flow-path. We could handle these but there
is no point in this added complexity, at this point the test is failed
anyway.
Some tests rely on `consume*()` calls on the permit to take effect
immediately. Soon this will only be true once the permit has been
admitted, so make sure the permit is admitted in these tests.
Currently per-shard reader contexts are cleaned up as soon as the reader
itself is destroyed. This causes two problems:
* Continuations attached to the reader destroy future might rely on
stuff in the context being kept alive -- like the semaphore.
* Shard 0's semaphore is special as it will be used to account buffers
allocated by the multishard reader itself, so it has to be alive until
after all readers are destroyed.
This patch changes this so that contexts are destroyed only when the
lifecycle policy itself is destroyed.
The reader recreation mechanism is a very delicate and error-prone one,
as proven by the countless bugs it had. Most of these bugs were related
to the recreated reader not continuing the read from the expected
position, inserting out-of-order fragments into the stream.
This patch adds a defense mechanism against such bugs by validating the
start position of the recreated reader.
The intent is to prevent corrupt data from getting into the system as
well as to help catch these bugs as close to the source as possible.
Fixes: #7208
Tests: unit(dev), mutation_reader_test:debug (v4)
* botond/evictable-reader-validate-buffer/v5:
mutation_reader_test: add unit test for evictable reader self-validation
evictable_reader: validate buffer after recreation the underlying
evictable_reader: update_next_position(): only use peek'd position on partition boundary
mutation_reader_test: add unit test for evictable reader range tombstone trimming
evictable_reader: trim range tombstones to the read clustering range
position_in_partition_view: add position_in_partition_view before_key() overload
flat_mutation_reader: add buffer() accessor
Currently, sstable_manager is used to create sstables, but it loses track
of them immediately afterwards. This series makes an sstable's life fully
contained within its sstable_manager.
The first practical impact (implemented in this series) is that file removal
stops being a background job; instead it is tracked by the sstable_manager,
so when the sstable_manager is stopped, you know that all of its sstable
activity is complete.
Later, we can make use of this to track the data size on disk, but this is not
implemented here.
Closes#7253
* github.com:scylladb/scylla:
sstables: remove background_jobs(), await_background_jobs()
sstables: make sstables_manager take charge of closing sstables
test: test_env: hold sstables_manager with a unique_ptr
test: drop test_sstable_manager
test: sstables::test_env: take ownership of manager
test: broken_sstable_test: prepare for asynchronously closed sstables_manager
test: sstable_utils: close test_env after use
test: sstable_test: dont leak shared_sstable outside its test_env's lifetime
test: sstables::test_env: close self in do_with helpers
test: perf/perf_sstable.hh: prepare for asynchronously closed sstables_manager
test: view_build_test: prepare for asynchronously closed sstables_manager
test: sstable_resharding_test: prepare for asynchronously closed sstables_manager
test: sstable_mutation_test: prepare for asynchronously closed sstables_manager
test: sstable_directory_test: prepare for asynchronously closed sstables_manager
test: sstable_datafile_test: prepare for asynchronously closed sstables_manager
test: sstable_conforms_to_mutation_source_test: remove references to test_sstables_manager
test: sstable_3_x_test: remove test_sstables_manager references
test: schema_changes_test: drop use of test_sstables_manager
mutation_test: adjust for column_family_test_config accepting an sstables_manager
test: lib: sstable_utils: stop using test_sstables_manager
test: sstables test_env: introduce manager() accessor
test: sstables test_env: introduce do_with_async_sharded()
test: sstables test_env: introduce do_with_async_returning()
test: lib: sstable test_env: prepare for life as a sharded<> service
test: schema_changes_test: properly close sstables::test_env
test: sstable_mutation_test: avoid constructing temporary sstables::test_env
test: mutation_reader_test: avoid constructing temporary sstables::test_env
test: sstable_3_x_test: avoid constructing temporary sstables::test_env
test: lib: test_services: pass sstables_manager to column_family_test_config
test: lib: sstables test_env: implement tests_env::manager()
test: sstable_test: detemplate write_and_validate_sst()
test: sstable_test_env: detemplate do_with_async()
test: sstable_datafile_test: drop bad 'return'
table: clear sstable set when stopping
table: prevent table::stop() race with table::query()
database: close sstable_manager:s
sstables_manager: introduce a stub close()
sstable_directory_test: fix threading confusion in make_sstable_directory_for*() functions
test: sstable_datafile_test: reorder table stop in compaction_manager_test
test: view_build_test: test_view_update_generator_register_semaphore_unit_leak: do not discard future in timer
test: view_build_test: fix threading in test_view_update_generator_register_semaphore_unit_leak
view: view_update_generator: drop references to sstables when stopping
sstables_manager should not be movable (since sstables hold a reference
to it). A following patch will enforce it.
Prepare by using unique_ptr to hold test_env::_manager. Right now, we'll
invoke sstables_manager move construction when creating a test_env with
do_with().
We could have chosen to update sstables when their sstables_manager is
moved, but we get nothing for the complexity.
With no users left (apart from some variants of column_family_test_config
which are removed in this patch) there are no more users, so remove it.
test_sstable_manager is obstructs sstables_manager from taking charge
of sstables ownership, since it a thread-local object. We can't close it,
since it will be used in the next test to run.
Instead of using test_sstables_manager, which we plan to drop,
carry our own sstables_manager in test_env, and close it when
test_env::stop() is called.
do_write_sst() creates a test_env, creates a shared_sstable using that test_env,
and destroys the test_env, and returns the sstable. This works now but will
stop working once sstable_manager becomes responsible for sstable lifetime.
Fortunately, do_write_sst() has one caller that isn't interested in the
return value at all, so fold it into that caller.
The test_env::do_with() are convenient for creating a scope
containing a test_env. Prepare them for asynchronously closed
sstables_manager by closing the test_env after use (which will,
in the future, close the embedded sstables_manager).
sstables_manager will soon be closed asynhronously, with a future-returning
close() function. To prepare for that, make the following changes
- replace test_sstables_manager with an sstables_manager obtained from test_env
- drop unneeded calls to await_background_jobs()
These changes allow lifetime management of the sstables_manager used
in the tests to be centralized in test_env.
sstables_manager will soon be closed asynhronously, with a future-returning
close() function. To prepare for that, make the following changes
- acquire a test_env with test_env::do_with() (or the sharded variant)
- change the sstable_from_existing_file function to be a functor that
works with either cql_test_env or test_env (as this is what individual
tests want); drop use of test_sstables_manager
- change new_sstable() to accept a test_env instead of using test_sstables_manager
- replace test_sstables_manager with an sstables_manager obtained from test_env
These changes allow lifetime management of the sstables_manager used
in the tests to be centralized in test_env.
sstables_manager will soon be closed asynhronously, with a future-returning
close() function. To prepare for that, make the following changes
- replace on-stack test_env with test_env::do_with()
- use the variant of column_family_for_tests that accepts an sstables_manager
- replace test_sstables_manager with an sstables_manager obtained from test_env
These changes allow lifetime management of the sstables_manager used
in the tests to be centralized in test_env.
Since test_env now calls await_background_jobs on termination, those
calls are dropped.
Use the sstables_manager from test_env. Use do_with_async() to create the test_env,
to allow for proper closing.
Since do_with_async() also takes care of await_background_jobs(), remove that too.
test_sstables_manager is going away, so replace it by test_env::manager().
column_family_test_config() has an implicit reference to test_sstables_manager,
so pass test_env::manager() as a parameter.
Calls to await_background_jobs() are removed, since test_env::stop() performs
the same task.
The large rows tests are special, since they use a custom sstables_manager,
so instead of using a test_env, they just close their local sstables_manager.
Acquire a test_env and extract an sstables_manager from that, passing it
to column_familty_test_config, in preparation for losing the default
constructor of column_familty_test_config.
Some tests need a sharded sstables_manager, prepare for that by
adding a stop() method and helpers for creating a sharded service.
Since test_env doesn't yet contain its own sstable_manager, this
can't be used in real life yet.
sstables::test_env needs to be properly closed (and will soon need it
even more). Use test_env::do_with_async() to do that. Removed
await_background_jobs(), which is now done by test_env::close().
A test_env contains an sstables_manager, which will soon have a close() method.
As such, it can no longer be a temporary. Switch to using test_env::do_with_async().
As a bonus, test_env::do_with_async() performs await_background_jobs() for us, so
we can drop it from the call sites.
A test_env contains an sstables_manager, which will soon have a close() method.
As such, it can no longer be a temporary. Switch to using test_env::do_with_async().
A test_env contains an sstables_manager, which will soon have a close() method.
As such, it can no longer be a temporary. Switch to using test_env::do_with_async().
As a bonus, test_env::do_with_async() performs await_background_jobs() for us, so
we can drop it from the call sites.
