The SSTable is removed from the reclaimed memory tracking logic only
when its object is deleted. However, there is a risk that the Bloom
filter reloader may attempt to reload the SSTable after it has been
unlinked but before the SSTable object is destroyed. Prevent this by
removing the SSTable from the reclaimed list maintained by the manager
as soon as it is unlinked.
The original logic that updated the memory tracking in
`sstables_manager::deactivate()` is left in place as (a) the variables
have to be updated only when the SSTable object is actually deleted, as
the memory used by the filter is not freed as long as the SSTable is
alive, and (b) the `_reclaimed.erase(*sst)` is still useful during
shutdown, for example, when the SSTable is not unlinked but just
destroyed.
Fixes#19722
Signed-off-by: Lakshmi Narayanan Sreethar <lakshmi.sreethar@scylladb.com>
Added a new method, on_unlink() to the sstable_manager. This method is
now used by the sstable to notify the manager when it has been unlinked,
enabling the manager to update its bookkeeping as required. The
on_unlink method doesn't do anything yet but will be updated by the next
patch.
Signed-off-by: Lakshmi Narayanan Sreethar <lakshmi.sreethar@scylladb.com>
In some cases, the S3 server will not know about a certain build and
any attempt to open a coredump which was generated by this build will
fail, because the S3 server returns an empty/illegal response.
There is already a bypass for missing package-url in the S3 server
response, but this doesn't help in the case when the response is also
missing other metadata, like build-id and version info.
Extend this existig mechanism with a new --scylla-package-url flag,
which provides complete bypass. When provided, the S3 server will not be
queried at all, instead the package is downloaded from the link and
version metadata is extracted from the package itself.
Closesscylladb/scylladb#19769
* seastar 908ccd93...67065040 (44):
> metrics: Use this_shard_id unconditionally
> sstring: prevent fmt from formatting sstring as a sequence
> coding style: allow lines up to 160 chars in length
> src/core: remove unnecessary includes
> when_all: stop using deprecated std::aligned_union_t
> reactor: respect preempt requests in debug mode
> core: fix -Wunused-but-set-variable
> gate: add try_hold
> sstring: declare nested type with typename
> rpc: pass start time to `wait_for_reply()` which accepts `no_wait_type`
> scripts/perftune.py: get rid of "SyntaxWarning: invalid escape sequence"
> scripts/perftune.py: add support for tweaking VLAN interfaces
> scripts/perftune.py: improve discovery of bond device slaves
> scripts/perftune.py: refactor __learn_slaves() function
> code-cleanup: add missing header guards
> code-cleanup: remove redundant includes of 'reactor.hh'
> code-cleanup: explicitly depend on io_desc.hh
> scripts/perftune.py: aRFS should be disabled by default in non-MQ mode
> code-cleanup: remove unneeded includes of fair_queue.hh
> docker: fix mount of install-dependencies
> code-cleanup: remove redundant includes of linux-aio.hh
> fstream: reformat the doxygen comment of make_file_input_stream()
> iostream: use new-style consumer to implement copy()
> stall-analyser: use 0 for default value of --minimum
> reactor: fix crash during metrics gathering
> build: run socket test with linux-aio reactor backend
> test: Add testing of connect()-ion abort ability
> linux_perf_event: exclude_idle only on x86_64
> linux_perf_event: add make_linux_perf_event
> stall-analyser: gracefully handle empty input
> shared_token_bucket: resolve FIXME
> io_tester: ensure that file object is valid when closing it
> tutorial.md: fix typo in Dan Kegel's name
> test,rpc: Extend simple ping-pong case
> rpc: Calculate delay and export it via metrics
> rpc: Exchange handler duration with server responses
> rpc: Track handler execution time
> rpc: Fix hard-coded constants when sending unknown verb reply
> reactor: Unfriend alien and smp queues
> reactor: Add and use stopped() getter
> reactor: Generalize wakeup() callers
> file: Use lighter access to map of fs-info-s
> file: Fix indentation after previous patch
> file: Don't return chain of ready futures from make_file_impl
Closesscylladb/scylladb#19780
Pass origin when opening the sstable from the writer and store it in the
sstable object. This will make the origin available for the entire write
path.
Closesscylladb/scylladb#19721
* github.com:scylladb/scylladb:
sstables: use _origin in write path
sstable::open_sstable: pass and store origin
This PR adds support for aborting index reads from within `index_consume_entry_context::consume_input` when the server is being stopped. The abort source is now propagated down to the `index_consume_entry_context`, making it available for `consume_input` to check if an abort has been requested. If an abort is detected, `consume_input` will throw an exception to stop the index read operation.
Closesscylladb/scylladb#19453
* github.com:scylladb/scylladb:
test/boost: test abort behaviour during index read
sstables/index_reader: stop consuming index when abort has been requested
sstables::index_consume_entry_context: store abort_source
sstable: drop old filter only after the new filter is built during rebuild
sstables/sstables_manager: store abort_source in sstable_manager
replica/database: pass abort_source to database constructor
Currently the guard does not account correctly for ongoing operation if semaphore acquisition fails. It may signal a semaphore when it is not held.
Should be backported to all supported versions.
Closesscylladb/scylladb#19699
* github.com:scylladb/scylladb:
test: add test to check that coordinator lwt semaphore continues functioning after locking failures
paxos: do not signal semaphore if it was not acquired
since fmt 11, it is required that the format() to be const, otherwise
its caller in fmt library would not be able to call it. and compile
would fail like:
```
/home/kefu/.local/bin/clang++ -DFMT_SHARED -DSCYLLA_BUILD_MODE=release -DSEASTAR_API_LEVEL=7 -DSEASTAR_LOGGER_COMPILE_TIME_FMT -DSEASTAR_LOGGER_TYPE_STDOUT -DSEASTAR_SCHEDULING_GROUPS_COUNT=16 -DSEASTAR_SSTRING -DXXH_PRIVATE_API -DCMAKE_INTDIR=\"RelWithDebInfo\" -I/home/kefu/dev/scylladb -I/home/kefu/dev/scylladb/seastar/include -I/home/kefu/dev/scylladb/build/seastar/gen/include -I/home/kefu/dev/scylladb/build/seastar/gen/src -I/home/kefu/dev/scylladb/build/gen -isystem /home/kefu/dev/scylladb/abseil -ffunction-sections -fdata-sections -O3 -g -gz -std=gnu++23 -fvisibility=hidden -Wall -Werror -Wextra -Wno-error=deprecated-declarations -Wimplicit-fallthrough -Wno-c++11-narrowing -Wno-deprecated-copy -Wno-mismatched-tags -Wno-missing-field-initializers -Wno-overloaded-virtual -Wno-unsupported-friend -Wno-enum-constexpr-conversion -Wno-unused-parameter -ffile-prefix-map=/home/kefu/dev/scylladb=. -march=westmere -Xclang -fexperimental-assignment-tracking=disabled -mllvm -inline-threshold=2500 -fno-slp-vectorize -U_FORTIFY_SOURCE -Werror=unused-result -MD -MT locator/CMakeFiles/scylla_locator.dir/RelWithDebInfo/abstract_replication_strategy.cc.o -MF locator/CMakeFiles/scylla_locator.dir/RelWithDebInfo/abstract_replication_strategy.cc.o.d -o locator/CMakeFiles/scylla_locator.dir/RelWithDebInfo/abstract_replication_strategy.cc.o -c /home/kefu/dev/scylladb/locator/abstract_replication_strategy.cc
In file included from /home/kefu/dev/scylladb/locator/abstract_replication_strategy.cc:9:
In file included from /home/kefu/dev/scylladb/locator/abstract_replication_strategy.hh:16:
In file included from /home/kefu/dev/scylladb/gms/inet_address.hh:11:
In file included from /usr/include/fmt/ostream.h:23:
In file included from /usr/include/fmt/chrono.h:23:
In file included from /usr/include/fmt/format.h:41:
/usr/include/fmt/base.h:1393:23: error: no matching member function for call to 'format'
1393 | ctx.advance_to(cf.format(*static_cast<qualified_type*>(arg), ctx));
| ~~~^~~~~~
/usr/include/fmt/base.h:1374:21: note: in instantiation of function template specialization 'fmt::detail::value<fmt::context>::format_custom_arg<locator::vnode_effective_replication_map::factory_key, fmt::formatter<locator::vnode_effective_replication_map::factory_key>>' requested here
1374 | custom.format = format_custom_arg<
| ^
/home/kefu/dev/scylladb/seastar/include/seastar/util/log.hh:299:33: note: in instantiation of function template specialization 'fmt::format_to<seastar::internal::log_buf::inserter_iterator &, locator::vnode_effective_replication_map::factory_key &, const void *, 0>' requested here
299 | return fmt::format_to(it, fmt.format, std::forward<Args>(args)...);
| ^
/home/kefu/dev/scylladb/seastar/include/seastar/util/log.hh:428:9: note: in instantiation of function template specialization 'seastar::logger::log<locator::vnode_effective_replication_map::factory_key &, const void *>' requested here
428 | log(log_level::debug, std::move(fmt), std::forward<Args>(args)...);
| ^
/home/kefu/dev/scylladb/locator/abstract_replication_strategy.cc:561:18: note: in instantiation of function template specialization 'seastar::logger::debug<locator::vnode_effective_replication_map::factory_key &, const void *>' requested here
561 | rslogger.debug("create_effective_replication_map: found {} [{}]", key, fmt::ptr(erm.get()));
| ^
/home/kefu/dev/scylladb/locator/abstract_replication_strategy.hh:471:10: note: candidate function template not viable: 'this' argument has type 'const fmt::formatter<locator::vnode_effective_replication_map::factory_key>', but method is not marked const
471 | auto format(const locator::vnode_effective_replication_map::factory_key& key, FormatContext& ctx) {
| ^
1 error generated.
```
Signed-off-by: Kefu Chai <kefu.chai@scylladb.com>
Closesscylladb/scylladb#19768
Fixes#19753
SSTable file open provides an `io_error_handler` instance which is applied to a file-wrapper to process any IO errors happing during read/write via the handler in `storage_service`, which in turn will effectively disable the node. However, this is not applied to the actual open operation itself, i.e. any exception generated by the file open call itself will instead just escape to caller.
This PR adds filtering via the `error_handler` to sstable open + makes `storage_service` "isolate" mechanism non-module-static (thus making it testable) and adds tests to check we exhibit the same behaviour in both cases.
The main motivation for this issue it discussions that secondary level IO issues (i.e. caused by extensions) should trigger the same behaviour as, for example, running out of disk space.
Closesscylladb/scylladb#19766
* github.com:scylladb/scylladb:
memtable_test: Add test for isolate behaviour on exceptions during flush
cql_test_env: Expose storage service
storage_service: Make isolate guard non-static and add test accessor
sstable: apply error_handler on open exceptions
cql-pytest's config_value_context is used to run a code sequence with
different ScyllaDB configuration applied for a while. When it reads
the original value (in order to restore it later), it applies
ast.literal_eval() to it. This is strange, since the config variable isn't
a Python literal.
It was added in 8c464b2ddb ("guardrails: restrict replication
strategy (RS)"). Presumably, as a workaround for #19604 - it sufficiently
massaged the input we read via SELECT to be acceptable later via UPDATE.
Now that #19604 is fixed, we can remove the call to ast.literal_eval,
but have to fix up the parameters to config_value_context to something
that will be accepted without further massaging.
This is a step towards fixing #15559, where we want to run some tests
with a boolean configuration variable changed, and literal_eval is
transforming the string representation of integers to integers and
confusing the driver.
Closesscylladb/scylladb#19696
`GitHub Actions / Analyze #includes in source files` keeps reporting
that the include shouldn't be present in the file. The reason is
that we use FMT with version >10, so the fragment of the code that
uses the include is not compiled. We move the include to a place
where it's used, which should fix the warnings.
Closesscylladb/scylladb#19776
Makes storage service isolate repeatable in same process and more testable.
Note, since the test var now is shard-local we need to check twice: once
on error, once on reaching shard zero for actual shutdown.
Now that the origin is available inside the sstable object, no need to
pass it to the methods called in the write path.
Signed-off-by: Lakshmi Narayanan Sreethar <lakshmi.sreethar@scylladb.com>
Pass origin when opening the sstable from the writer and store it in the
sstable object. This will make the origin available for the entire write
path.
Signed-off-by: Lakshmi Narayanan Sreethar <lakshmi.sreethar@scylladb.com>
Added a new boost test, index_reader_test, with a testcase to verifyi
the abort behaviour during an index read using
index_consume_entry_context.
Signed-off-by: Lakshmi Narayanan Sreethar <lakshmi.sreethar@scylladb.com>
When an abort is requested, stop further reading of the index file and
throw and exception from index_consume_entry_context::process_state().
Signed-off-by: Lakshmi Narayanan Sreethar <lakshmi.sreethar@scylladb.com>
Store abort source inside sstables::index_consume_entry_context, so that
the next patch can implement cancelling the index read when abort is
requested.
Signed-off-by: Lakshmi Narayanan Sreethar <lakshmi.sreethar@scylladb.com>
sstable::maybe_rebuild_filter_from_index drops the existing filter first
and then rebuilds the new filter as the method is only called before the
sstable is sealed. But to make the index read abortable, the old filter
can be dropped only after the new filter is built so that in case if the
index consumer gets aborted, we still have the old filter to write to
disk.
Signed-off-by: Lakshmi Narayanan Sreethar <lakshmi.sreethar@scylladb.com>
Add a new member that stores the abort_source. This can later be used by
the sstables to check if an abort has been requested. Also implement
sstables_manager::get_abort_source() that returns a const reference to
the abort source.
Signed-off-by: Lakshmi Narayanan Sreethar <lakshmi.sreethar@scylladb.com>
This is in preparation for the following patch that adds abort_source
variable to the sstables_manager.
Signed-off-by: Lakshmi Narayanan Sreethar <lakshmi.sreethar@scylladb.com>
The python driver might currently trigger spurios reconnects that cause
the `NoHostAvailable` to be thrown, which is not expected.
This patch adds a retry mechanism to the test to make skip this failure
if it occurs, as a work-around.
The proper fix is expected to be done in the scylladb/python-driver#295,
once fixed there this work-around can be reverted.
Fixes: scylladb/scylla#18547Closesscylladb/scylladb#19759
The guard signals a semaphore during destruction if it is marked as
locked, but currently it may be marked as locked even if locking failed.
Fix this by using semaphore_units instead of managing the locked flag
manually.
Fixes: https://github.com/scylladb/scylladb/issues/19698
There are two schemas associated with a sstable writer:
the sstable's schema (i.e. the schema of the table at the time when the
sstable object was created), and the writer's schema (equal to the schema
of the reader which is feeding into the writer).
It's easy to mix up the two and break something as a result.
The writer's schema is needed to correctly interpret and serialize the data
passing through the writer, and to populate the on-disk metadata about the
on-disk schema.
The sstables's schema is used to configure some parameters for newly created
sstable, such as bloom filter false positive ratio, or compression.
This series fixes the known mixups between the two — when setting up compression,
and when setting up the bloom filters.
Fixes#16065
The bug is present in all supported versions, so the patch has to be backported to all of them.
Closesscylladb/scylladb#19695
* github.com:scylladb/scylladb:
sstables/mx/writer: when creating local_compression, use the sstables's schema, not the writer's
sstables/mx/writer: when creating filter, use the sstables's schema, not the writer's
sstables: for i_filter downcasts, use dynamic_cast instead of static_cast
```
DEBUG 2024-07-03 00:59:58,291 [shard 0:main] compaction_manager - Compaction task 0x51800002a480 for table tests.3 compaction_group=0 [0x503000062050]: switch_state: none -> pending: pending=2 active=0 done=0 errors=0
DEBUG 2024-07-03 01:00:02,868 [shard 0:main] compaction - Checking droppable sstables in tests.3, candidates=0
DEBUG 2024-07-03 01:00:02,868 [shard 0:main] compaction - time_window_compaction_strategy::newest_bucket:
now 1720314000000000
buckets = {
key=1720314000000000, size=2
key=1720310400000000, size=2
1720314000000000: GMT: Sunday, July 7, 2024 1:00:00 AM
1720310400000000: GMT: Sunday, July 7, 2024 12:00:00 AM
```
the test failed to complete when ran across different clock hours, as it
expected all sstables produced to belong to same window of 1h size.
let's fix it by reusing timestamps, so it's always consistent.
Fixes#13280.
Fixes#18564.
Signed-off-by: Raphael S. Carvalho <raphaelsc@scylladb.com>
Closesscylladb/scylladb#19749
The leader_host API handler was eventually using the `req` unique_ptr
after it has been already destroyed (passed down to the future lambda
by reference). This was causing an occassional crash in some tests.
Reworked the leader_host handler to use the req only outside of the
future lambda.
Also updated the code to handle the possibility that the non-default
leader group (other than Group 0) might reside on a different shard
than the shard 0 - using the same concept of calling on all shards via
`invoke_on_all()` as done for the other requests.
Fixesscylladb/scylladb#19714Closesscylladb/scylladb#19715
utils::in uses std::aligned_storage, which is deprecated. Rather than fixing it, replace its only
user with simpler code and remove it.
No backport needed as this isn't fixing a bug.
Closesscylladb/scylladb#19683
* github.com:scylladb/scylladb:
utils: remove utils/in.hh
gossiper: remove initializer-list overload of add_local_application_state()
Since Python 3.12, version parsing becomes strict, parse_version() does
not accept the version string like '6.1.0~dev'.
To fix this, we need to pass acceptable version string to parse_version() like
'6.1.0.dev0', which is allowed on Python version scheme.
Also, release canditate version like '6.0.0~rc3' has same issue, it
should be replaced to '6.0.0rc3' to compare in parse_version().
reference: https://packaging.python.org/en/latest/specifications/version-specifiers/Fixes#19564Closesscylladb/scylladb#19572
This reverts commit 65fbf72ed0, since
it breaks scylla-housekeeping and SCT because the patch modified
version string.
We shoudn't modify version string directly, need to pass
modified string just for parse_version() instead.
Setting the error condition for all nodes in the cluster to avoid
having to check which one is the coordinator. This should make the test
more stable and avoid the flakiness observed when the coordinator node
is the one that got the error condition injected.
Randomizing the retrieved running servers to reproduce the issue more
frequently and to avoid making any assumptions about the order of the
servers.
Note that only the "raft_topology_barrier_fail" needs to run
on a non-coordinator node, the other error "stream_ranges_fail" can be
injected on any node (including the coordinator).
Fixes: scylladb/scylladb#18614Closesscylladb/scylladb#19663
This patch is a follow-up to scylladb/scylladb#16585.
Once we have service levels on raft, we can get rid of update loop, which updates the configuration in a configured interval (default is 10s).
Instead, this PR introduces methods to `group0_state_machine` which look through table ids in mutations in `write_mutation` and update submodules based on that ids.
Fixes: scylladb/scylladb#18060Closesscylladb/scylladb#18758
* github.com:scylladb/scylladb:
test: remove `sleep()`s which were required to reload service levels configuration
test/cql_test_env: remove unit test service levels data accessors
service/storage_service: reload SL cache on topology_state_load()
service/qos/service_level_controller: move semaphore breaking to stop
service/qos/service_level_controller: maybe start and stop legacy update loop
service/qos/service_level_controller: make update loop legacy
raft/group0_state_machine: update submodules based on table_id
service/storage_service: add a proxy method to reload sl cache
There are two schema's associated with a sstable writer:
the sstable's schema (i.e. the schema of the table at the time when the
sstable object was created), and the writer's schema (equal to the schema
of the reader which is feeding into the writer).
It's easy to mix up the two and break something as a result.
The writer's schema is needed to correctly interpret and serialize the data
passing through the writer, and to populate the on-disk metadata about the
on-disk schema.
The sstables's schema is used to configure some parameters for newly created
sstable, such as bloom filter false positive ratio, or compression.
The problem fixed by this patch is that the writer was wrongly creating
the compressor objects based on its own schema, but using them based
based on the sstable's schema the sstable's schema.
This patch forces the writer to use the sstable's schema for both.
There are two schema's associated with a sstable writer:
the sstable's schema (i.e. the schema of the table at the time when the
sstable object was created), and the writer's schema (equal to the schema
of the reader which is feeding into the writer).
It's easy to mix up the two and break something as a result.
The writer's schema is needed to correctly interpret and serialize the data
passing through the writer, and to populate the on-disk metadata about the
on-disk schema.
The sstables's schema is used to configure some parameters for newly created
sstable, such as bloom filter false positive ratio, or compression.
The problem fixed by this patch is that the writer was wrongly creating
the filter based on its own schema, while the layer outside the writer
was interpreting it as if it was created with the sstable's schema.
This patch forces the writer to pick the filter's parameters based on the
sstable's schema instead.
As of this patch, those static_casts are actually invalid in some cases
(they cast to the wrong type) because of an oversight.
A later patch will fix that. But to even write a reliable reproducer
for the problem, we must force the invalid casts to manifest as a crash
(instead of weird results).
This patch both allows writing a reproducer for the bug and serves
as a bit of defensive programming for the future.
If the index was created on collection (both frozen or not), its description wasn't a correct create statement.
This patch fixes the bug and includes functions like `full()`, `keys()`, `values()`, ... used to create index on collections.
Fixesscylladb/scylladb#19278Closesscylladb/scylladb#19381
* github.com:scylladb/scylladb:
cql-pytest/test_describe: add a test for describe indexes
schema/schema: fix column names in index description
