this change updates the cqlsh submodule:
* tools/cqlsh/ ba83aea3...73bdbeb0 (4):
> install.sh: replace tab with spaces
> define the the debug packge is empty
> tests: switch from using cqlsh bash to the test the python file
> package python driver as wheels
it also includes follow change to package cqlsh as a regular
rpm instead of as a "noarch" rpm:
so far cqlsh bundles the python-driver in, but only as source.
meaning the package wasn't architecture, and also didn't
have the libev eventloop compiled in.
Since from python 3.12 and up, that would mean we would
fallback into asyncio eventloop (which still exprimental)
or into error (once we'll sync with the driver upstream)
so to avoid those, we are change the packaging of cqlsh
to be architecture specific, and get cqlsh compiled, and bundle
all of it's requirements as per architecture installed bundle of wheels.
using `shiv`, i.e. one file virtualenv that we'll be packing
into our artifacts
Ref: https://github.com/scylladb/scylla-cqlsh/issues/90
Ref: https://github.com/scylladb/scylla-cqlsh/pull/91
Ref: https://github.com/linkedin/shivClosesscylladb/scylladb#19385
* tools/cqlsh ba83aea...242876c (1):
> Merge 'package python driver as wheels' from Israel Fruchter
Update tools/cqlsh/ submodule
in which, the change of `define the the debug packge is empty`
should address the build failure like
```
Processing files: scylla-cqlsh-debugsource-6.1.0~dev-0.20240624.c7748f60c0bc.aarch64
error: Empty %files file /jenkins/workspace/scylla-master/next/scylla/tools/cqlsh/build/redhat/BUILD/scylla-cqlsh/debugsourcefiles.list
RPM build errors:
Empty %files file /jenkins/workspace/scylla-master/next/scylla/tools/cqlsh/build/redhat/BUILD/scylla-cqlsh/debugsourcefiles.list
```
Closesscylladb/scylladb#19473
When preparing statement, the server code first does it on non-local
shards, then on local one. The former call is done the hard way, while
there's a short sugar sharded<> class method doing it.
Signed-off-by: Pavel Emelyanov <xemul@scylladb.com>
Closesscylladb/scylladb#19485
The series adds a step during node's boot process, just before completing
the initialization, in which the node sends a notification to all other
normal nodes in the cluster that it is UP now. Other nodes wait for this
node to be UP and in normal state before replying. This ensures that,
in a healthy cluster, when a node start serving queries the entire
cluster knows its up-to-date state. The notification is a best effort
though. If some nodes are down or do not reply in time the boot process
continues. It is somewhat similar to shutdown notification in this regard.
* 'gleb/notify-up-v2' of github.com:scylladb/scylla-dev:
gossiper: wait for a bootstrapping node to be seen as normal on all nodes before completing initialization
Wait for booting node to be marked UP before complete booting.
gossiper: move gossip verbs to the idl
Delete 10s timeout from read barrier in table_sync_and_check,
so that the function always considers all previous group0 changes.
Fixes: #18490.
Closesscylladb/scylladb#18752
The `system.batchlog` table has a partition for each batch that failed to complete. After finally applying the batch, the partition is deleted. Although the table has gc_grace_second = 0, tombstones can still accumulate in memory, because we don't purge partition tombstones from either the memtable or the cache. This can lead to the cache and memtable of this table to accumulate many thousands of even millions of tombstones, making batchlog replay very slow. We didn't notice this before, because we would only replay all failed batches on unbootstrap, which is rare and a heavy and slow operation on its own right already.
With repair-based tombstone-gc however, we do a full batchlog replay at the beginning of each repair, and now this extra delay is noticeable.
Fix this by making sure batchlog replays don't have to scan through all the tombstones generated by previous replays:
* flush the `system.batchlog` memtable at the end of each batchlog replay, so it is cleared of tombstones
* bypass the cache
Fixes: https://github.com/scylladb/scylladb/issues/19376
Although this is not a regression -- replay was like this since forever -- now that repair calls into batchlog replay, every release which uses repair-based tombstone-gc should get this fix
Closesscylladb/scylladb#19377
* github.com:scylladb/scylladb:
db/batchlog_manager: bypass cache when scanning batchlog table
db/batchlog_manager: replace open-coded paging with internal one
db/batchlog_manager: implement cleanup after all batchlog replay
cql3/query_processor: for_each_cql_result(): move func to the coro frame
The bloom filters are built with partition estimates because the actual
partition count might not be available in all cases. If the estimate is
inaccurate, the bloom filters might end up being too large or too small
compared to their optimal sizes. This PR rebuilds bloom filters with
inaccurate partition estimates using the actual partition count before
the filter is written to disk. A bloom filter is considered to have an
inaccurate estimate if its false positive rate based on the current
bitmap size is either less than 75% or more than 125% of the configured
false positive rate.
Fixes#19049
A manual test was run to check the impact of rebuild on compaction.
Table definition used : CREATE TABLE scylla_bench.simple_table (id int PRIMARY KEY);
Setup : 3 billion random rows with id in the range [0, 1e8) were inserted as batches of 5 rows into scylla_bench.simple_table via 80 threads.
Compaction statistics :
scylla_bench.simple_table :
(a) Total number of compactions : `1501`
(b) Total time spent in compaction : `9h58m47.269s`
(c) Number of compactions which rebuilt bloom filters : `16`
(d) Total time taken by these 16 compactions which rebuilt bloom filters : `2h55m11.89s`
(e) Total time spent by these 16 compactions to rebuild bloom filters : `8m6.221s` which is
- `4.63%` of the total time taken by the compactions which rebuilt filters (d)
- `1.35%` of the total compaction time (b).
(f) Total bytes saved by rebuilding filters : `388 MB`
system.compaction_history :
(a) Total number of compactions : `77`
(b) Total time spent in compaction : `21.24s`
(c) Number of compactions which rebuilt bloom filters : `74`
(d) Time taken by these 74 compactions which rebuilt bloom filters : `20.48s`
(e) Time spent by these 74 compactions to rebuild bloom filters : `377ms` which is
- `1.84%` of the total time taken by the compactions which rebuilt filters (d)
- `1.77%` of the total compaction time (b).
(f) Total bytes saved by rebuilding filters : `20 kB`
The following tables also had compactions and the bloom filter was rebuilt in all those compactions.
However, the time taken for every rebuild was observed as 0ms from the logs as it completed within a microsecond :
system.raft :
(a) Total number of compactions : `2`
(b) Total time spent in compaction : `106ms`
(c) Total bytes saved by rebuilding filters : `960 B`
system_schema.tables :
(a) Total number of compactions : `1`
(b) Total time spent in compaction : `25ms`
(c) Total bytes saved by rebuilding filter : `312 B`
system.topology :
(a) Total number of compactions : `1`
(b) Total time spent in compaction : `25ms`
(c) Total bytes saved by rebuilding filter : `320 B`
Closesscylladb/scylladb#19190
* github.com:scylladb/scylladb:
bloom_filter_test: add testcase to verify filter rebuilds
test/boost: move bloom filter tests from sstable_datafile_test into a new file
sstables/mx/writer: rebuild bloom filters with bad partition estimates
sstables/mx/writer: add variable to track number of partitions consumed
sstable: introduce sstable::maybe_rebuild_filter_from_index()
sstable: add method to return filter format for the given sstable version
utils/i_filter: introduce get_filter_size()
API endpoints that need a particular service to get data from are registered next to this service (#2737). In /storage_proxy function there live some endpoints that work with config, so this PR moves them to the existing config.cc with config-related endpoints. The path these endpoints are registered with remains intact, so some tweak in proxy API registration is also here.
Closesscylladb/scylladb#19417
* github.com:scylladb/scylladb:
api: Use provided db::config, not the one from ctx
api: Move some config endpoints from proxy to config
api: Split storage_proxy api registration
api: Unset config endpoints
Scans should not pollute the cache with cold data, in general. In the
case of the batchlog table, there is another reason to bypass the cache:
this table can have a lot of partition tombstones, which currently are
not purged from the cache. So in certain cases, using the cache can make
batch replay very slow, because it has to scan past tombstones of
already replayed batches.
We have a commented code snippet from Origin with cleanup and a FIXME to
implement it. Origin flushes the memtables and kicks a compaction. We
only implement the flush here -- the flush will trigger a compaction
check and we leave it up to the compaction manager to decide when a
compaction is worthwhile.
This method used to be called only from unbootstrap, so a cleanup was
not really needed. Now it is also called at the end of repair, if the
table is using repair-based tombstone-gc. If the memtable is filled with
tombstones, this can add a lot of time to the runtime of each repair. So
flush the memtable at the end, so the tombstones can be purged (they
aren't purged from memtables yet).
Said method has a func parameter (called just f), which it receives as
rvalue ref and just uses as a reference. This means that if caller
doesn't keep the func alive, for_each_cql_result() will run into
use-after-free after the first suspention point. This is unexpected for
callers, who don't expect to have to keep something alive, which they
passed in with std::move().
Adjust the signature to take a value instead, value parameters are moved
to the coro frame and survive suspention points.
Adjust internal callers (query_internal()) the same way.
There are no known vulnerable external callers.
Today after Mergify opened a Backport PR, it will stay open until someone manually close the backport PR , also we can't track using labels which backport was done or not since there is no indication for that except digging into the PR and looking for a comment or a commit ref
The following changes were made in this PR:
* trigger add-label-when-promoted.yaml also when the push was made to `branch-x.y`
* Replace label `backport/x.y` with `backport/x.y-done` in the original PR (this will automatically update the original Issue as well)
* Add a comment on the backport PR and close it
Fixes: https://github.com/scylladb/scylladb/issues/19441Closesscylladb/scylladb#19442
Currently, the returned `ranges` vector is first initialized
to `product_size` and then the returned partition ranges are
copied into it.
Instead, we can simply reserve the vector capacity,
without initializing it, and then emplace all partition ranges
onto it using std::back_inserter.
Signed-off-by: Benny Halevy <bhalevy@scylladb.com>
Closesscylladb/scylladb#19457
since we've switched almost all callers of the operator<< to {fmt},
let's drop the unused operator<<:s.
the callers in alternator/streams.cc is updated to use `fmt::print()`
to format the `bytes` instances.
Signed-off-by: Kefu Chai <kefu.chai@scylladb.com>
Closesscylladb/scylladb#19448
The Ninja makefile (build.ninja) generated by the ./configure.py script
is smart enough to notice when the configure.py script is modified and
re-runs the script in order to regenerate itself. However, this
operation is currently not idempotent and quickly breaks because
information about the Ninja makefile's name is not passed properly.
This is the rule used for makefile's regeneration:
```
rule configure
command = {python} configure.py --out={buildfile}.new $configure_args && mv {buildfile}.new {buildfile}
generator = 1
description = CONFIGURE $configure_args
```
The `buildfile` variable holds the value of the `--out` option which is
set to `build.ninja` if not provided explicitly.
Note that regenerating the makefile passes a name with the `.new` suffix
added to the end; we want to first write the file in full and then
overwrite the old file via a rename. However, notice that the script was
called with `--out=build.ninja.new`; the `configure` rule in the
regenerated file will have `configure.py --out=build.ninja.new.new` and
then `mv build.ninja.new.new build.ninja.new`. So, second regeneration
will just leave a build.ninja.new file which is not useful.
Fix this by introducing an additional parameter `--out-final-name`.
This parameter is only supposed to be used in the regeneration rule and
its purpose is to preserve information about the original file name.
After this change I no longer see `build.ninja.new` being created after
a sequence of `touch configure.py && ninja` calls.
Closesscylladb/scylladb#19428
The docs [1] clearly say "install-dependencies.sh" should be run as
"root"; however, the script silently assumes that the umask inherited from
the calling environment is 0022. That's not necessarily the case, and
there's an argument to be made for "root" setting umask 0077 by default.
The script behaves unexpectedly under such circumstances; files and
directories it creates under /opt and /usr/local are then not accessible
to unprivileged users, leading to compilation failures later on.
Set the creation mask explicitly to 0022.
[1] https://github.com/scylladb/scylladb/blob/master/HACKING.md#dependencies
Signed-off-by: Laszlo Ersek <laszlo.ersek@scylladb.com>
Closesscylladb/scylladb#19464
Since some time executing our ninja build
targets generates also build.ninja.new file.
Adding it to .gitignore for convenience as we
won't commit this file.
Closesscylladb/scylladb#19367
Since 3afbd21f, we are able to selectively choose a single test
in a boost test executable which represents a test suite, and to
choose a single test in a pytest script with the syntax of
"test_suite::test_case". it's very handy for manual testing.
so let's document in the command line help message as well.
Signed-off-by: Kefu Chai <kefu.chai@scylladb.com>
Closesscylladb/scylladb#19454
before this change, when linking an executable referencing `marker`,
we could have following error:
```
13:58:02 ld.lld: error: undefined symbol: alternator::event_id::marker
13:58:02 >>> referenced by streams.cc
13:58:02 >>> build/dev/alternator/streams.o:(from_string_helper<rapidjson::GenericValue<rapidjson::UTF8<char>, rjson::internal::throwing_allocator>, alternator::event_id>::Set(rapidjson::GenericValue<rapidjson::UTF8<char>, rjson::internal::throwing_allocator>&, alternator::event_id, rjson::internal::throwing_allocator&))
13:58:02 clang-16: error: linker command failed with exit code 1 (use -v to see invocation)
```
it turns out `event_id::marker` is only declared, but never defined.
please note, the non-inline static member variable in its class
definition is not considered as a definition, see
[class.static.data](https://eel.is/c++draft/class.static.data#3)
> The declaration of a non-inline static data member in its class
> definition is not a definition and may be of an incomplete type
> other than cv void.
so, let's declare it as a `constexpr` instead. it implies `inline`.
Signed-off-by: Kefu Chai <kefu.chai@scylladb.com>
Closesscylladb/scylladb#19452
before this change, when running test like:
```console
./test.py --mode release topology_experimental_raft/test_tablets
/home/kefu/dev/scylladb/test/pylib/scylla_cluster.py:333: SyntaxWarning: invalid escape sequence '\('
deleted_sstable_re = f"^.*/{keyspace}/{table}-[0-9a-f]{{32}}/.* \(deleted\)$"
```
we could have the warning above. because `\(` is not a valid escape
sequence, but the Python interpreter accepts it as two separated
characters of `\(` after complaining. but it's still annoying.
so, let's use a raw string here, as we want to match "(deleted)".
Signed-off-by: Kefu Chai <kefu.chai@scylladb.com>
Closesscylladb/scylladb#19451
The bloom filters are built with partition estimates, as the actual
partition count might not be available in all the cases. If the estimate
was bad, the bloom filters might end up too large or too small than
their optimal sizes. Rebuild such bloom filters with actual partition
count before the filter is written to disk and the sstable is sealed.
Fixes#19049
Signed-off-by: Lakshmi Narayanan Sreethar <lakshmi.sreethar@scylladb.com>
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>
since we are now able to use C++20, there is no need to use the
homebrew rotl64(). so in this change, we replace rotl64() with
std::rotl(), and remove the former from the source tree.
the underlying implementations of these two solutions are equivalent,
so no performance changes are expected. all caller sites have been
audited: all of them pass `uint64` as the first parameter.
Signed-off-by: Kefu Chai <kefu.chai@scylladb.com>
Closesscylladb/scylladb#19447
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
