As noted in issue #11979, Scylla inconsistently (and unlike Cassandra)
requires "IS NOT NULL" one some but not all materialized-view key
columns. Specifically, Scylla does not require "IS NOT NULL" on the
base's partition key, while Cassandra does.
This patch is a test which demonstrates this inconsistency. It currently
passes on Cassandra and fails on Scylla, so is marked xfail.
Refs #11979
Signed-off-by: Nadav Har'El <nyh@scylladb.com>
Closes#11980
The get_live_token_owners returns the nodes that are part of the ring
and live.
The get_unreachable_token_owners returns the nodes that are part of the ring
and is not alive.
The token_metadata::get_all_endpoints returns nodes that are part of the
ring.
The patch changes both functions to use the more authoritative source to
get the nodes that are part of the ring and call is_alive to check if
the node is up or down. So that the correctness does not depend on
any derived information.
This patch fixes a truncate issue in storage_proxy::truncate_blocking
where it calls get_live_token_owners and get_unreachable_token_owners to
decide the nodes to talk with for truncate operation. The truncate
failed because incorrect nodes were returned.
Fixes#10296Fixes#11928Closes#11952
This PR is a follow-up to https://github.com/scylladb/scylladb/pull/11918.
With this PR:
- The "ScyllaDB Enterprise" label is added to all the features that are only available in ScyllaDB Enterprise.
- The previous Enterprise-only note is removed (it was included in multiple files as _/rst_include/enterprise-only-note.rst_ - this file is removed as it is no longer used anywhere in the docs).
- "Scylla Enterprise" was removed from `versionadded `because now it's clear that the feature was added for Enterprise.
Closes#11975
* github.com:scylladb/scylladb:
doc: remove the enterprise-only-note.rst file, which was replaced by the ScyllaDB Enterprise label and is not used anymore
doc: add the ScyllaDB Enterprise label to the descriptions of Enterprise-only features
The helper is already widely used, one (last) test case can benefit from using it too
Closes#11978
* github.com:scylladb/scylladb:
test: Indentation fix after previous patch
test: Wse with_sstable_directory() helper
We use Barrett tables (misspelled in the code unfortunately) to fold
crc computations of multiple buffers into a single crc. This is important
because it turns out to be faster to compute crc of three different buffers
in parallel rather than compute the crc of one large buffer, since the crc
instruction has latency 3.
Currently, we have a separate code generation step to compute the
fold tables. The step generates a new C++ source files with the tables.
But modern C++ allows us to do this computation at compile time, avoiding
the code generation step. This simplifies the build.
This series does that. There is some complication in that the code uses
compiler intrinsics for the computation, and these are not constexpr friendly.
So we first introduce constexpr-friendly alternatives and use them.
To prove the transformation is correct, I compared the generated code from
before the series and from just before the last step (where we use constexpr
evaluation but still retain the generated file) and saw no difference in the values.
Note that constexpr is not strictly needed - we could have run the code in the
global variables' initializer. But that would cause a crash if we run on a pre-clmul
machine, and is not as fun.
Closes#11957
* github.com:scylladb/scylladb:
test: crc: add unit tests for constexpr clmul and barrett fold
utils: crc combine table: generate at compile time
utils: barrett: inline functions in header
utils: crc combine table: generate tables at compile time
utils: crc combine table: extract table generation into a constexpr function
utils: crc combine table: extract "pow table" code into constexpr function
utils: crc combine table: store tables std::arrray rather than C array
utils: barrett: make the barrett reduction constexpr friendly
utils: clmul: add 64-bit constexpr clmul
utils: barrett: extract barrett reduction constants
utils: barrett: reorder functions
utils: make clmul() constexpr
It's already used everywhere, but one test case wires up the
sstable_directory by hand. Fix it too, but keep in mind, that the caller
fn stops the directory early.
(indentation is deliberately left broken)
Signed-off-by: Pavel Emelyanov <xemul@scylladb.com>
Currently, when replacing a node ip, keeping the old host,
we might end up with the the old endpoint in system.peers
if it is inserted back into the topology by `handle_state_normal`
when on_join is called with the old endpoint.
Then, later on, on_change sees that:
```
if (get_token_metadata().is_member(endpoint)) {
co_await do_update_system_peers_table(endpoint, state, value);
```
As described in #11925.
Fixes#11925Closes#11930
* github.com:scylladb/scylladb:
storage_service, system_keyspace: add debugging around system.peers update
storage_service: handle_state_normal: update topology and notify_joined endpoint only if not removed
Today, compaction_backlog_tracker is managed in each compaction_strategy
implementation. So every compaction strategy is managing its own
tracker and providing a reference to it through get_backlog_tracker().
But this prevents each group from having its own tracker, because
there's only a single compaction_strategy instance per table.
To remove this limitation, compaction_strategy impl will no longer
manage trackers but will instead provide an interface for trackers
to be created, such that each compaction_group will be allowed to
create its own tracker and manage it by itself.
Now table's backlog will be the sum of all compaction_group backlogs.
The normalization factor is applied on the sum, so we don't have
to adjust each individual backlog to any factor.
Closes#11762
* github.com:scylladb/scylladb:
replica: Allow one compaction_backlog_tracker for each compaction_group
compaction: Make compaction_state available for compaction tasks being stopped
compaction: Implement move assignment for compaction_backlog_tracker
compaction: Fix compaction_backlog_tracker move ctor
compaction: Use table_state's backlog tracker in compaction_read_monitor_generator
compaction: kill undefined get_unimplemented_backlog_tracker()
replica: Refactor table::set_compaction_strategy for multiple groups
Fix exception safety when transferring ongoing charges to new backlog tracker
replica: move_sstables_from_staging: Use tracker from group owning the SSTable
replica: Move table::backlog_tracker_adjust_charges() to compaction_group
replica: table::discard_sstables: Use compaction_group's backlog tracker
replica: Disable backlog tracker in compaction_group::stop()
replica: database_sstable_write_monitor: use compaction_group's backlog tracker
replica: Move table::do_add_sstable() to compaction_group
test/sstable_compaction_test: Switch to table_state::get_backlog_tracker()
compaction/table_state: Introduce get_backlog_tracker()
Check that the constexpr variants indeed match the runtime variants.
I verified manually that exactly one computation in each test is
executed at run time (and is compared against a constant).
By now the crc combine tables are generated at compile time,
but still in a separate code generation step. We now eliminate
the code generation step and instead link the global variables
directly into the main executable. The global variables have
been conveniently named exactly as the code generation step
names them, so we don't need to touch any users.
Move the tables into global constinit variables that are
generated at compile time. Note the code that creates
the generated crc32_combine_table.cc is still called; it
transorms compile-time generated tables into a C++ source
that contains the same values, as literals.
If we generate a diff between gen/utils/gz/crc_combine_table.cc
before this series and after this patch, we see the only change
in the file is the type of the variable (which changed to
std::array), proving our constexpr code is correct.
Move the code to a constexpr function, so we can later generate the tables at
compile time. Note that although the function is constexpr, it is still
evaluated at runtime, since the calling function (main()) isn't constexpr
itself.
A "pow table" is used to generate the Barrett fold tables. Extract its
code into a constexpr function so we can later generate the fold tables
at compile time.
C arrays cannot be returned from functions and therefore aren't suitable
for constexpr processing. std::array<> is a regular value and so is
constexpr friendly.
This is used when generating the Barrett reduction tables, and also when
applying the Barrett reduction at runtime, so we need it to be constexpr
friendly.
clmul() is a pure function and so should already be constexpr,
but it uses intrinsics that aren't defined as constexpr and
so the compiler can't really compute it at compile time.
Fix by defining a constexpr variant and dispatching based
on whether we're being constant-evaluated or not.
The implementation is simple, but in any case proof that it
is correct will be provided later on.
Today, compaction_backlog_tracker is managed in each compaction_strategy
implementation. So every compaction strategy is managing its own
tracker and providing a reference to it through get_backlog_tracker().
But this prevents each group from having its own tracker, because
there's only a single compaction_strategy instance per table.
To remove this limitation, compaction_strategy impl will no longer
manage trackers but will instead provide an interface for trackers
to be created, such that each compaction group will be allowed to
have its own tracker, which will be managed by compaction manager.
On compaction strategy change, table will update each group with
the new tracker, which is created using the previously introduced
ompaction_group_sstable_set_updater.
Now table's backlog will be the sum of all compaction_group backlogs.
The normalization factor is applied on the sum, so we don't have
to adjust each individual backlog to any factor.
Signed-off-by: Raphael S. Carvalho <raphaelsc@scylladb.com>
compaction_backlog_tracker will be managed by compaction_manager, in the
per table state. As compaction tasks can access the tracker throughout
its lifetime, remove() can only deregister the state once we're done
stopping all tasks which map to that state.
remove() extracted the state upfront, then performed the stop, to
prevent new tasks from being registered and left behind. But we can
avoid the leak of new tasks by only closing the gate, which waits
for all tasks (which are stopped a step earlier) and once closed,
prevents new tasks from being registered.
Signed-off-by: Raphael S. Carvalho <raphaelsc@scylladb.com>
Luckily it's not used anywhere. Default move ctor was picked but
it won't clear _manager of old object, meaning that its destructor
will incorrectly deregister the tracker from
compaction_backlog_manager.
Signed-off-by: Raphael S. Carvalho <raphaelsc@scylladb.com>
Refactoring the function for it to accomodate multiple compaction
groups.
To still provide strong exception guarantees, preparation and
execution of changes will be separated.
Once multiple groups are supported, each group will be prepared
first, and the noexcept execution will be done as a last step.
Signed-off-by: Raphael S. Carvalho <raphaelsc@scylladb.com>
When setting a new strategy, the charges of old tracker is transferred
to the new one.
The problem is that we're not reverting changes if exception is
triggered before the new strategy is successfully set.
To fix this exception safety issue, let's copy the charges instead
of moving them. If exception is triggered, the old tracker is still
the one used and remain intact.
Signed-off-by: Raphael S. Carvalho <raphaelsc@scylladb.com>
When moving SSTables from staging directory, we'll conditionally add
them to backlog tracker. As each group has its own tracker, a given
sstable will be added to the tracker of the group that owns it.
Signed-off-by: Raphael S. Carvalho <raphaelsc@scylladb.com>
Procedures that call this function happen to be in compaction_group,
so let's move it to group. Simplifies the change where the procedure
retrieves tracker from the group itself.
Signed-off-by: Raphael S. Carvalho <raphaelsc@scylladb.com>
As we're moving backlog tracker to compaction group, we need to
stop the tracker there too. We're moving it a step earlier in
table::stop(), before sstables are cleared, but that's okay
because it's still done after the group was deregistered
from compaction manager, meaning no compactions are running.
Signed-off-by: Raphael S. Carvalho <raphaelsc@scylladb.com>
All callers of do_add_sstable() live in compaction_group, so it
should be moved into compaction_group too. It also makes easier
for the function to retrieve the backlog tracker from the group.
Signed-off-by: Raphael S. Carvalho <raphaelsc@scylladb.com>
This interface will be helpful for allowing replica::table, unit
tests and sstables::compaction to access the compaction group's tracker
which will be managed by the compaction manager, once we complete
the decoupling work.
Signed-off-by: Raphael S. Carvalho <raphaelsc@scylladb.com>
In commit 544ef2caf3 we fixed a bug where
a reveresed clustering-key order caused problems using a secondary index
because of incorrect type comparison. That commit also included a
regression test for this fix.
However, that fix was incomplete, and improved later in commit
c8653d1321. That later fix was labeled
"better safe than sorry", and did not include a test demonstrating
any actual bug, so unsurprisingly we never backported that second
fix to any older branches.
Recently we discovered that missing the second patch does cause real
problems, and this patch includes a test which fails when the first
patch is in, but the second patch isn't (and passes when both patches
are in, and also passes on Cassandra).
Signed-off-by: Nadav Har'El <nyh@scylladb.com>
Closes#11943
Instead of using assigned IP addresses, use a local integer ID for
managing servers. IP address can be reused by a different server.
While there, get host ID (UUID). This can also be reused with `node
replace` so it's not good enough for tracking.
Closes#11747
* github.com:scylladb/scylladb:
test.py: use internal id to manage servers
test.py: rename hostname to ip_addr
test.py: get host id
test.py: use REST api client in ScyllaCluster
test.py: remove unnecessary reference to web app
test.py: requests without aiohttp ClientSession
Instead of using assigned IP addresses, use an internal server id.
Define types to distinguish local server id, host ID (UUID), and IP
address.
This is needed to test servers changing IP address and for node replace
(host UUID).
Signed-off-by: Alejo Sanchez <alejo.sanchez@scylladb.com>
The code explicitly manages an IP as string, make it explicit in the
variable name.
Define its type and test for set in the instance instead of using an
empty string as placeholder.
Signed-off-by: Alejo Sanchez <alejo.sanchez@scylladb.com>
When initializing a ScyllaServer, try to get the host id instead of only
checking the REST API is up.
Use the existing aiohttp session from ScyllaCluster.
In case of HTTP error check the status was not an internal error (500+).
Signed-off-by: Alejo Sanchez <alejo.sanchez@scylladb.com>
Move the REST api client to ScyllaCluster. This will allow the cluster
to query its own servers.
Signed-off-by: Alejo Sanchez <alejo.sanchez@scylladb.com>
