Commit Graph

6075 Commits

Author SHA1 Message Date
Kefu Chai
cf932888de Update seastar submodule
* seastar e0d515b6...70349b74 (33):
  > util/log: drop unused function
  > util/log, rpc, core: use compile-time formatting with fmtlib >= 8.0
  > Fix edge case in memory sampler at OOM
  > exp/geo distribution benchmark
  > Additional allocation tests
  > Remove null pointer check on free hot path
  > Optimize final part of allocation hot path
  > Optimize zero size checking in allocator
  > memory: Optimize free fast path
  > memory: Optimize small alloc alloation path
  > memory: Limit alloc_sites size
  > memory: Add general comment about sampling strategy
  > memory: Use probabilistic sampler
  > util: Adapt memory sampler to seastar
  > util: Import Android Memory Sampler
  > memory: Use separate small pool for tracking sampled allocations
  > memory: Support enabling memory profiling at runtime
  > util/source_location-compat: mark `source_location::current()` consteval
  > build: use new behavior defined by CMP0155 when building C++ modules
  > circleci: build with C++20 modules enabled
  > seastar.cc: replace cryptopp with gnutls when building seastar modules
  > alien: include used header
  > seastar.cc: include used headers in the global purview
  > docker: install clang-tools-17
  > net/tcp: generate a random src_port hashed to current shard if smp::count > 1
  > net, websocket: replace Crypto++ calls with GnuTLS
  > README-DPDK.md: point user to DPDK's quick start guide
  > reactor: print fatal error using logger as well
  > Avoid ping-pong in spinlock::lock
  > memory: Add allocator perf tests
  > memory: Add a basic sized deletion test
  > Prometheus: Disable Prometheus protobuf with a configuration
  > treewide: bring back prometheus protobuf support
* test/manual/sstable_scan_footprint_test: update to adapt to the
  breaking change of "memory: Use probabilistic sampler" in seastar

Closes scylladb/scylladb#16610
2024-01-04 09:36:53 +02:00
Kefu Chai
50cf62e186 build: cmake: do not link against Boost::dynamic_linking
Boost::dynamic_linking was introduced as a compatibility target
which adds "BOOST_ALL_DYN_LINK" macro on Win32 platform. but since
Scylla only runs on Linux, there is no need to link against this
library.

Signed-off-by: Kefu Chai <kefu.chai@scylladb.com>

Closes scylladb/scylladb#16544
2024-01-04 08:06:19 +02:00
Kefu Chai
2ad532df43 test: randomized_nemesis_test: move std::variant formatter up
we format `std::variant<std::monostate, seastar::timed_out_error,
raft::not_a_leader, raft::dropped_entry, raft::commit_status_unknown,
raft::conf_change_in_progress, raft::stopped_error, raft::not_a_member>`
in this source file. and currently, we format `std::variant<..>` using
the default-generated `fmt::formatter` from `operator<<`, so in order to
format it using {fmt}'s compile-time check enabled, we have to make the
`operator<<` overload for `std::variant<...>` visible from the caller
sites which format `std::variant<...>` using {fmt}.

in this change, the `operator<<` for `std::variant<...>` is moved to
from the middle of the source file to the top of it, so that it can
be found when the compiler looks up for a matched `fmt::formatter`
for `std::variant<...>`.

please note, we cannot use the `fmt::formatter` provided by `fmt/std.h`,
as its specialization for `std::variant` requires that all the types
of the variant is `is_formattable`. but the default generated formatter
for type `T` is not considered as the proof that `T` is formattable.

this should address the FTBFS with the latest seastar like:

```
 /usr/include/fmt/core.h:2743:12: error: call to deleted constructor of 'conditional_t<has_formatter<mapped_type, context>::value, formatter<mapped_type, char_type>, fallback_formatter<stripped_type, char_type>>' (aka 'fmt::detail::fallback_formatter<std::variant<std::monostate, seastar::timed_out_error, raft::not_a_leader, raft::dropped_entry, raft::commit_status_unknown, raft::conf_change_in_progress, raft::stopped_error, raft::not_a_member>>')
```

Signed-off-by: Kefu Chai <kefu.chai@scylladb.com>

Closes scylladb/scylladb#16616
2024-01-03 16:38:25 +01:00
Avi Kivity
20531872a7 Merge 'test: randomized_nemesis_test: add formatter for append_entry' from Kefu Chai
we are using `seastar::format()` to format `append_entry` in
`append_reg_model`, so we have to provide a `fmt::formatter` for
these callers which format `append_entry`.

despite that, with FMT_DEPRECATED_OSTREAM, the formatter is defined
by fmt v9, we don't have it since fmt v10. so this change prepares us
for fmt v10.

Refs https://github.com/scylladb/scylladb/issues/13245

Closes scylladb/scylladb#16614

* github.com:scylladb/scylladb:
  test: randomized_nemesis_test: add formatter for append_entry
  test: randomized_nemesis_test: move append_reg_model::entry out
