Commit Graph

1320 Commits

Author SHA1 Message Date
Botond Dénes
9fe64b5d70 Merge 'Remove datadir string from table::config' from Pavel Emelyanov
The datadir keeps path to directory where local sstables can be. The very same information is now kept in table's storage options (#20542). This set fixes the remaining places that still use table::config::datadir and table::dir() and removes the datadir field.

Closes scylladb/scylladb#20675

* github.com:scylladb/scylladb:
  treewide: Remove table::config::datadir
  distributed_loader: Print storage options, not datadir
  data_dictionary: Add formatter for storage_options
  test: Construct table_for_tests with table storage options
  test: Generalize pair of make_table_for_tests helpers
  tests: Add helper to get snapshot directory from storage options
  table: snapshot_exists: Get directory from storage options
  table: snapshot_on_all_shards: Get directory from storage options
2024-09-26 15:26:45 +03:00
Avi Kivity
d16ea0afd6 Merge 'cql3: Extend DESC SCHEMA by auth and service levels' from Dawid Mędrek
Auth has been managed via Raft since Scylla 6.0. Restoring data
following the usual procedure (1) is error-prone and so a safer
method must have been designed and implemented. That's what
happens in this PR.

We want to extend `DESC SCHEMA` by auth and service levels
to provide a safe way to backup and restore those two components.
To realize that, we change the meaning of `DESC SCHEMA WITH INTERNALS`
and add a new "tier": `DESC SCHEMA WITH INTERNALS AND PASSWORDS`.

* `DESC SCHEMA` -- no change, i.e. the statement describes the current
  schema items such as keyspaces, tables, views, UDTs, etc.
* `DESC SCHEMA WITH INTERNALS` -- does the same as the previous tier
  and also describes auth and service levels. No information about
  passwords is returned.
* `DESC SCHEMA WITH INTERNALS AND PASSWORDS` -- does the same
  as the previous tier and also includes information about the salted
  hashes corresponding to the passwords of roles.

To restore existing roles, we extend the `CREATE ROLE` statement
by allowing to use the option `WITH SALTED HASH = '[...]'`.

---

Implementation strategy:

* Add missing things/adjust existing ones that will be used later.
* Implement creating a role with salted hash.
* Add tests for creating a role with salted hash.
* Prepare for implementing describe functionality of auth and service levels.
* Implement describe functionality for elements of auth and service levels.
* Extend the grammar.
* Add tests for describe auth and service levels.
* Add/update documentation.

---

(1): https://opensource.docs.scylladb.com/stable/operating-scylla/procedures/backup-restore/restore.html
In case the link stops working, restoring a schema was realised
by managing raw files on disk.

Fixes scylladb/scylladb#18750
Fixes scylladb/scylladb#18751
Fixes scylladb/scylladb#20711

Closes scylladb/scylladb#20168

* github.com:scylladb/scylladb:
  docs: Update user documentation for backup and restore
  docs/dev: Add documentation for DESC SCHEMA
  test: Add tests for describing auth and service levels
  cql3/functions/user_function: Remove newline character before and after UDF body
  cql3: Implement DESCRIBE SCHEMA WITH INTERNALS AND PASSWORDS
  auth: Implement describing auth
  auth/authenticator: Add member functions for querying password hash
  service/qos/service_level_controller: Describe service levels
  data_dictionary: Remove keyspace_element.hh
  treewide: Start using new overloads of describe
  treewide: Fix indentation in describe functions
  treewide: Return create statement optionally in describe functions
  treewide: Add new describe overloads to implementations of data_dictionary::keyspace_element
  treewide: Start using schema::ks_name() instead of schema::keyspace_name()
  cql3: Refactor `description`
  cql3: Move description to dedicated files
  test: Add tests for `CREATE ROLE WITH SALTED HASH`
  cql3/statements: Restrict CREATE ROLE WITH SALTED HASH
  auth: Allow for creating roles with SALTED HASH
  types: Introduce a function `cql3_type_name_without_frozen()`
  cql3/util: Accept std::string_view rather than const sstring&
2024-09-24 21:44:32 +03:00
Avi Kivity
5c329e3db0 Merge 'Put sstables::test class on a diet' from Pavel Emelyanov
This one is aimed at giving tests the ability to call private methods of class sstable. Some of the wrappers in the test class wrap public methods and can be removed.

Closes scylladb/scylladb#20614

* github.com:scylladb/scylladb:
  test: Remove sstables::test::binary_search()
  test: Remove sstables::test::move_summary()
  test: Remove sstables::test::read_toc()
  test: Remove sstables::test::get_summary()
  test: Remove sstables::test::get_statistics()
  test: Remove sstables::test::data_read()
2024-09-23 21:40:40 +03:00
Dawid Mędrek
7b4f9c806c treewide: Start using new overloads of describe
We continue removing `data_dictionary::keyspace_element`.
In this commit, we start using the overloads returning
`cql3::description` in places where the methods specified
by `data_dictionary::keyspace_element` were used.
2024-09-20 14:24:54 +02:00
Dawid Mędrek
39cf106151 treewide: Start using schema::ks_name() instead of schema::keyspace_name()
We're going to remove the interface `data_dictionary::keyspace_element`.
As `schema::keyspace_name()` is an implementation of one of the methods
specified by that interface, we replace its uses by `schema::ks_name()`.
`schema::keyspace_name()` was an alias for it, so no semantic change
has occured.
2024-09-20 14:24:53 +02:00
Pavel Emelyanov
8487f2fd93 treewide: Remove table::config::datadir
It's write-only now, all the places than wanted to know where table's
storage is, already use storage_options.

