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
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
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
