Add method sstable::maybe_rebuild_filter_from_index() that rebuilds
bloom filters which had bad partition estimates when they were built.
The method checks the false positive rate based on the current bitset
size against the configured false positive rate to decide whether a
filter needs to be rebuilt. If the current false positive rate is within
75% to 125% of the configured false positive rate, the bloom filter will
not be rebuilt. Otherwise, the filter will be rebuilt from the index
entries. This method should only be called before an SSTable is sealed
as the bloom filter is updated in-place.
Signed-off-by: Lakshmi Narayanan Sreethar <lakshmi.sreethar@scylladb.com>
Extract out the filter format computing logic from sstable::read_filter
into a separate function. This is done so that the subsequent patches
can make use of this function.
Signed-off-by: Lakshmi Narayanan Sreethar <lakshmi.sreethar@scylladb.com>
Currently, the only way to get the size of a filter, for certain
parameters is to actually create one. This requires a seastar thread
context and potentially also allocates huge amount of memory.
Provdide a method which just calculates the size, without any of the
above mentioned baggage.
Signed-off-by: Lakshmi Narayanan Sreethar <lakshmi.sreethar@scylladb.com>
Default role creation in auth-v1 is asynchronous and all nodes race to
create it so we'd need to delay the test and wait. Checking this particular
role doesn't bring much value to the test as we check other roles
to demonstrate correctness.
Fixesscylladb/scylladb#19039Closesscylladb/scylladb#19424
Gossiper has two blocs of endpoints, both are registered in legacy/random place in main. This PR moves them next to gossiper start and adds unregistration for both.
refs: #2737Closesscylladb/scylladb#19425
* github.com:scylladb/scylladb:
api: Remove dedicated failure_detector registration method
api: Move failure_detector endpoints set/unset to gossiper
api: Unset failure detector endpoints method
api: (Un)Register gossiper API in correct place
api: Unset gossiper endpoints on stop
asi: Coroutinize set_server_gossip()
these jobs are scheduled to verify the builds of scylla, like
if it builds with the latest Seastar, if scylla can generated
reproducible builds, and if it builds with the nightly build of
clang. the failure of these workflow are not very visible without
clicking into the corresponding workflow in
https://github.com/scylladb/scylladb/actions.
in this change, we add their badges in the testing section of README.md,
so one can identify the test failures of them if any,
Signed-off-by: Kefu Chai <kefu.chai@scylladb.com>
Closesscylladb/scylladb#19430
since we've switched almost all callers of the operator<< to {fmt},
let's drop the unused operator<<:s.
Signed-off-by: Kefu Chai <kefu.chai@scylladb.com>
Closesscylladb/scylladb#19432
The sharded<database> is used as a map_reduce0() method provider,
there's no real need in database itself. Simple smp::map_reduce()
would work just as good.
Signed-off-by: Pavel Emelyanov <xemul@scylladb.com>
Closesscylladb/scylladb#19364
SELECT's "LIMIT" feature is tested in combination with other features
in different test/cql-pytest/*.py source files - for examples the
combination of LIMIT and GROUP BY is tested in test_group_by.py.
This patch adds a new test file, test_limit.py, for testing aspects
basic usage of LIMIT that weren't already tested in other files.
The new file also has a comment saying where we have other tests
for LIMIT combined with other features.
All the new tests pass (on both Scylla and Cassandra). But they can
be useful as regression tests to test patches which modify the
behavior of LIMIT - e.g., pull reques #18842.
This patch also adds another test in test_group_by.py. This adds to
one of the tests for the combination of LIMIT and GROUP BY (in this
case, GROUP BY of clustering prefix, no aggregation) also a check
for paging, that was previously missing.
Signed-off-by: Nadav Har'El <nyh@scylladb.com>
Closesscylladb/scylladb#19392
These two api functions both need gossiper service and only it, and thus
should have set/unset calls next to each other. It's worth putting them
into a single place
Signed-off-by: Pavel Emelyanov <xemul@scylladb.com>
There's one more set of endpoints that need gossiper -- the
failure_detector ones. They are registered, but not unregistered, so
here's the method to do it. It's not called by any code yet, because
next patch would need to rework the caller anyway.
Signed-off-by: Pavel Emelyanov <xemul@scylladb.com>
One of the next patches will add more async calls here, so not to
create then-chains, convert it into a coroutine
Signed-off-by: Pavel Emelyanov <xemul@scylladb.com>
`absl::headers` is a library, not the path to its headers.
before this change, the command lines of genereated build rule look
like:
```
-I/home/kefu/dev/scylladb/repair/absl::headers
```
this does not hurt, as other libraries might add the intended include
dir to the compiler command line, but this is just wrong.
so let's remove it. please note, `repair` target already links against
`absl::headers`. so we don't need to add `absl::headers` to its linkage
again.
Signed-off-by: Kefu Chai <kefu.chai@scylladb.com>
Closesscylladb/scylladb#19384
this change was created in the same spirit of ebff5f5d.
despite that we include Seastar as a submodule, Seastar is not a
part of scylla project. so we'd better include its headers using
brackets.
ebff5f5d addressed this cosmetic issue a while back. but probably
clangd's header-insertion helped some of contributor to insert
the missing headers with `"`. so this style of `include` returned
to the tree with these new changes.
unfortunately, clangd does not allow us to configure the style
of `include` at the time of writing.
Signed-off-by: Kefu Chai <kefu.chai@scylladb.com>
Closesscylladb/scylladb#19406
Before these changes, it could happen that Scylla initialized
endpoint managers for hint directories representing
* host IDs before migrating hinted handoff to using host IDs,
* IP addresses after the migration.
One scenario looked like this:
1. Start Scylla and upgrade the cluster to using host IDs.
2. Create, by hand, a hint directory representing an IP address.
3. Trigger changing the host filter in hinted handoff; it could
be achieved by, for example, restricting the set of data
centers Scylla is allowed to save hints for.
When changing the host filter, we browse the hint directories
and create endpoint managers if we can send hints towards
the node corresponding to a given hint directory. We only
accepted hint directories representing IP addresses
and host IDs. However, we didn't check whether the local node
has already been upgraded to host-ID-based hinted handoff
or not. As a result, endpoint managers were created for
both IP addresses and host IDs, no matter whether we were
before or after the migration.
These changes make sure that any time we browse the hint
directories, we take that into account.
Fixesscylladb/scylladb#19172Closesscylladb/scylladb#19173
also add `auth` and `cdc` to iwyu's `CLEANER_DIR` setting.
---
it's a cleanup, hence no need to backport.
Closesscylladb/scylladb#19410
* github.com:scylladb/scylladb:
.github: add auth and cdc to iwyu's CLEANER_DIR
cdc: do not include unused headers
in 7952200c, we changed the `selected_format` from `mc` to `me`,
but to be backward compatible the cluster starts with "md", so
when the nodes in cluster agree on the "ME_SSTABLE_FORMAT" feature,
the format selector believes that the node is already using "ME",
which is specified by `_selected_format`. even it is actually still
using "md", which is specified by `sstable_manager::_format`, as
changed by 54d49c04. as explained above, it was specified to "md"
in hope to be backward compatible when upgrading from an existign
installation which might be still using "md". but after a second
thought, since we are able to read sstables persisted with older
formats, this concern is not valid.
in other words, 7952200c introduced a regression which changed the
"default" sstable format from `me` to `md`.
to address this, we just change `sstable_manager::_format` to "me",
so that all sstables are created using "me" format.
a test is added accordingly.
Fixes#18995
Signed-off-by: Kefu Chai <kefu.chai@scylladb.com>
Closesscylladb/scylladb#19293
in
b8c705bc54
i modified the even name to `pull_request_target`,
This caused skipping sync process when PR label was added/removed
Fixing it
Closesscylladb/scylladb#19408
The node booting in gossip topology waits until all NORMAL
nodes are UP. If we removed a different node just before,
the booting node could still see it as NORMAL and wait for
it to be UP, which would time out and fail the bootstrap.
This issue caused scylladb/scylladb#17526.
Fix it by recalculating the nodes to wait for in every step of the
of the `wait_alive` loop.
Although the issue fixed by this PR caused only test flakiness,
it could also manifest in real clusters. It's best to backport this
PR to 5.4 and 6.0.
Fixesscylladb/scylladb#17526Closesscylladb/scylladb#19387
* github.com:scylladb/scylladb:
join_token_ring, gossip topology: update obsolete comment
join_token_ring, gossip topology: fix indendation after previous patch
join_token_ring, gossip topology: recalculate sync nodes in wait_alive
This patch adds a check if aggregation query is doing single-partition read and if so, makes the query to not use forward_service and do not parallelize the request.
Fixesscylladb/scylladb#19349Closesscylladb/scylladb#19350
* github.com:scylladb/scylladb:
test/boost/cql_query_test: add test for single-partition aggregation
cql3/select_statement: do not parallelize single-partition aggregations
flat_mutation_reader_v2 was introduced in a pair of commits in 2021:
e3309322c3 "Clone flat_mutation_reader related classes into v2 variants"
08b5773c12 "Adapt flat_mutation_reader_v2 to the new version of the API"
as a replacement for flat_mutation_reader, using range_tombstone_change
instead of range_tombstone to represent represent range tombstones. See
those commits for more information.
The transition was incremental; the last use of the original
flat_mutation_reader was removed in 2022 in commit
026f8cc1e7 "db: Use mutation_partition_v2 in mvcc"
In turn, flat_mutation_reader was introduced in 2017 in commit
748205ca75 "Introduce flat_mutation_reader"
To transition from a mutation_reader that nested rows within
a partition in a separate stream, to a flat reader that streamed
partitions and rows in the same stream.
Here, we reclaim the original name and rename the awkward
flat_mutation_reader_v2 to mutation_reader.
Note that mutation_fragment_v2 remains since we still use the original
for compatibilty, sometimes.
Some notes about the transition:
- files were also renamed. In one case (flat_mutation_reader_test.cc), the
rename target already existed, so we rename to
mutation_reader_another_test.cc.
- a namespace 'mutation_reader' with two definitions existed (in
mutation_reader_fwd.hh). Its contents was folded into the mutation_reader
class. As a result, a few #includes had to be adjusted.
Closesscylladb/scylladb#19356
Normally, the space overhead for TWCS is 1/N, where is number of windows. But during off-strategy, the overhead is 100% because input sstables cannot be released earlier.
Reshaping a TWCS table that takes ~50% of available space can result in system running out of space.
That's fixed by restricting every TWCS off-strategy job to 10% of free space in disk. Tables that aren't big will not be penalized with increased write amplification, as all input (disjoint) sstables can still be compacted in a single round.
Fixes#16514.
Closesscylladb/scylladb#18137
* github.com:scylladb/scylladb:
compaction: Reduce twcs off-strategy space overhead to 10% of free space
compaction: wire storage free space into reshape procedure
sstables: Allow to get free space from underlying storage
replica: don't expose compaction_group to reshape task
so that "wasm" target is built. "wasm" generates the text format
of wasm code. and these wasm applications are used by the test_wasm
tests.
the rules generated by `configure.py` adds these .wat files as a
dependency of `{mode}-build`, which is in turn a dependency of `{mode}`.
in this change, let's mirror this behavior by making `wasm` ALL,
so it is built by the default target.
Signed-off-by: Kefu Chai <kefu.chai@scylladb.com>
Closesscylladb/scylladb#19391
The service in question is pretty small one, but it has its API endpoint that lives in /storage_service group. Currently when a service starts and has any endpoints that depend on it, the endpoint registration should follow it (#2737). Here's the PR that does it for load meter. Another goal of this change is that http context now has one less dependency onboard.
Closesscylladb/scylladb#19390
* github.com:scylladb/scylladb:
api: Remove ctx->load_meter dependency
api: Use local load_meter reference in handlers
api: Fix indentation after previous patch
api: Coroutinize load_meter::get_load_map handler
api: Move load meter handlers
api: Add set/unset methods for load_meter
It seems that we skip the sync label process between PR and linked
Issues
Adding those debug prints will allow us to understand why
Closesscylladb/scylladb#19393
since we've switched almost all callers of the operator<< to {fmt}, let's drop the unused operator<<:s.
there are more occurrences of unused operator<< in the tree, but let's do the cleanup piecemeal.
---
this is a cleanup, so no need to backport
Closesscylladb/scylladb#19346
* github.com:scylladb/scylladb:
types: remove unused operator<<
node_ops: remove unused operator<<
lang: remove unused operator<<
gms: remove unused operator<<
dht: remove unused operator<<
test: do not use operator<< for std::optional
Now they are in storage service set/unset helper, but there's the
dedicated set/unset pair for meter's enpoints.
Signed-off-by: Pavel Emelyanov <xemul@scylladb.com>
The meter is pretty small sevice and its API is also tiny. Still, it's a
standalone top-level service, and its API should come next to it.
Signed-off-by: Pavel Emelyanov <xemul@scylladb.com>
Currently if task_manager::task::impl::abort preempts before children are recursively aborted and then the task gets unregistered, we hit use after free since abort uses children vector which is no longer alive.
Modify abort method so that it goes over all tasks in task manager and aborts those with the given parent.
Fixes: #19304.
Requires backport to all versions containing task manager
Closesscylladb/scylladb#19305
* github.com:scylladb/scylladb:
test: add test for abort while a task is being unregistered
tasks: fix tasks abort
before this change, because it seems that we move away from `p2` in
each iteration, so the succeeding iterations are moving from an empty
`p2`, clang-tidy warns at seeing this.
but we only move from `p2._static_row` in the first iteration when the
dest `mutation_partition` instance's static row is empty. and in the
succeeding iterations, the dest `mutation_partition` instance's static
row is not empty anymore if it is set. so, this is a false alarm.
in this change, we silence this warning. another option is to extract
the single-shot mutation out of the loop, and pass the `std::move(p2)`
only for the single-shot mutation, but that'd be a much more intrusive
change. we can revisit this later.
Signed-off-by: Kefu Chai <kefu.chai@scylladb.com>
Closesscylladb/scylladb#19331