Signed-off-by: Pavel Emelyanov <xemul@scylladb.com>
2024-09-19 13:06:39 +03:00
Pavel Emelyanov
5046cfab4b test: Construct table_for_tests with table storage options
The only place that constructs table_for_tests is make_table_for_tests
helper. It can and should prepare the correct storage options, because
that's the last place where the target directory is still known.

Signed-off-by: Pavel Emelyanov <xemul@scylladb.com>
2024-09-19 13:06:39 +03:00
Pavel Emelyanov
eaad4f348b test: Generalize pair of make_table_for_tests helpers
They only differ in a way they get target directory from -- one via
argument, andother from test_env. Respectively, the latter can call the
former.

Signed-off-by: Pavel Emelyanov <xemul@scylladb.com>
2024-09-19 13:06:39 +03:00
Kefu Chai
cb1670b79b Update seastar submodule
* seastar ec5da7a6...69f88e2f (38):
  > build: s/Sanitizers_COMPILER_OPTIONS/Sanitizers_COMPILE_OPTIONS
  > test: Update httpd test with request/reply body writing sugar
  > http: Add sugar to request and response body writers
  > utils: Add util::write_to_stream() helper
  > seastar-addr2line: adjust llvm termination regex
  > README.md: add Crimson project
  > rpc: conditionally use fmt::runtime() based on SEASTAR_LOGGER_COMPILE_TIME_FMT
  > build: check the combination of Sanitizers
  > tls: clear session ticket before releasing
  > print: remove dead code
  > doc/lambda-coroutine-fiasco: reword for better readability
  > rpc: fix compilation error caused by fmt::runtime()
  > tutorial: explain the use case of rethrow_exception and coroutine::exception
  > reactor: print more informative error when io_submit fails
  > README.md: note GitHub discussions
  > prometheus: `fmt::print` to stringstream directly
  > doc: add document for testing with seastar
  > seastar/testing: only include used headers
  > test: Add abortable http client test cases
  > http/client: Add abortable make_request() API method
  > http/client: Abort established connections
  > http/client: Handle abort source in pool wait
  > http/client: Add abort source to factory::make() method
  > http/client: Pass abort_source here and there
  > http/client: Idnentation fix after previous patch
  > http/client: Merge some continuations explicitly
  > signal: add seastar signal api
  > httpd: remove unused prometheus structs
  > print: use fmtlib's fmt::format_string in format()
  > rpc: do not use seastar::format() in rpc logger
  > treewide: s/format/seastar::format/
  > prometheus: sanitize label value for text protocol
  > tests: unit test prometheus wire format
  > io-tester: Introduce batches to rate-based submission
  > io-tester: Generalize issueing request and collecting its result
  > io-tester: Cancel intent once
  > io-tester: Dont carry rps/parallelism variables over lambdas
  > io-tester: Simplify in-flight management

The breaking changes in the seastar submodule necessitate corresponding
modifications in our code. These changes must be implemented together in
a single commit to maintain consistency. So that each commit is buildable.

following changes are included in addition to seastar submodule update:
* instead of passing a `const char*` for the format string, pass a
  templated `fmt::format_string<...>`, this depends on the
  `seastar::format()` change in seastar.
* explicitly call `fmt::runtime()` if the format string is not a
  consteval expression. this depends on the `seastar::format()` change
  in seastar. as `seastar::format()` does not accept a plain
  `const char*` which is not constexpr anymore.
* pass abort_source to `dns_connection_factory::make()`. this depends on
  the change in seastar, which added a `abort_source*` argument to
  the pure virtual member function of `connection_factory::make()`.
* call call {fmt,seastar}::format() explicitly. this is a follow up of
  3e84d43f, which takes care of all places where we should call
  `fmt::format()` and `seastar::format()` explicitly to disambiguate the
  `format()` call. but more `format()` call made their way into the source
   tree after 3e84d43f. so we need fix them as well.
* include used header in tests

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

Update seastar submodule

 Please enter the commit message for your changes. Lines starting

Closes scylladb/scylladb#20649
2024-09-18 13:59:22 +03:00
Piotr Dulikowski
cc5c3aaae7 Merge 'message/messaging_service: guard adding maintenance tenant under cluster feature' from Michał Jadwiszczak
In https://github.com/scylladb/scylladb/pull/18729, we introduced a new statement tenant `$maintenance`, but the change wasn't protected by any cluster feature.
This wasn't a problem for OSS, since unknown isolation cookie just uses default scheduling group. However, in enterprise that leads to creating a service level on not-upgraded nodes, which may end up in an error if user create maximum number of service levels.

This patch adds a cluster feature to guard adding the new tenant. It's done in the way to handle two upgrade scenarios:
- version without `$maintenance` tenant -> version with `$maintenance` tenant guarded by a feature
- version with `$maintenance` tenant but not guarded by a feature -> version with `$maintenance` tenant guarded by a feature

The PR adds `enabled` flag to statement tenants.
This way, when the tenant is disabled, it cannot be used to create a connection, but it can be used to accept an incoming connection.
The `$maintenance` tenant is added to the config as disabled and it gets enabled once the corresponding feature is enabled.

Fixes scylladb/scylladb#20070
Refs scylladb/scylla-enterprise#4403

Closes scylladb/scylladb#19802

* github.com:scylladb/scylladb:
  message/messaging_service: guard adding maintenance tenant under cluster feature
  message/messaging_service: add feature_service dependency
  message/messaging_service: add `enabled` flag to statement tenants
2024-09-17 18:24:34 +02:00
Michał Jadwiszczak
71a03ef6b0 message/messaging_service: add feature_service dependency 2024-09-16 15:33:40 +02:00
Pavel Emelyanov
bf5021e735 test: Remove sstables::test::binary_search()
That's the most mysterious wrapper in this set as it doesn't need
sstable itself at all, it just duplicates the existing non-class
function out there.