2024-01-03 15:06:33 +02:00
Tomasz Grabiec
715e062d4a Merge 'table, memtable: share log structured allocator statistics across all tablets in a table' from Avi Kivity
In 7d5e22b43b ("replica: memtable: don't forget memtable
memory allocation statistics") we taught memtable_list to remember
learned memory allocation reserves so a new memtable inherits these
statistics from an older memtable. Share it now further across tablets
that belong to the same table as well. This helps the statistics be more
accurate for tablets that are migrated in, as they can share existing
tablet's memory allocation history.

Closes scylladb/scylladb#16571

* github.com:scylladb/scylladb:
  table, memtable: share log-structured allocator statistics across all memtables in a table
  memtable: consolidate _read_section, _allocating_section in a struct
2024-01-03 14:03:40 +01:00
Kefu Chai
3ef0345b7f test/nodetool: log response from mock server when handling JSONDecodeError
it's observed that the mock server could return something not decodable
as JSON. so let's print out the response in the logging message in this case.
this should help us to understand the test failure better if it surfaces again.

Refs #16542
Signed-off-by: Kefu Chai <kefu.chai@scylladb.com>

Closes scylladb/scylladb#16543
2024-01-03 14:46:10 +02:00
Kefu Chai
0484ac46af test: randomized_nemesis_test: add formatter for append_entry
we are using `seastar::format()` to format `append_entry` in
`append_reg_model`, so we have to provide a `fmt::formatter` for
these callers which format `append_entry`.

despite that, with FMT_DEPRECATED_OSTREAM, the formatter is defined
by fmt v9, we don't have it since fmt v10. so this change prepares us
for fmt v10.

Refs #13245
Signed-off-by: Kefu Chai <kefu.chai@scylladb.com>
2024-01-03 08:38:43 +08:00
Kefu Chai
32e55731ab test: randomized_nemesis_test: move append_reg_model::entry out
this change prepares for adding fmt::formatter for append_entry.
as we are using its formatter in the inline member functions of
`append_reg_model`. but its `fmt::formatter` can only be specialized out of
this class. and we don't have access to `format_as()` yet in {fmt} 9.1.0
which is shipped along with fedora38, which is in turn used for
our base build image.

so, in this change, `append_reg_model::entry` is extracted and renamed
to `append_entry`.

Signed-off-by: Kefu Chai <kefu.chai@scylladb.com>
2024-01-03 08:38:43 +08:00
Sylwia Szunejko
91a5a41313 add a way to negotiate generation of the tablet info for drivers
Tablets metadata is quite expensive to generate (each data_value is
an allocation), so an old driver (without support for tablets) will
generate huge amounts of such notifications. This commit adds a way
to negotiate generation of the notification: a new driver will ask
for them, and an old driver won't get them. It uses the
OPTIONS/SUPPORTED/STARTUP protocol described in native_protocol_v4.spec.

Closes scylladb/scylladb#16611
2024-01-02 20:00:50 +02:00
Kamil Braun
7f6955b883 Merge 'test: make use of concurrent bootstrap' from Patryk Jędrzejczak
In #16102, we added a test for concurrent bootstrap in the raft-based
topology. This test was running in CI for some time and
never failed. Now, we can believe that concurrent bootstrap is not
bugged or at least the probability of a failure is very low. Therefore,
we can safely make use of it in all tests using the raft-based topology.

This PR:
- makes all initial servers start concurrently in topology tests,
- replaces all multiple `server_add` calls with a single `servers_add`
  call in tests using the raft-based topology,
- removes no longer needed `test_concurrent_bootstrap`.

The changes listed above:
- make running tests a bit faster due to concurrent bootstraps,
- make multiple tests test concurrent bootstrap previously tested by
  a single test.

Fixes scylladb/scylladb#15423

Closes scylladb/scylladb#16384

* github.com:scylladb/scylladb:
  test: test_different_group0_ids: fix comments
  test: remove test_concurrent_bootstrap
  test: replace multiple server_add calls with servers_add
  test: ScyllaCluster: start all initial servers concurrently
  test: ManagerClient: servers_add: specify consistent-topology-changes assumption
2024-01-02 15:11:18 +01:00
Sylwia Szunejko
467d466f7e put all tablet info into one field of custom_payload and update docs
Previously, the tablet information was sent to the drivers
in two pieces within the custom_payload. We had information
about the replicas under the `tablet_replicas` key and token range
information under `token_range`. These names were quite generic
and might have caused problems for other custom_payload users.
Additionally, dividing the information into two pieces raised
the question of what to do if one key is present while the other
is missing.

This commit changes the serialization mechanism to pack all information
under one specific name, `tablets-routing-v1`.

From: Sylwia Szunejko <sylwia.szunejko@scylladb.com>

Closes scylladb/scylladb#16148
2024-01-02 14:35:37 +02:00
Patryk Jędrzejczak
215534d527 test: test_different_group0_ids: fix comments
The test disables consistent topology changes, not cluster
management.
2024-01-02 12:19:33 +01:00
Patryk Jędrzejczak
466723a74f test: remove test_concurrent_bootstrap
This test only adds 3 nodes concurrently to the empty cluster.
After making many other tests use ManagerClient.servers_add, it
serves no purpose.

We had added this test before we decided to use
ManagerClient.servers_add in many tests to avoid multiple failures
in CI if it turned out that the concurrent bootstrap is flaky with
high frequency there. This test was running in CI for some time and
never failed. Now, we can believe that concurrent bootstrap is not
bugged or at least the probability of a failure is very low.
2024-01-02 12:19:33 +01:00
Patryk Jędrzejczak
a8513bd41b test: replace multiple server_add calls with servers_add
ManagerClient.servers_add can be used in every test that uses
consistent topology changes. We replace all multiple server_add
calls in such tests with a single servers_add call to make these
tests faster and simplify their code. Additionally, these
servers_add calls will test concurrent bootstraps for free.
2024-01-02 12:19:33 +01:00
Patryk Jędrzejczak
debf1db3ef test: ScyllaCluster: start all initial servers concurrently
Starting all initial servers concurrently makes tests in suites
with initial_size > 1 run a bit faster. Additionally, these tests
test concurrent bootstraps for free.

add_servers can be called only if the cluster uses consistent
topology changes. We can use this function unconditionally in
install_and_start because every suite uses consistent topology
changes by default. The only way to not use it is by adding all
servers with a config that contains experimental_features without
consistent-topology-changes.
2024-01-02 12:19:33 +01:00
Patryk Jędrzejczak
16b0eeb3d6 test: ManagerClient: servers_add: specify consistent-topology-changes assumption
ManagerClient.servers_add can be called only if the cluster uses
consistent topology changes. We add this specification to the
leading comment.
2024-01-02 12:19:31 +01:00
Benny Halevy
5abf556399 gms: endpoint_state: define application_state_map
Have a central definition for the map held
in the endpoint_state (before changing it to
std::unordered_map).

Signed-off-by: Benny Halevy <bhalevy@scylladb.com>
2023-12-31 18:37:34 +02:00
Nadav Har'El
91636f6d21 test/cql-pytest: reproducer of slightly too strict parser of timestamp
Scylla refuses the timestamp format "2014-01-01 12:15:45.0000000Z" that
has 6 digits of precision for the fractional second, and only allows
3 digits of precision. This restriction makes sense - after all CQL
timestamp columns (note - this is NOT "using timestamp"!) only have
millisecond precision. Nevertheless, Cassandra does not have this
restriction and does allow these over-precise timestamps. In this patch
we add a test that demonstrates this difference.

Curiously, in the past Scylla *generated* this forbidden timestamp
format when outputting the timestamp to a string (e.g. toJson()),
which it then couldn't read back! This was issue #16575.
Today Scylla no longer generates this forbidden timestamp format.

Signed-off-by: Nadav Har'El <nyh@scylladb.com>

Closes scylladb/scylladb#16576
2023-12-28 19:01:25 +02:00
Avi Kivity
6394854f04 Merge 'Some cleanups in tests for tablets + MV ' from Nadav Har'El
This small series improves two things in the multi-node tests for tablet supports in materialized views:

1. The test for Alternator LSI, which "sometimes" could reproduce the bug by creating 10-node cluster with a random tablet distribution, is replaced by a reliable 2-node cluster which controls the tablet distribution. The new test also confirms that tablets are actually enabled in Alternator (reviewers of the original test noted it would be easy to pass the test if tablets were accidentally not enabled... :-)).
2. Simplify the tablet lookup code in the test to not go through a "table id", and lookup the table's (or view's) name directly (requires a full-table of the tablets table, but that's entirely reasonable in a test).

The third patch in this series also fixes a comment typo discovered in a previous review.

Closes scylladb/scylladb#16440

* github.com:scylladb/scylladb:
  materialized views: fix typo in comment
  test_mv_tablets: simplify lookup of tablets
  alternator, tablets: improve Alternator LSI tablets test
2023-12-27 20:18:14 +02:00
Avi Kivity
3ce0576a31 Merge 'Sanitize keyspace_metadata creation' from Pavel Emelyanov
The amount of arguments needed to create ks metadata object is pretty large and there are many different ways it can be and it is created over the code. This set simplifies it for the most typical patterns.

closes: #16447
closes: #16449

Closes scylladb/scylladb#16565

* github.com:scylladb/scylladb:
  schema_tables: Use new_keyspace() sugar
  keyspace_metadata: Drop vector-of-schemas argument from new_keyspace()
  keyspace_metadata: Add default value for new_keyspace's durable_writes
  keyspace_metadata: Pack constructors with default arguments
2023-12-27 17:15:04 +02:00
Botond Dénes
1647b29cba tools/schema_loader: add db::config parameter to all load methods
So that a single centrally managed db::config instance can be shared by
all code requiring it, instead of creating local instances where needed.
This is required to load schema from encrypted schema-tables, and it
also helps memory consumption a bit (db::config consumes a lot of
memory).

Fixes: #16480

Closes scylladb/scylladb#16495
2023-12-27 16:28:38 +02:00
Nadav Har'El
e6dc9bca0d Merge 'Profile dumping rest api support' from Eliran Sinvani
This change is motivated by wanting to have code coverage reporting support.
Currently the only way to get a profile dump in ScyllaDB is stopping it with SIGTERM, however, this doesn't
suite all cases, more specifically:
1. In dtest, when some of the tests intentionally abruptly kill a node
2. In test.py, where we would like to distinguish (at least for now), graceful shutdown of ScyllaDB testing and
teardown procedures (which currently kills the nodes).

This mini series adds two changes:
1. It adds the support for profile dumping in ScyllaDB with rest api ('/system/dump_profile')
2. It adds the support for this API in test.py and also adds a call for it as part of the node stop procedure in a permissive way that will not fail the teardown or test if the call doesn't succeed for whatever reason - after this change, all current
test.py suits except for pylib_test (expected) dumps profiles if instrumented and will be able to participate in coverage
reporting.

Refs #16323

Closes scylladb/scylladb#16557

* github.com:scylladb/scylladb:
  test.py: Dump coverage profile before killing a node
  rest api: Add an api for profile dumping
2023-12-27 12:06:39 +02:00
Eliran Sinvani
e49b3ffc89 test.py: Dump coverage profile before killing a node
Up until now the only way to get a coverage profile was to shut down the
ScyllaDB nodes gracefully (using SIGTERM), this means that the coverage
profile was lost for every node that was killed abruptly (SIGKILL).
This in turn would have been requiring us to shut down all nodes
gracefully which is not something we set out to do.
Here we use the rest API for dumping the coverage profile which will
cause the most minimal impact possible on the test runs.
If the dumping fails (due to the node doesn't support the API or due to
a real error in dumping we ignore it as it is not part of the system we
would like to test.

Signed-off-by: Eliran Sinvani <eliransin@scylladb.com>
2023-12-27 07:17:26 +02:00
Avi Kivity
02111d6754 memtable: consolidate _read_section, _allocating_section in a struct
Those two members are passed from memtable_list to memtable. Since we
wish to pass them from table, it becomes awkward to pass them as two
separate variables as their contents are specific to memtable internals.

Wrap them in a name that indicates their role (being table-wide shared
data for memtables) and pass them as a unit.
2023-12-26 21:11:48 +02:00
Nadav Har'El
fc71c34597 Merge 'select statement: verify EXECUTE permissions only for non native functions' from Eliran Sinvani
Commit 62458b8e4f introduced the enforcement of EXECUTE permissions of functions in cql select. However, according to the reference in #12869, the permissions should be enforced only on UDFs and UDAs.
The code does not distinguish between the two so the permissions are also unintenionally enforced also on native function. This commit introduce the distinction and only enforces the permissions on non native functions.

Fixes #16526

Manually verified (before and after change) with the reproducer supplied in #16526 and also with some the `min` and `max` native functions.
Also added test that checks for regression on native functions execution and verified that it fails on authorization before
the fix and passes after the fix.

Closes scylladb/scylladb#16556

* github.com:scylladb/scylladb:
  test.py: Add test for native functions permissions
  select statement: verify EXECUTE permissions only for non native functions
2023-12-26 18:14:21 +02:00
Pavel Emelyanov
a1ad2571fc keyspace_metadata: Drop vector-of-schemas argument from new_keyspace()
It's only testing code that wants to call new_keyspace with existing
schemas, all the other callers either construct the ks metadata
directly, or use convenience new_keyspace with explicitly empty schemas.
By and large it's nicer if new_keyspace() doesn't requires this
argument.

Signed-off-by: Pavel Emelyanov <xemul@scylladb.com>
2023-12-26 13:00:44 +03:00
Eliran Sinvani
a336550041 test.py: Add test for native functions permissions
Native functions (non UDF/UDA functions), should be usable even if a
user is not granted EXECUTE permissions on them.

This is a regression test that was added following:
https://github.com/scylladb/scylladb/issues/16526

Signed-off-by: Eliran Sinvani <eliransin@scylladb.com>
2023-12-26 10:27:04 +02:00
Avi Kivity
3968fc11bf Merge 'cql: fix regression in SELECT * GROUP BY' from Nadav Har'El
This short series fixes a regression from Scylla 5.2 to Scylla 5.4 in "SELECT * GROUP BY" - this query was supposed to return just a single row from each partition (the first one in clustering order), but after the expression rewrite started to wrongly return all rows.

The series also includes a regression test that verifies that this query works doesn't work correctly before this series, but works with this patch - and also works as expected in Scylla 5.2 and in Cassadra.

Fixes #16531.

Closes scylladb/scylladb#16559

* github.com:scylladb/scylladb:
  test/cql-pytest: check that most aggregators don't take "*"
  cql-pytest: add reproducer for GROUP BY regression
  cql: fix regression in SELECT * GROUP BY
2023-12-25 19:53:55 +02:00
Pavel Emelyanov
ac3dd4bf5d test: Coroutinize some secondary_index_test cases
Now they are long then-chains that are hard to read

Signed-off-by: Pavel Emelyanov <xemul@scylladb.com>

Closes scylladb/scylladb#16547
2023-12-25 18:08:19 +02:00
Nadav Har'El
55317666c6 test/cql-pytest: check that most aggregators don't take "*"
Although you can "SELECT COUNT(*)", this has special handling in the CQL
parser (it is converted into a special row-counting request) and you can't
give "*" to other aggregators - e.g., "SELECT SUM(*)". This patch includes
a simple test that confirms this.

I wanted to check this in relation to the previous patch, which did,
sort of, a "SELECT $$first$$(*)" - a syntax which this test shows
wouldn't have actually worked if we tried it.

Signed-off-by: Nadav Har'El <nyh@scylladb.com>
2023-12-25 17:53:42 +02:00
Nadav Har'El
e2773b4a3a cql-pytest: add reproducer for GROUP BY regression
test/cql-pytest/test_group_by.py has tests that verifies that requests
like

   SELECT p,c1,c2,v FROM tbl WHERE p=0 GROUP BY p

work as expected - the "GROUP BY p" means in this case that we should
only return the first row in the p=0 partition.

As a user discovered, it turns out that the almost identical request:

   SELECT * FROM tbl WHERE p=0 GROUP BY p

Doesn't work the same - before the fix in the previous patch, it
erroneously returned all rows in p=0, not just the first one.
The test in this patch demonstrates this - it fails on Scylla 5.4,
passes on Scylla 5.2 and on Cassandra - and passes when the fix
from the previous patch is used.

This patch includes another tiny test, to check the interaction of GROUP BY
with filtering. This second test passes on Scylla - but I want it in
anyway because it is yet another interaction that might break (the
user that reported #16531 also had filtering, and I was worried it might
have been related).

Refs #16531

Signed-off-by: Nadav Har'El <nyh@scylladb.com>
2023-12-25 17:53:42 +02:00
Pavel Emelyanov
1d2c871219 test: Add sanity tests for tablets initialization and altering
Check that the initial_tablets appears in system_schema.scylla_keyspaces
if turned on explicitly

Check that it's possible to change initial_tablets with ALTER KEYSPACE

Check that changing r.s. from simple to network-topology doesn't
activate tablets

Signed-off-by: Pavel Emelyanov <xemul@scylladb.com>
2023-12-25 16:09:01 +03:00
Pavel Emelyanov
c43501d973 locator,schema: Move initial tablets from r.s. options to params
The option is kepd in DDL, but is _not_ stored in
system_schema.keyspaces. Instead, it's removed from the provided options
and kept in scylla_keyspaces table in its own column. All the places
that had optional initial_tablets disengaged now set this value up the
way the find appropriate.

Signed-off-by: Pavel Emelyanov <xemul@scylladb.com>
2023-12-25 16:07:10 +03:00
Pavel Emelyanov
562fcf0c19 locator: Keep optional initial_tablets on r.s. params
Now all the callers have it at hands (spoiler: not yet initialized, but
still) so the params can also have it.

Signed-off-by: Pavel Emelyanov <xemul@scylladb.com>
2023-12-25 16:02:41 +03:00
Pavel Emelyanov
a67c535539 keyspace_metadata: Carry optional<initial_tablets> on board
The object in question fully describes the keyspace to be created and,
among other things, contains replication strategy options. Next patches
move the "initial_tablets" option out of those options and keep it
separately, so the ks metadata should also carry this option separately.

This patch is _just_ extending the metadata creation API, in fact the
new field is unused (write-only) so all the places that need to provide
this data keep it disengaged and are explicitly marked with FIXME
comment. Next patches will fix that.

Signed-off-by: Pavel Emelyanov <xemul@scylladb.com>
2023-12-25 15:58:05 +03:00
Pavel Emelyanov
a943bd927b locator: Call create_replication_strategy() with r.s. params
Previous patch added params to r.s. classes' constructors, but callers
don't construct those directly, instead they use the create_r.s.()
wrapper. This patch adds params to the wrapper too.

Signed-off-by: Pavel Emelyanov <xemul@scylladb.com>
2023-12-25 15:54:59 +03:00
Pavel Emelyanov
f88ba0bf5a locator: Wrap replication_strategy_config_options into replication_strategy_params
When replication strategy class is created caller parr const reference
on the config options which is, in turn, a map<string, string>. In the
future r.s. classes will need to get "scylla specific" info along with
legacy options and this patch prepares for that by passing more generic
params argument into constructor. Currently the only inhabitant of the
new params is the legacy options.

Signed-off-by: Pavel Emelyanov <xemul@scylladb.com>
2023-12-25 15:53:03 +03:00
Nadav Har'El
f9f20e779c test_mv_tablets: simplify lookup of tablets
The tests looked up a table's tablets in an elaborate two-stage search -
first find the table's "id", and then look up this id in the list of
tablets. It is much simpler to just look up the table's name directly
in the list of tablets - although this name is not a key, an ALLOW
FILTERING search is good enough for a test.

As a bonus, with the new technique we don't care if the given name
is the name of a table or a view, further simplifying the test.

This is just a test code cleanup - there is no functional change in
the test.

Signed-off-by: Nadav Har'El <nyh@scylladb.com>
2023-12-24 10:12:44 +02:00
Nadav Har'El
cdd5b19f12 alternator, tablets: improve Alternator LSI tablets test
The test test_tablet_alternator_lsi_consistency, checking that Alternator
LSI allow strongly-consistent reads even with tablets, used a large
cluster (10 nodes), to improve the chance of reaching an "unlucky" tablet
placement - and even then only failed in about half the runs without
the code fixed.

In this patch, we rewrite the test using a much more reliable approach:
We start only two nodes, and force the base's tablet onto one node, and
the view's table onto the second node. This ensures with 100% certainty
that the view update is remote, and the new test fails every single time
before the code fix (I reverted the fix to verify) - and passes after it.

The new test is not only more reliable, it's also significantly faster
because it doesn't need to start a 10-node cluster.

We can also remove the tag that excluded this test from debug build
mode tests because the 10-node boot was too slow.

Signed-off-by: Nadav Har'El <nyh@scylladb.com>
2023-12-24 10:11:43 +02:00
Tomasz Grabiec
2590274f95 Merge 'Don't allow ALTER KEYSPACE to change replication strategy vnode/per-table flavor' from Pavel Emelyanov
This switch is currently possible, but results in not supported keyspace state

Closes scylladb/scylladb#16513

* github.com:scylladb/scylladb:
  test: Add a test that switching between vnodes and tablets is banned
  cql3/statements: Don't allow switching between vnode and per-table replication strategies
  cql3/statements: Keep local keyspace variable in alter_keyspace_statement::validate
2023-12-22 17:22:36 +01:00
Kefu Chai
642652efab test/cql-pytest/test_tools.py: test shard-of with a single partition
test_scylla_sstable_shard_of takes lots of time preparing the keys for a
certain shard. with the debug build, it takes 3 minutes to complete the
test.

so in order to test the "shard-of" subcommand in an more efficient way,
in this change, we improve the test in two ways:

1. cache the output of 'scylla types shardof`. so we can avoid the
   overhead of running a seastar application repeatly for the
   same keys.
2. reduce the number of partitions from 42 to 1. as the number of
   partitions in an sstable does not matter when testing the
   output of "shard-of" command of a certain sstable. because,
   the sstable is always generated by a certain shard.

before this change, with pytest-profiling:

```
   ncalls  tottime  percall  cumtime  percall filename:lineno(function)
      4/3    0.000    0.000  181.950   60.650 runner.py:219(call_and_report)
      4/3    0.000    0.000  181.948   60.649 runner.py:247(call_runtest_hook)
      4/3    0.000    0.000  181.948   60.649 runner.py:318(from_call)
      4/3    0.000    0.000  181.948   60.649 runner.py:262(<lambda>)
    44/11    0.000    0.000  181.935   16.540 _hooks.py:427(__call__)
    43/11    0.000    0.000  181.935   16.540 _manager.py:103(_hookexec)
    43/11    0.000    0.000  181.935   16.540 _callers.py:30(_multicall)
      361    0.001    0.000  181.531    0.503 contextlib.py:141(__exit__)
   782/81    0.001    0.000  177.578    2.192 {built-in method builtins.next}
     1044    0.006    0.000   92.452    0.089 base_events.py:1894(_run_once)
       11    0.000    0.000   91.129    8.284 fixtures.py:686(<lambda>)
    17/11    0.000    0.000   91.129    8.284 fixtures.py:1025(finish)
        4    0.000    0.000   91.128   22.782 fixtures.py:913(_teardown_yield_fixture)
      2/1    0.000    0.000   91.055   91.055 runner.py:111(pytest_runtest_protocol)
      2/1    0.000    0.000   91.055   91.055 runner.py:119(runtestprotocol)
        2    0.000    0.000   91.052   45.526 conftest.py:50(cql)
        2    0.000    0.000   91.040   45.520 util.py:161(cql_session)
        1    0.000    0.000   91.040   91.040 runner.py:180(pytest_runtest_teardown)
        1    0.000    0.000   91.040   91.040 runner.py:509(teardown_exact)
     1945    0.002    0.000   90.722    0.047 events.py:82(_run)
```

after this change:
```
   ncalls  tottime  percall  cumtime  percall filename:lineno(function)
      4/3    0.000    0.000    8.271    2.757 runner.py:219(call_and_report)
    44/11    0.000    0.000    8.270    0.752 _hooks.py:427(__call__)
    44/11    0.000    0.000    8.270    0.752 _manager.py:103(_hookexec)
    44/11    0.000    0.000    8.270    0.752 _callers.py:30(_multicall)
      4/3    0.000    0.000    8.269    2.756 runner.py:247(call_runtest_hook)
      4/3    0.000    0.000    8.269    2.756 runner.py:318(from_call)
      4/3    0.000    0.000    8.269    2.756 runner.py:262(<lambda>)
       48    0.000    0.000    8.269    0.172 {method 'send' of 'generator' objects}
       27    0.000    0.000    5.671    0.210 contextlib.py:141(__exit__)
       11    0.000    0.000    4.297    0.391 fixtures.py:686(<lambda>)
      2/1    0.000    0.000    4.228    4.228 runner.py:111(pytest_runtest_protocol)
      2/1    0.000    0.000    4.228    4.228 runner.py:119(runtestprotocol)
        2    0.000    0.000    4.213    2.106 capture.py:877(pytest_runtest_teardown)
        1    0.000    0.000    4.213    4.213 runner.py:180(pytest_runtest_teardown)
        1    0.000    0.000    4.213    4.213 runner.py:509(teardown_exact)
        2    0.000    0.000    3.628    1.814 capture.py:872(pytest_runtest_call)
        1    0.000    0.000    3.627    3.627 runner.py:160(pytest_runtest_call)
        1    0.000    0.000    3.627    3.627 python.py:1797(runtest)
   114/81    0.001    0.000    3.505    0.043 {built-in method builtins.next}
       15    0.784    0.052    3.183    0.212 subprocess.py:417(check_output)
```

Fixes #16516
Signed-off-by: Kefu Chai <kefu.chai@scylladb.com>

Closes scylladb/scylladb#16523
2023-12-22 15:20:03 +02:00
Tomasz Grabiec
9c7e5f6277 Merge 'Fix secondary index feature with tablets' from Nadav Har'El
Before this series, materialized views already work correctly on keyspaces with tablets, but secondary indexes do not. The goal of these series is make CQL secondary indexes fully supported on tablets:

1. First we need to make CREATE INDEX work with tablets (it didn't before this series). Fixes #16396.
2. Then we need to keep the promise that our documentation makes - that **local** secondary index should be synchronously updated - Fixes #16371.

As you can see in the patches below, and as was expected already in the design phase, the code changes needed to make indexes support tablets were minimal. But writing reliable tests for these issues was the biggest effort that went into this series.

Closes scylladb/scylladb#16436

* github.com:scylladb/scylladb:
  secondary-index, tablets: ensure that LSI are synchronous
  test: add missing "tags" schema extension to cql_test_env
  mv, test: fix delay_before_remote_view_update injection point
  secondary index: fix view creation when using tablets
2023-12-21 23:37:00 +01:00
Botond Dénes
1ce07c6f27 test/cql-pytest: test_select_from_mutation_fragments: bump timeout for test_many_partitions
The test test_many_partitions is very slow, as it tests a slow scan over
a lot of partitions. This was observed to time out on the slower ARM
machines, making the test flaky. To prevent this, create an
extra-patient cql connection with a 10 minutes timeout for the scan
itself.
This is a follow-up to fb9379edf1, which
attempted to fix this, but didn't patch all the places doing slow scans.
This patch fixes the other scan, the one actually observed to time-out
in CI.

Fixes: #16145

Closes scylladb/scylladb#16370
2023-12-21 19:55:06 +02:00
Pavel Emelyanov
a03755d6d7 test: Add a test that switching between vnodes and tablets is banned
Signed-off-by: Pavel Emelyanov <xemul@scylladb.com>
2023-12-21 19:57:55 +03:00
Nadav Har'El
a41140f569 Merge 'scylla-sstable: handle attempt to load schema for non-existent tables more gracefully' from Botond Dénes
In other words, print more user-friendly messages, and avoid crashing.
Specifically:
* Don't crash when attempting to load schema tables from configured data-dir, while configuration does not have any configured data-directories.
* Detect the case where schema mutations have no rows for the current table -- the keyspace exists, but the table doesn't.
* Add negative tests for schema-loading.

Fixes: https://github.com/scylladb/scylladb/issues/16459

Closes scylladb/scylladb#16494

* github.com:scylladb/scylladb:
  test/cql-pytest: test_tools.py: add test for failed schema loadig
  tools/scylla-sstable: use at() instead of operator [] when obtaining data dirs
  tools/schema_loader: also check for empty table/column mutations
  tools/schema_loader: log more details when loading schema from schema tables
2023-12-21 15:40:51 +02:00
Nadav Har'El
a613a3cad2 secondary-index, tablets: ensure that LSI are synchronous
CQL Local Secondary Index is a Scylla-only extension to Cassandra's
secondary index API where the index is separate per partition.
Scylla's documentation guarantees that:

  "As of Scylla Open Source 4.0, updates for local secondary indexes are
   performed synchronously. When updates are synchronous, the client
   acknowledges the write operation only after both the base table
   modification and the view up date are written."

This happened automatically with vnodes, because the base table and the
view have the same partition key, so base and view replicas are co-located,
and the view update is always local and therefore done synchronously.

But with tablets, this does NOT happen automatically - the base and view
tablets may be located on different nodes, and the view update may be
remote, and NOT synchronous.

So in this patch we explicitly mark the view as synchronous_update when
building the view for an LSI.

The bigger part of this patch is to add a test which reliably fails
before this patch, and passes after it. The test creates a two-node
cluster and a table with LSI, and pins the base's tablets to one node
and the view's to the second node, forcing the view updates to be
remote. It also uses an injection point to make the view update slower.
The test then writes to the base and immediately tries to use the index
to read. Before this patch, the read doesn't find the new data (contrary
to the guarantee in the documentation). After this patch, the read
does find the new data - because the write waited for the index to
be updated.

Fixes #16371

Signed-off-by: Nadav Har'El <nyh@scylladb.com>
2023-12-21 11:44:50 +02:00
Nadav Har'El
7c5092cb8f test: add missing "tags" schema extension to cql_test_env
One of the unfortunate anti-features of cql_test_env (the framework used
in our CQL tests that are written in C++) is that it needs to repeat
various bizarre initializations steps done in main.cc, otherwise various
requests work incorrectly. One of these steps that main.cc is to initialize
various "schema extensions" which some of the Scylla features need to work
correctly.

We remembered to initialize some schema extensions in cql_test_env, but
forgot others. The one I will need in the following patch is the "tags"
extension, which we need to mark materialized views used by local
secondary indexes as "synchronous_updates" - without this patch the LSI
tests in secondary_index_test.cc will crash.

In addition to adding the missing extension, this patch also replaces
the segmentation-fault crash when it's missing (caused by a dynamic
cast failure) by a clearer on_internal_error() - so if we ever have
this bug again, it will be easier to debug.

Signed-off-by: Nadav Har'El <nyh@scylladb.com>
2023-12-21 11:44:50 +02:00
Nadav Har'El
8181e28731 secondary index: fix view creation when using tablets
In commit 88a5ddabce, we fixed materialized
view creation to support tablets. We added to the function called to
create materialized views in CQL, prepare_new_view_announcement()
a missing call to the on_before_create_column_family() notifier that
creates tablets for this new view.

Unfortunately, We have the same problem when creating a secondary index,
because it does not use prepare_new_view_announcement(), and instead uses
a generic function to "update" the base table, which in some cases ends
up creating new views when a new index is requested. In this path, the
notifier did not get called to the notifier, so we must add it here too.
Unfortunately, the notifiers must run in a Seastar thread, which means
that yet another function now needs to run in a Seastar thread.

Before this patch, creating a secondary index in a table using tablets
fails with "Tablet map not found for table <uuid>". With this patch,
it works.

The patch also includes tests for creating a regular and local secondary
index. Both tests fail (with the aforementioned error) before this
patch, and pass with it.

Fixes #16396

Signed-off-by: Nadav Har'El <nyh@scylladb.com>
2023-12-21 11:44:50 +02:00
Raphael S. Carvalho
ee203f846e test: Fix segfault when running offstrategy test
Observer, that references table_for_test, must of course, not
outlive table_for_test. Observer can be called later after the
last input sstable is removed from sstable manager.

Signed-off-by: Raphael S. Carvalho <raphaelsc@scylladb.com>

Closes scylladb/scylladb#16428
2023-12-20 19:04:41 +02:00
Raphael S. Carvalho
d1e6dfadea sstables: Harden estimate_droppable_tombstone_ratio() interface
The interface is fragile because the user may incorrectly use the
wrong "gc before". Given that sstable knows how to properly calculate
"gc before", let's do it in estimate__d__t__r(), leaving no room
for mistakes.

sstable_run's variant was also changed to conform to new interface,
allowing ICS to properly estimate droppable ratio, using GC before
that is calculated using each sstable's range. That's important for
upcoming tablets, as we want to query only the range that belongs
to a particular tablet in the repair history table.

Signed-off-by: Raphael S. Carvalho <raphaelsc@scylladb.com>

Closes scylladb/scylladb#15931
2023-12-20 19:04:41 +02:00