Signed-off-by: Pavel Emelyanov <xemul@scylladb.com>
2024-09-16 12:51:35 +03:00
Pavel Emelyanov
309d315af7 test: Remove sstables::test::move_summary()
This one is a bit tricky, as it needs to modify the sstables's summary.
However, the sstables::test::_summary() one returns mutable reference
and the only caller can use it.

Signed-off-by: Pavel Emelyanov <xemul@scylladb.com>
2024-09-16 12:50:48 +03:00
Pavel Emelyanov
deec952111 test: Remove sstables::test::read_toc()
The sstable::read_toc() is public method, use it directly.

Signed-off-by: Pavel Emelyanov <xemul@scylladb.com>
2024-09-16 12:50:19 +03:00
Pavel Emelyanov
25cd8ccdd8 test: Remove sstables::test::get_summary()
Same as previous patch -- callers can come with const reference to
summary, so they can live with existing public sstable::get_summary().

Signed-off-by: Pavel Emelyanov <xemul@scylladb.com>
2024-09-16 12:49:39 +03:00
Pavel Emelyanov
f714ac9b48 test: Remove sstables::test::get_statistics()
Just call the public sstable::get_statistics(). The callers would get
const reference on it, but they don't need more than that.

Signed-off-by: Pavel Emelyanov <xemul@scylladb.com>
2024-09-16 12:48:43 +03:00
Pavel Emelyanov
53afa583e8 test: Remove sstables::test::data_read()
The wrapper just changes the order of arguments for a public method.
Drop it, and call the wrapee directly.

Signed-off-by: Pavel Emelyanov <xemul@scylladb.com>
2024-09-16 12:47:59 +03:00
Pavel Emelyanov
36863d4ad0 sstables_manager: Remove table_dir from make_sstable()
It used to be passed to sstable constructor, but now it doesn't need
this argument.

Signed-off-by: Pavel Emelyanov <xemul@scylladb.com>
2024-09-13 16:49:50 +03:00
Pavel Emelyanov
4425cf54c6 tests: Properly initialize storage options with "dir"
Most of the tests work with local storage options. Some support S3
options as well. Whatever it is, when creating an sstable, tests need to
put proper "dir" on the options, this patch does so.

In fact, storage options for tests are created together with the
test-env, and ideally this is the place where dir should be assigned on
it. However, there are still places that explicitly specify path they
want to see sstables at, for those the new temporary options should be
constructed.

Signed-off-by: Pavel Emelyanov <xemul@scylladb.com>
2024-09-13 16:49:50 +03:00
Botond Dénes
c7c5817808 Merge 'Improve timestamp heuristics for tombstone garbage collection' from Benny Halevy
When purging regular tombstone consult the min_live_timestamp, if available.
This is safe since we don't need to protect dead data from resurrection, as it is already dead.

For shadowable_tombstones, consult the min_memtable_live_row_marker_timestamp,
if available, otherwise fallback to the min_live_timestamp.

If we see in a view table a shadowable tombstone with time T, then in any row where the row marker's timestamp is higher than T the shadowable tombstone is completely ignored and it doesn't hide any data in any column, so the shadowable tombstone can be safely purged without any effect or risk resurrecting any deleted data.

In other words, rows which might cause problems for purging a shadowable tombstone with time T are rows with row markers older or equal T. So to know if a whole sstable can cause problems for shadowable tombstone of time T, we need to check if the sstable's oldest row marker (and not oldest column) is older or equal T. And the same check applies similarly to the memtable.

If both extended timestamp statistics are missing, fallback to the legacy (and inaccurate) min_timestamp.

Fixes scylladb/scylladb#20423
Fixes scylladb/scylladb#20424

> [!NOTE]
> no backport needed at this time
> We may consider backport later on after given some soak time in master/enterprise
> since we do see tombstone accumulation in the field under some materialized views workloads

Closes scylladb/scylladb#20446

* github.com:scylladb/scylladb:
  cql-pytest: add test_compaction_tombstone_gc
  sstable_compaction_test: add mv_tombstone_purge_test
  sstable_compaction_test: tombstone_purge_test: test that old deleted data do not inhibit tombstone garbage collection
  sstable_compaction_test: tombstone_purge_test: add testlog debugging
  sstable_compaction_test: tombstone_purge_test: make_expiring: use next_timestamp
  sstable, compaction: add debug logging for extended min timestamp stats
  compaction: get_max_purgeable_timestamp: use memtable and sstable extended timestamp stats
  compaction: define max_purgeable_fn
  tombstone: can_gc_fn: move declaration to compaction_garbage_collector.hh
  sstables: scylla_metadata: add ext_timestamp_stats
  compaction_group, storage_group, table_state: add extended timestamp stats getters
  sstables, memtable: track live timestamps
  memtable_encoding_stats_collector: update row_marker: do nothing if missing
2024-09-13 08:56:51 +03:00
Kefu Chai
3e84d43f93 treewide: use seastar::format() or fmt::format() explicitly
before this change, we rely on `using namespace seastar` to use
`seastar::format()` without qualifying the `format()` with its
namespace. this works fine until we changed the parameter type
of format string `seastar::format()` from `const char*` to
`fmt::format_string<...>`. this change practically invited
`seastar::format()` to the club of `std::format()` and `fmt::format()`,
where all members accept a templated parameter as its `fmt`
parameter. and `seastar::format()` is not the best candidate anymore.
despite that argument-dependent lookup (ADT for short) favors the
function which is in the same namespace as its parameter, but
`using namespace` makes `seastar::format()` more competitive,
so both `std::format()` and `seastar::format()` are considered
as the condidates.

that is what is happening scylladb in quite a few caller sites of
`format()`, hence ADT is not able to tell which function the winner
in the name lookup:

```
/__w/scylladb/scylladb/mutation/mutation_fragment_stream_validator.cc:265:12: error: call to 'format' is ambiguous
  265 |     return format("{} ({}.{} {})", _name_view, s.ks_name(), s.cf_name(), s.id());
      |            ^~~~~~
/usr/bin/../lib/gcc/x86_64-redhat-linux/14/../../../../include/c++/14/format:4290:5: note: candidate function [with _Args = <const std::basic_string_view<char> &, const seastar::basic_sstring<char, unsigned int, 15> &, const seastar::basic_sstring<char, unsigned int, 15> &, const utils::tagged_uuid<table_id_tag> &>]
 4290 |     format(format_string<_Args...> __fmt, _Args&&... __args)
      |     ^
/__w/scylladb/scylladb/seastar/include/seastar/core/print.hh:143:1: note: candidate function [with A = <const std::basic_string_view<char> &, const seastar::basic_sstring<char, unsigned int, 15> &, const seastar::basic_sstring<char, unsigned int, 15> &, const utils::tagged_uuid<table_id_tag> &>]
  143 | format(fmt::format_string<A...> fmt, A&&... a) {
      | ^
```

in this change, we

change all `format()` to either `fmt::format()` or `seastar::format()`
with following rules:
- if the caller expects an `sstring` or `std::string_view`, change to
  `seastar::format()`
- if the caller expects an `std::string`, change to `fmt::format()`.
  because, `sstring::operator std::basic_string` would incur a deep
  copy.

we will need another change to enable scylladb to compile with the
latest seastar. namely, to pass the format string as a templated
parameter down to helper functions which format their parameters.
to miminize the scope of this change, let's include that change when
bumping up the seastar submodule. as that change will depend on
the seastar change.

Signed-off-by: Kefu Chai <kefu.chai@scylladb.com>
2024-09-11 23:21:40 +03:00
Avi Kivity
ed7d352e7d Merge 'Validate checksums for uncompressed SSTables' from Nikos Dragazis
This PR introduces a new file data source implementation for uncompressed SSTables that will be validating the checksum of each chunk that is being read. Unlike for compressed SSTables, checksum validation for uncompressed SSTables will be active for scrub/validate reads but not for normal user reads to ensure we will not have any performance regression.

It consists of:
* A new file data source for uncompressed SSTables.
* Integration of checksums into SSTable's shareable components. The validation code loads the component on demand and manages its lifecycle with shared pointers.
* A new `integrity_check` flag to enable the new file data source for uncompressed SSTables. The flag is currently enabled only through the validation path, i.e., it does not affect normal user reads.
* New scrub tests for both compressed and uncompressed SSTables, as well as improvements in the existing ones.
* A change in JSON response of `scylla validate-checksums` to report if an uncompressed SSTable cannot be validated due to lack of checksums (no `CRC.db` in `TOC.txt`).

Refs #19058.

New feature, no backport is needed.

Closes scylladb/scylladb#20207

* github.com:scylladb/scylladb:
  test: Add test to validate SSTables with no checksums
  tools: Fix typo in help message of scylla validate-checksums
  sstables: Allow validate_checksums() to report missing checksums
  test: Add test for concurrent scrub/validate operations
  test: Add scrub/validate tests for uncompressed SSTables
  test/lib: Add option to create uncompressed random schemas
  test: Add test for scrub/validate with file-level corruption
  test: Check validation errors in scrub tests
  sstables: Enable checksum validation for uncompressed SSTables
  sstables: Expose integrity option via crawling mutation readers
  sstables: Expose integrity option via data_consume_rows()
  sstables: Add option for integrity check in data streams
  sstables: Remove unused variable
  sstables: Add checksum in the SSTable components
  sstables: Introduce checksummed file data source implementation
  sstables: Replace assert with on_internal_error
2024-09-11 23:09:45 +03:00
Calle Wilund
b7839ec5d0 cql_test_env: Use temp socket + retry to ensure usable port for message_service if listen is enabled
Fixes #20543

In cql_test_env, if cfg_in.ms_listen is set, we try to get a free port for the current test on
which message service rpc can bind. This to allow multiple tests in parallel.

However, we just do this by using random and getting a number, not actually verifying it against
host ports in use.

This is complicated further by the fact that port reuse is effectively disabled in seastar
(see reactor::posix_reuseport_detect()). Due to this, the solution applied here is a combo
of
* Create temp socket with port = 0 to get a previously free port
* Close socket right before listen (to handle reuse not working)
* Retry on EADDRINUSE

Closes scylladb/scylladb#20547
2024-09-11 23:02:41 +03:00
Piotr Dulikowski
d98708013c Merge 'view: move view_build_status to group0' from Michael Litvak
Migrate the `system_distributed.view_build_status` table to `system.view_build_status_v2`. The writes to the v2 table are done via raft group0 operations.

The new parameter `view_builder_version` stored in `scylla_local` indicates whether nodes should use the old or the new table.

New clusters use v2. Otherwise, the migration to v2 is initiated by the topology coordinator when the feature is enabled. It reads all the rows from the old table and writes them to the new table, and sets `view_builder_version` to v2. When the change is applied, all view_builder services are updated to write and read from the v2 table.

The old table `system_distributed.view_build_status` is set to read virtually from the new table in order to maintain compatibility.

When removing a node from the cluster, we remove its rows from the table atomically (fixes https://github.com/scylladb/scylladb/issues/11836). Also, during the migration, we remove all invalid rows.

Fixes scylladb/scylladb#15329

dtest https://github.com/scylladb/scylla-dtest/pull/4827

Closes scylladb/scylladb#19745

* github.com:scylladb/scylladb:
  view: test view_build_status table with node replace
  test/pylib: use view_build_status_v2 table in wait_for_view
  view_builder: common write view_build_status function
  view_builder: improve migration to v2 with intermediate phase
  view: delete node rows from view_build_status on node removal
  view: sanitize view_build_status during migration
  view: make old view_build_status table a virtual table
  replica: move streaming_reader_lifecycle_policy to header file
  view_builder: test view_build_status_v2
  storage_service: add view_build_status to raft snapshot
  view_builder: migration to v2
  db:system_keyspace: add view_builder_version to scylla_local
  view_builder: read view status from v2 table
  view_builder: introduce writing status mutations via raft
  view_builder: pass group0_client and qp to view_builder
  view_builder: extract sys_dist status operations to functions
  db:system_keyspace: add view_build_status_v2 table
2024-09-11 13:02:58 +02:00
Nikos Dragazis
d1152a200f test: Add test to validate SSTables with no checksums
In a previous patch we extended the return status of
`sstables::validate_checksums()` to report if an SSTable cannot be
validated due to a missing CRC component (i.e., CRC.db does not appear
in TOC.txt).

Add a test case for this.

Signed-off-by: Nikos Dragazis <nikolaos.dragazis@scylladb.com>
2024-09-11 13:12:40 +03:00
Nikos Dragazis
2991b09c8e test/lib: Add option to create uncompressed random schemas
Extend the `random_schema_specification` to support creating both
compressed and uncompressed schemas.

Signed-off-by: Nikos Dragazis <nikolaos.dragazis@scylladb.com>
2024-09-11 13:12:32 +03:00
Benny Halevy
6f202cf48b compaction_group, storage_group, table_state: add extended timestamp stats getters
To return the minimum live timestamp and live row-marker
timestamp across a compaction_group, storage_group, or
table_state.

Signed-off-by: Benny Halevy <bhalevy@scylladb.com>
2024-09-10 19:05:57 +03:00
Pavel Emelyanov
cfa59ab73d test: Use single temp dir for sharded<sstables::test_env>
The test-env in question is mostly started in one-shard mode. Also there
are several boost tests that start sharded<> environment. In that case
instances on different shards live in different temp dirs. That's not
critical yet, but better to have single directory for the whole test.

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

Closes scylladb/scylladb#20412
2024-09-10 11:25:04 +03:00
Michael Litvak
b8c7a10ae6 view_builder: pass group0_client and qp to view_builder
Store references of group0_client and query_processor in the
view_builder service.
They are required for generating mutations and writing them via group0.
2024-09-05 15:41:04 +03:00
Pavel Emelyanov
e9980bd6dd test: Move sstable_open_config to reusable_sst()'s argument
So that callers are able to provide custom config in the future

Signed-off-by: Pavel Emelyanov <xemul@scylladb.com>
2024-09-03 14:00:59 +03:00
Botond Dénes
52bed81a1e Merge 'cql3: add option to not unify bind variables with the same name' from Avi Kivity
Bind variables in CQL have two formats: positional (`?`) where a variable is referred to by its relative position in the statement, and named (`:var`), where the user is expected to supply a name->value mapping.

In 19a6e69001 we identified the case where a named bind variable appears twice in a query, and collapsed it to a single entry in the statement metadata. Without this, a driver using the named variable syntax cannot disambiguate which variable is referred to.

However, it turns out that users can use the positional call form even with the named variable syntax, by using the positional API of the driver. To support this use case, we add a configuration variable to disable the same-variable detection.

Because the detection has to happen when the entire statement is visible, we have to supply the configuration to the parser. We call it the `dialect` and pass it from all callers. The alternative would be to add a pre-prepare call similar to fill_prepare_context that rewrites all expressions in a statement to deduplicate variables.

A unit test is added.

Fixes #15559

This may be useful to users transitioning from Cassandra, so merits a backport.

Closes scylladb/scylladb#19493

* github.com:scylladb/scylladb:
  cql3: add option to not unify bind variables with the same name
  cql3: introduce dialect infrastructure
  cql3: prepared_statement_cache: drop cache key default constructor
2024-09-02 08:34:24 +03:00
Pavel Emelyanov
7df43312ac test: Remove sstable making helpers from table_for_tests
All users of it have sstable_test_env at hand (in fact -- they call env
method to get table_for_test). And since sstable_test_env already has a
bunch of methods to create sstable, the table_for_test wrapper doesn't
need to duplicate this code.

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

Closes scylladb/scylladb#20360
2024-09-01 19:58:15 +03:00
Kefu Chai
753188c33d test: include seastar/testing/random.hh when appropriate
in a recent seastar change (644bb662), we do not include
`seastar/testing/random.hh` in `seastar/testing/test_runner.hh` anymore,
as the latter is not a facade of the former, and neither does it use the
former. as a sequence, some tests which take the advantage of the
included `seastar/testing/random.hh` do not build with the latest
seastar:

```
FAILED: test/lib/CMakeFiles/test-lib.dir/key_utils.cc.o
/usr/bin/clang++ -DBOOST_REGEX_DYN_LINK -DBOOST_REGEX_NO_LIB -DBOOST_UNIT_TEST_FRAMEWORK_DYN_LINK -DBOOST_UNIT_TEST_FRAMEWORK_NO_LIB -DDEVEL -DFMT_SHARED -DSCYLLA_BUILD_MODE=dev -DSCYLLA_ENABLE_ERROR_INJECTION -DSCYLLA_ENABLE_PREEMPTION_SOURCE -DSEASTAR_API_LEVEL=7 -DSEASTAR_ENABLE_ALLOC_FAILURE_INJECTION -DSEASTAR_LOGGER_COMPILE_TIME_FMT -DSEASTAR_LOGGER_TYPE_STDOUT -DSEASTAR_SCHEDULING_GROUPS_COUNT=16 -DSEASTAR_SSTRING -DSEASTAR_TYPE_ERASE_MORE -DXXH_PRIVATE_API -I/__w/scylladb/scylladb -I/__w/scylladb/scylladb/build/gen -I/__w/scylladb/scylladb/seastar/include -I/__w/scylladb/scylladb/build/seastar/gen/include -I/__w/scylladb/scylladb/build/seastar/gen/src -I/__w/scylladb/scylladb/build -isystem /__w/scylladb/scylladb/abseil -isystem /__w/scylladb/scylladb/build/rust -O2 -std=gnu++23 -fvisibility=hidden -Wall -Werror -Wextra -Wno-error=deprecated-declarations -Wimplicit-fallthrough -Wno-c++11-narrowing -Wno-deprecated-copy -Wno-mismatched-tags -Wno-missing-field-initializers -Wno-overloaded-virtual -Wno-unsupported-friend -Wno-enum-constexpr-conversion -Wno-unused-parameter -ffile-prefix-map=/__w/scylladb/scylladb/build=. -march=westmere -Xclang -fexperimental-assignment-tracking=disabled -Werror=unused-result -fstack-clash-protection -MD -MT test/lib/CMakeFiles/test-lib.dir/key_utils.cc.o -MF test/lib/CMakeFiles/test-lib.dir/key_utils.cc.o.d -o test/lib/CMakeFiles/test-lib.dir/key_utils.cc.o -c /__w/scylladb/scylladb/test/lib/key_utils.cc
In file included from /__w/scylladb/scylladb/test/lib/key_utils.cc:11:
/__w/scylladb/scylladb/test/lib/random_utils.hh:25:30: error: no member named 'local_random_engine' in namespace 'seastar::testing'
   25 |     return seastar::testing::local_random_engine;
      |            ~~~~~~~~~~~~~~~~~~^
1 error generated.
```

in this change, we include `seastar/testing/random.hh` when the random
facility is used, so that they can be compiled with the latest seastar
library.

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

Closes scylladb/scylladb#20368
2024-09-01 18:57:07 +03:00
Avi Kivity
ea8441dfa3 cql3: add option to not unify bind variables with the same name
Bind variables in CQL have two formats: positional (`?`) where a
variable is referred to by its relative position in the statement,
and named (`:var`), where the user is expected to supply a
name->value mapping.

In 19a6e69001 we identified the case where a named bind variable
appears twice in a query, and collapsed it to a single entry in the
statement metadata. Without this, a driver using the named variable
syntax cannot disambiguate which variable is referred to.

However, it turns out that users can use the positional call form
even with the named variable syntax, by using the positional
API of the driver. To support this use case, we add a configuration
variable to disable the same-variable detection.

Because the detection has to happen when the entire statement is
visible, we have to supply the configuration to the parser. We
call it the `dialect` and pass it from all callers. The alternative
would be to add a pre-prepare call similar to fill_prepare_context that
rewrites all expressions in a statement to deduplicate variables.

A unit test is added.

Fixes #15559
2024-09-01 17:27:48 +03:00
Avi Kivity
d69bf4f010 cql3: introduce dialect infrastructure
A dialect is a different way to interpret the same CQL statement.

Examples:
 - how duplicate bind variable names are handled (later in this series)
 - whether `column = NULL` in LWT can return true (as is now) or
   whether it always returns NULL (as in SQL)

Currently, dialect is an empty structure and will be filled in later.
It is passed to query_processor methods that also accept a CQL string,
and from there to the parser. It is part of the prepared statement cache
key, so that if the dialect is changed online, previous parses of the
statement are ignored and the statement is prepared again.

The patch is careful to pick up the dialect at the entry point (e.g.
CQL protocol server) so that the dialect doesn't change while a statement
is parsed, prepared, and cached.
2024-08-29 21:19:23 +03:00
Avi Kivity
72a85e3812 Merge 'Integrated backup' from Pavel Emelyanov
This adds minimal implementation of the start-backup API call.

The method starts a task that uploads all files from the given keyspace's snapshot to the requested endpoint/bucket. Arguments are:
- endpoint -- the ID in object_store.yaml config file
- bucket -- the target bucket to put objects into
- keyspace -- the keyspace to work on
- snapshot -- the method assumes that the snapshot had been already taken and only copies sstables from it

The task runs in the background, its task_id is returned from the method once it's spawned and it should be used via /task_manager API to track the task execution and completion (hint: it's good to have non-zero TTL value to make sure fast backups don't finish before the caller manages to call wait_task API).

Sstables components are scanned for all tables in the keyspace and are uploaded into the /bucket/${cf_name}/${snapshot_name}/ path.

refs: #18391

Closes scylladb/scylladb#19890

* github.com:scylladb/scylladb:
  tools/scylla-nodetool: add backup integration
  docs: Document the new backup method
  test/object_store: Test that backup task is abortable
  test/object_store: Add simple backup test
  test/object_store: Move format_tuples()
  test/pylib: Add more methods to rest client
  backup-task: Make it abortable (almost)
  code: Introduce backup API method
  database: Export parse_table_directory_name() helper
  database: Introduce format_table_directory_name() helper
  snapshot-ctl: Add config to snapshot_ctl
  snapshot-ctl: Add sstables::storage_manager dependency
  snapshot-ctl: Maintain task manager module
  snapshot-ctl: Add "snapshots" logger
  snapshot-ctl: Outline stop() method and constructor
  snapshot-ctl: Inline run_snapshot_list<>
  test/cql_test_env: Export task manager from cql test env
  task_manager: Print task ttl on start (for debugging)
  docs: Update object_storage.md with AWS_ environment
  docs: Restructure object_storage.md
2024-08-25 20:19:10 +03:00
Pavel Emelyanov
38edbebb10 compaction_manager: Keep flush-all-before-major option on own config
Currently the major compaction task impl grabs this (non-updateable)
value from db::config. That's not good, all services including
compaction manager have their own configs from which they take options.
Said that, this patch puts the said option onto
compaction_manager::config, makes use of it and configures one from
db::config on start (and tests).

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

Closes scylladb/scylladb#20174
2024-08-23 10:31:55 +03:00
Kefu Chai
ee19bbed05 test: do not define boost_test_print_type() for types with operator<<
in 30e82a81, we add a contraint to the template parameter of
boost_test_print_type() to prevent it from being matched with
types which can be formatted with operator<<. but it failed to
work. we still have test failure reports like:

```
[Exception] - critical check ['s', 's', 't', '_', 'm', 'r', '.', 'i', 's', '_', 'e', 'n', 'd', '_', 'o', 'f', '_', 's', 't', 'r', 'e', 'a', 'm', '(', ')'] has failed
```

this is not what we expect. the reason is that we passed the template
parameters to the `has_left_shift` trait in the wrong order, see
https://live.boost.org/doc/libs/1_83_0/libs/type_traits/doc/html/boost_typetraits/reference/has_left_shift.html.
we should have passed the lhs of operator<< expression as first
parameter, and rhs the second.

so, in this change, we correct the type constraint by passing the
template parameter in the right order, now the error message looks
better, like:

```
test/boost/mutation_query_test.cc(110): error: in "test_partition_query_is_full": check !partition_slice_builder(*s) .with_range({}) .build() .is_full() has failed
```

it turns out boost::transformed_range<> is formattable with operator<<,
as it fulfills the constraints of `boost::has_left_shift<ostream, R>`,
but when printing it, the compiler fails when it tries to insert the
elements in the range to the output stream.

so, in order to workaround this issue, we add a specialization for
`boost::transformed_range<F, R`.

also, to improve the readability, we reimplement the `has_left_shift<>`
as a concept, so that it's obvious that we need to put both the output
stream as the first parameter.

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

Closes scylladb/scylladb#20233
2024-08-23 09:26:22 +03:00
Pavel Emelyanov
4e73b4d8ad test/cql_test_env: Export task manager from cql test env
To be used by one of the next patches

Signed-off-by: Pavel Emelyanov <xemul@scylladb.com>
2024-08-22 14:08:21 +03:00
Avi Kivity
8c257db283 Merge 'Native reverse pages over RPC' from Łukasz Paszkowski
Drop half-reversed (legacy) format of query::partition_slice.

The select query builds a fully reversed (native) slice for reversed queries and use it together with a reversed
schema to construct query::read_command that is further propagated to the database.

A cluster feature is added to support nodes that still operate on half-reversed slices. When the feature is turned off:
- query::read_command is transformed (to have table schema and half-reversed slices) before sending to other nodes
- query::read_command is transformed (to have query schema (reversed) and reversed slices) after receiving it from other nodes
- Similarly, mutations are transformed. They are reversed before being sent to other nodes or after receiving them from other nodes.

Additional manual tests were performed to test a mixed-node cluster:

1. 3-node cluster with one node upgraded: reverse read queries performed on an old node
2. 3-node cluster with one node upgraded: reverse read queries performed on a new node
3. 3-node cluster with one node upgraded and all its sstable files deleted to trigger repair: reverse read queries performed on an old node
4. 3-node cluster with one node upgraded and all its sstable files deleted to trigger repair: reverse read queries performed on a new node

All reverse read queries above consists of:

- single-partition reverse reads with no clustering key restrictions, with single column restrictions and multi column restrictions both with and without paging turned on
- multi-partition reverse reads with range restrictions with optional partition limit and partial ordering

The exact same tests were also performed on a fully upgraded cluster.

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

Closes scylladb/scylladb#18864

* github.com:scylladb/scylladb:
  mutation_partition: drop reverse parameter in compact_for_query
  clustering_key_filter: unify get_ranges and get_native_ranges
  streamed_mutation_freezer: drop the reverse parameter
  reverse-reads.md: Drop legacy reverse format information
  Fix comments refering to half-reversed (legacy) slices
  select_statement::do_execute: Add tracing informaction
  query::trim_clustering_row_ranges_to: require reversed schema for native reversed ranges
  query-request: Drop half_reverse_slice as it is no longer used anywhere
  readers: Use reversed schema and native reversed slices
  database: accept reversed schema for reversed queries
  storage_proxy: Support reverse queries in native format
  query_pagers: Replace _schema with _query_schema
  query_pagers: Support reverse queries in native format
  select_statement: Execute reversed query in native format
  storage_proxy::remote: Add support for mixed-node clusters
  mutation_query: Add reversed function to reverse reconcilable_result
  query-request: Add reversed function to reverse read_command
  features: add native_reverse_queries
  kl::reader::make_reader: Unify interface with mx::reader::make_reader
  config: drop reversed_reads_auto_bypass_cache
  config: drop enable_optimized_reversed_reads
2024-08-14 17:51:56 +03:00
Michał Chojnowski
4d77faa61e cql_test_env: ensure shutdown() before stop() for system_keyspace
If system_keyspace::stop() is called before system_keyspace::shutdown(),
it will never finish, because the uncleared shared pointers will keep
it alive indefinitely.

Currently this can happen if an exception is thrown before the construction
of the shutdown() defer. This patch moves the shutdown() call to immediately
before stop(). I see no reason why it should be elsewhere.

Fixes scylladb/scylla-enterprise#4380

Closes scylladb/scylladb#20089
2024-08-14 12:16:44 +03:00
Łukasz Paszkowski
da95f44adc readers: Use reversed schema and native reversed slices
The reconcilable_result is built as it would be constructed for
forward read queries for tables with reversed order.

Mutations constructed for reversed queries are consumed forward.

Drop overloaded reversed functions that reverse read_command and
reconcilable_result directly and keep only those requiring smart
pointers. They are not used any more.
2024-08-13 10:03:46 +02:00
Avi Kivity
aa1270a00c treewide: change assert() to SCYLLA_ASSERT()
assert() is traditionally disabled in release builds, but not in
scylladb. This hasn't caused problems so far, but the latest abseil
release includes a commit [1] that causes a 1000 insn/op regression when
NDEBUG is not defined.

Clearly, we must move towards a build system where NDEBUG is defined in
release builds. But we can't just define it blindly without vetting
all the assert() calls, as some were written with the expectation that
they are enabled in release mode.

To solve the conundrum, change all assert() calls to a new SCYLLA_ASSERT()
macro in utils/assert.hh. This macro is always defined and is not conditional
on NDEBUG, so we can later (after vetting Seastar) enable NDEBUG in release
mode.

[1] 66ef711d68

Closes scylladb/scylladb#20006
2024-08-05 08:23:35 +03:00
Piotr Dulikowski
44f327675d Merge 'Remove gossiper argument from storage_service::join_cluster()' from Pavel Emelyanov
It's only needed to start hints via proxy, but proxy can do it without gossiper argument

Closes scylladb/scylladb#19894

* github.com:scylladb/scylladb:
  storage_service: Remote gossiper argument from join_cluster()
  proxy: Use remote gossiper to start hints resource manager
  hints: Const-ify gossiper references and anchor pointers
2024-08-01 10:18:14 +02:00
Emil Maskovsky
2dbe9ef2f2 raft: use the abort source reference in raft group0 client interface
Most callers of the raft group0 client interface are passing a real
source instance, so we can use the abort source reference in the client
interface. This change makes the code simpler and more consistent.
2024-07-31 09:18:54 +02:00
Pavel Emelyanov
aaad2bbeaf storage_service: Remote gossiper argument from join_cluster()
This pointer was only needed to pull all the way down the hints resource
manager start() method. It's no longer needed for that.

Signed-off-by: Pavel Emelyanov <xemul@scylladb.com>
2024-07-26 16:29:58 +03:00
Aleksandra Martyniuk
8e56913fdf service: node_ops: keep node ops module in storage service
Keep task manager node ops module in storage service. It will be
used to create and manage tasks related to topology changes.

The module is created and registered in storage service constructor.
In storage_service::stop() the module is stopped and so all the remaining
tasks would be unregistered immediately after they are finished.
2024-07-23 13:35:01 +02:00
Botond Dénes
591876b44e Merge 'sstables: do not reload components of unlinked sstables' from Lakshmi Narayanan Sreethar
The SSTable is removed from the reclaimed memory tracking logic only
when its object is deleted. However, there is a risk that the Bloom
filter reloader may attempt to reload the SSTable after it has been
unlinked but before the SSTable object is destroyed. Prevent this by
removing the SSTable from the reclaimed list maintained by the manager
as soon as it is unlinked.

The original logic that updated the memory tracking in
`sstables_manager::deactivate()` is left in place as (a) the variables
have to be updated only when the SSTable object is actually deleted, as
the memory used by the filter is not freed as long as the SSTable is
alive, and (b) the `_reclaimed.erase(*sst)` is still useful during
shutdown, for example, when the SSTable is not unlinked but just
destroyed.

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

Closes scylladb/scylladb#19717

* github.com:scylladb/scylladb:
  boost/bloom_filter_test: add testcase to verify unlinked sstables are not reloaded
  sstables: do not reload components of unlinked sstables
  sstables/sstables_manager: introduce on_unlink method
2024-07-22 12:08:25 +03:00
Avi Kivity
358147959e Merge 'keep table directory open for flushing' from Laszlo Ersek
`filesystem_storage` methods frequently call `sync_directory()`, for the sake of flushing (sync'ing) a directory. `sync_directory()` always brackets the sync with open and close, and given that most `sync_directory()` calls target the sstable base directory, those repeated opens and closes are considered wasteful. Rework the `filesystem_storage::_dir` member (from a mere pathname) so that it stand for an `opened_directory` object, which keeps the sstable base directory open, for the purpose of repeated sync'ing.

Resolves #2399.

Closes scylladb/scylladb#19624

* github.com:scylladb/scylladb:
  sstables/storage: synch "dst_dir" more leanly in create_links_common()
  sstables/storage: close previous directory asynchronously upon dir change
  sstables/storage: futurize change_dir_for_test()
  sstables/storage: sync through "opened_directory" in filesystem...::move()
  sstables/storage: sync through "opened_directory" in the "easy" cases
  sstables/storage: introduce "opened_directory" class
2024-07-21 17:07:44 +03:00
Piotr Dulikowski
4571262e46 Merge 'Improve constness of functions schema code' from Marcin Maliszkiewicz
In v4 of scylladb/scylladb#19598 the last commit of the patch was replaced but this change missed merge so submitting it in a separate patch.

In the current patch, the original functions class correctly marks methods as const where appropriate, and the instance() method now returns a const object. This ensures protection against accidental modifications, as all changes must go through the change_batch object.

Since the functions_changer class was intended to serve the same purpose, it is now redundant. Therefore, we are reverting the commit that introduced it.

Relates scylladb/scylladb#19153

Closes scylladb/scylladb#19647

* github.com:scylladb/scylladb:
  cql3: functions: replace template with std::function in with_udf_iter()
  cql3: functions: improve functions class constness handling
  Revert "cql3: functions: make modification functions accessible only via batch class"
2024-07-19 19:23:11 +02:00