when comparing the disabled warnings specified by `configured.py`
and the ones specified by `cmake/mode.common.cmake`, it turns out
we are now able to enable more warning options. so let's enable them.
the change was tested using Clang-17 and GCC-13.
there are many errors from GCC-13, like:
```
/home/kefu/dev/scylladb/db/view/view.hh:114:17: error: declaration of ‘column_kind db::view::clustering_or_static_row::column_kind() const’ changes meaning of ‘column_kind’ [-fpermissive]
114 | column_kind column_kind() const {
| ^~~~~~~~~~~
```
so the build with GCC failed.
and with this change, Clang-17 is able to build build the tree without
warnings.
Signed-off-by: Kefu Chai <kefu.chai@scylladb.com>
time_t is defined as a "Arithmetic type capable of representing times".
so we can just initialize it with 0 without braces. this change should
silence warning like:
```
test/boost/aggregate_fcts_test.cc:238:45: error: braces around scalar initializer [-Werror,-Wbraced-scalar-init]
auto tp = db_clock::from_time_t({ 0 }) + std::chrono::milliseconds(1);
^~~~~
```
Signed-off-by: Kefu Chai <kefu.chai@scylladb.com>
in general, the more static analysis the merrier. with the updated
Seastar, which includes the commit of "core/sstring: define <=> operator
for sstring", all defaulted '<=> operator' which previously rely
on sstring's operator<=> will not be deleted anymore, so we can
enable `-Wdefaulted-function-deleted` now.
Signed-off-by: Kefu Chai <kefu.chai@scylladb.com>
Closes#12861
this change also includes change to main, to make this commit compile.
see below:
* seastar 9b6e181e42...9cbc1fe889 (46):
> Merge 'Make io-tester jobs share sched classes' from Pavel Emelyanov
> io_tester.md: Update the `rps` configuration option description
> io_tester: Add option to limit total number of requests sent
> Merge 'Keep outgoing queue all cancellable while negotiating (again)' from Pavel Emelyanov
> io_tester: Add option to share classes between jobs
> rpc: Abort connection if send_entry() fails
> Merge 'build: build dpdk with `-fPIC` if BUILD_SHARED_LIBS' from Kefu Chai
> build: cooking.sh: use the same BUILD_SHARED_LIBS when building ingredients
> build: cooking.sh: use the same generator when building ingredients
> core/memory: handle `strerror_r` returning static string
> Merge 'build, rpc: lz4 related cleanups' from Kefu Chai
> build, rpc: do not support lz4 < 1.7.3
> build: set the correct version when finding lz4
> build: include CheckSymbolExists
> rpc: do not include lz4.h in header
> build: set CMP0135 for Cooking.cmake
> docs: drop building-*.md
> Merge 'seastar-addr2line: cleanups' from Kefu Chai
> seastar-addr2line: refactor tests using unittest
> seastar-addr2line: extract do_test() and main()
> seastar-addr2line: do not import unused modules
> scheduling: add a `rename` callback to scheduling_group_key_config
> reactor: syscall thread: wakeup up reactor with finer granularity
> build: build dpdk with `-fPIC` if BUILD_SHARED_LIBS
> build: extract dpdk_extra_cflags out
> core/sstring: remove a temporary variable
> Merge 'treewide: include what we use, and add a checkheaders target' from Kefu Chai
> perftune.py: auto-select the same number of IRQ cores on each NUMA
> prometheus: remove unused headers
> core/sstring: define <=> operator for sstring
> Merge 'core: s/reserve_additional_memory/reserve_additional_memory_per_shard/' from Kefu Chai
> include: do not include <concepts> directly
> coding_style: note on self-contained header requirement
> circileci: build checkheaders in addition to default target
> build: add checkheaders target
> net/toeplitz: s/u_int/unsigned/
> net/tcp-stack: add forward declaration for seastar::socket
> core, net, util: include used headers
* main: set reserved memory for wasm on per-shard basis
this change is a follow-up of
f05d612da8 and
4a0134a097.
this change depends on the related change in Seastar to reserve
additional memory on a per-shard basis.
per Wojciech Mitros's comment:
> it should have probably been 50MB per shard
in other words, as we always execute the same set of udf on all
shards. and since one cannot predict the number of shards, but she
could have a rough estimation on the size of memory a regular (set
of) udf could use. so a per-shard setting makes more sense.
Signed-off-by: Kefu Chai <kefu.chai@scylladb.com>
There was a bug in `expr::search_and_replace`.
It doesn't preserve the `order` field of binary_operator.
`order` field is used to mark relations created
using the SCYLLA_CLUSTERING_BOUND.
It is a CQL feature used for internal queries inside Scylla.
It means that we should handle the restriction as a raw
clustering bound, not as an expression in the CQL language.
Losing the SCYLLA_CLUSTERING_BOUND marker could cause issues,
the database could end up selecting the wrong clustering ranges.
Fixes: #13055
Signed-off-by: Jan Ciolek <jan.ciolek@scylladb.com>
Closes#13056
* throw marshal_exception if not the whole string is parsed, we
should error out if the parsed string contains gabage at the end.
before this change, we silent accept uuid like
"ce84997b-6ea2-4468-9f02-8a65abf4wxyz", and parses it as
"ce84997b-6ea2-4468-9f02-8a65abf4". this is not correct.
* throw marshal_exception if stoull() throws,
`stoull()` throws if it fails to parse a string to an unsigned long
long, we should translate the exception to `marshal_exception`, so
we can handle these exception in a consistent manner.
test is updated accordingly.
Signed-off-by: Kefu Chai <kefu.chai@scylladb.com>
Closes#13069
instead of passing '0' in the initializer list to do aggregate
initialization, just use zero initialization. simpler this way.
also, this helps to silence a `-Wmissing-braces` warning, like
```
/home/kefu/dev/scylladb/auth/passwords.cc:21:43: error: suggest braces around initialization of subobject [-Werror,-Wmissing-braces]
static thread_local crypt_data tlcrypt = {0, };
^
{}
```
Signed-off-by: Kefu Chai <kefu.chai@scylladb.com>
Closes#13060
because `seastar::to_sstring()` defaults to `fmt::format_to()`. so
any type which is supported by `fmt::formatter()` is also supported
by `seastar::to_sstring()`. and the behavior of existing implementation
is exactly the same as the defaulted one.
so let's drop the specialization and let
`fmt::formatter<sstables::generation_type>` do its job.
Signed-off-by: Kefu Chai <kefu.chai@scylladb.com>
Closes#13070
- build: cmake: find ANTLR3 before using it
- build: cmake: define FMT_DEPRECATED_OSTREAM
- build: cmake: add include directory for lua
- build: cmake: link redis against db
Closes#13071
* github.com:scylladb/scylladb:
build: cmake: add more tests
build: cmake: find and link against RapidJSON
build: cmake: link couple libraries as whole archive
build: cmake: find ANTLR3 before using it
build: cmake: define FMT_DEPRECATED_OSTREAM
build: cmake: add include directory for lua
build: cmake: link redis against db
in general, the more static analysis the merrier. these warnings were previously added to silence warnings from Clang and/or GCC, but since we've addressed all of them, let's reenable them to detect potential issues early.
Closes#13063
* github.com:scylladb/scylladb:
build: reenable disabled warnings
test: lib: do not return a local reference
dht: incremental_owned_ranges_checker: use lower_bound()
types: reimplement in terms of a variable template
query_id: extract into new header
test/cql-pytest: test for CLUSTERING ORDER BY verification in MV
test/cql-pytest: allow "run-cassandra" without building Scylla
build: reenable unused-{variable,lambda-capture} warnings
test: reader_concurrency_semaphore_test: define target_memory in debug mode
flat_mutation_reader_test: cleanup, seastar::async -> SEASTAR_THREAD_TEST_CASE
make_nonforwardable: test through run_mutation_source_tests
make_nonforwardable: next_partition and fast_forward_to when single_partition is true
make_forwardable: fix next_partition
flat_mutation_reader_v2: drop forward_buffer_to
nonforwardable reader: fix indentation
nonforwardable reader: refactor, extract reset_partition
nonforwardable reader: add more tests
nonforwardable reader: no partition_end after fast_forward_to()
nonforwardable reader: no partition_end after next_partition()
nonforwardable reader: no partition_end for empty reader
api::failure_detector: mark set_phi_convict_threshold unimplemented
test: memtable_test: mark dummy variable for loop [[maybe_unused]]
idl-compiler: mark captured this used
raft: reference this explicitly
util/result_try: reference this explicitly
sstables/sstables: mark dummy variable for loop [[maybe_unused]]
treewide: do not define/capture unused variables
service: storage_service: clear _node_ops in batch
cql-pytest: add tests for sum() aggregate
build: cmake: extract mutation,db,replica,streaming out
build: cmake: link the whole auth
build: cmake: extract thrift out
build: cmake: expose scylla_gen_build_dir from "interface"
build: cmake: find libxcrypt before using it
build: cmake: find Thrift before using it
build: cmake: support thrift < 0.11.0
test/cql-pytest: move aggregation tests to one file
Revert "Revert "storage_service: Enable Repair Based Node Operations (RBNO) by default for all node ops""
storage_service: Wait for normal state handler to finish in replace
storage_service: Wait for normal state handler to finish in bootstrap
row_cache: pass partition_start though nonforwardable reader
doc: fix the version in the comment on removing the note
doc: specify the versions where Alternator TTL is no longer experimental
in general, the more static analysis the merrier. these warnings
were previously disabled to silence warnings from Clang and/or GCC,
but since we've addressed all of them, let's reenable them to
detect potential issues early.
Signed-off-by: Kefu Chai <kefu.chai@scylladb.com>
the type of return value of `get_table_views()` is a reference, so we
cannot return a reference to a temporary value.
in this change, a member variable is added to hold the _table_schema,
so it can outlive the function call.
this should silence following warning from Clang:
```
test/lib/expr_test_utils.cc:543:16: error: returning reference to local temporary object [-Werror,-Wreturn-stack-address]
return {view_ptr(_table_schema)};
^~~~~~~~~~~~~~~~~~~~~~~~~
```
Signed-off-by: Kefu Chai <kefu.chai@scylladb.com>
despite that RapidJSON is a header-only library, we still need to
find it and "link" against it for adding the include directory.
Signed-off-by: Kefu Chai <kefu.chai@scylladb.com>
turns out we are using static variables to register entries in
global registries, and these variables are not directly referenced,
so linker just drops them when linking the executables or shared
libraries. to address this problem, we just link the whole archive.
another option would be create a linker script or pass
--undefined=<symbol> to linker. neither of them is straightforward.
a helper function is introduced to do this, as we cannot use CMake
3.24 as yet.
Signed-off-by: Kefu Chai <kefu.chai@scylladb.com>
if ANTLR3's header files are not installed into the /usr/include, or
other directories searched by compiler by default. there are chances,
we cannot build the tree. so we have to find it first. as /opt/scylladb
is the directory where `scylla-antlr35-c++-dev` is installed on
debian derivatives, this directory is added so the find package module
can find the header files.
```
In file included from /home/kefu/dev/scylla/db/legacy_schema_migrator.cc:38:
In file included from /home/kefu/dev/scylla/cql3/util.hh:21:
/home/kefu/dev/scylla/build/cmake/cql3/CqlParser.hpp:55:10: fatal error: 'antlr3.hpp' file not found
^~~~~~~~~~~~
```
Signed-off-by: Kefu Chai <kefu.chai@scylladb.com>
otherwise, we'd have
```
In file included from /home/kefu/dev/scylla/redis/keyspace_utils.cc:19:
In file included from /home/kefu/dev/scylla/db/query_context.hh:14:
In file included from /home/kefu/dev/scylla/cql3/query_processor.hh:24:
In file included from /home/kefu/dev/scylla/lang/wasm_instance_cache.hh:19:
/home/kefu/dev/scylla/lang/wasm.hh:14:10: fatal error: 'rust/wasmtime_bindings.hh' file not found
^~~~~~~~~~~~~~~~~~~~~~~~~~~
```
Signed-off-by: Kefu Chai <kefu.chai@scylladb.com>
This commit makes the following changes to the docs landing page:
- Adds the ScyllaDB enterprise docs as one of three tiles.
- Modifies the three tiles to reflect the three flavors of ScyllaDB.
- Moves the "New to ScyllaDB? Start here!" under the page title.
- Renames "Our Products" to "Other Products" to list the products other
than ScyllaDB itself. In addtition, the boxes are enlarged from to
large-4 to look better.
The major purpose of this commit is to expose the ScyllaDB
documentation.
docs: fix the link
Closes#13065
Implementation of task_manager's task that covers major keyspace compaction
on one shard.
Closes#12662
* github.com:scylladb/scylladb:
test: extend major keyspace compaction tasks test
compaction: create task manager's task for major keyspace compaction on one shard
- build: cmake: extract more subsystem out into its own CMakeLists.txt
- build: cmake: remove swagger_gen_files
- build: cmake: remove stale TODO comments
- build: cmake: expose scylla_gen_build_dir
- build: cmake: link against cryptopp
- build: cmake: add missing source to utils
- build: cmake: move lib sources into test-lib
- build: cmake: add test/perf
Closes#13059
* github.com:scylladb/scylladb:
build: cmake: add expr_test test
build: cmake: allow test to specify the sources
build: cmake: add test/perf
build: cmake: move lib sources into test-lib
build: cmake: add missing source to utils
build: cmake: link against cryptopp
build: cmake: expose scylla_gen_build_dir
build: cmake: remove stale TODO comments
build: cmake: remove swagger_gen_files
build: cmake: extract more subsystem out into its own CMakeLists.txt
Fixes#12810
We did not update total_size_on_disk in commitlog totals when use o_dsync was off.
This means we essentially ran with no registered footprint, also causing broken comparisons in delete_segments.
Closes#12950
* github.com:scylladb/scylladb:
commitlog: Fix updating of total_size_on_disk on segment alloc when o_dsync is off
commitlog: change type of stored size
This method requires callers to remember that the sstable is the collection of files on a filesystem and to know what exact directory they are all in. That's not going to work for object storage, instead, sstable should be moved between more abstract states.
This PR replaces move_to_new_dir() call with the change_state() one that accepts target sub-directory string and moves files around. Currently supported state changes:
* staging -> normal
* upload -> normal | staging
* any -> quarantine
All are pretty straightforward and move files between table basedir subdirectories with the exception that upload -> quarantine should move into upload/quarantine subdirectory. Another thing to keep in mind, that normal state doesn't have its subdir but maps directory to table's base directory.
Closes#12648
* github.com:scylladb/scylladb:
sstable: Remove explicit quarantization call
test: Move move_to_new_dir() method from sstable class
sstable, dist.-loader: Introduce and use pick_up_from_upload() method
sstables, code: Introduce and use change_state() call
distributed_loader: Let make_sstables_available choose target directory
some tests are compiled from more source files, so add an extra
parameter, so they can customize the sources.
Signed-off-by: Kefu Chai <kefu.chai@scylladb.com>
due to circular dependency: the .cc files under the root of
project references the symbols defined by the source files under
subdirectories, but the source files under subdirectories also
reference the symbols defined by the .cc files under the root
of project, the targets in test/perf do not compile. but
the general structure is created.
Signed-off-by: Kefu Chai <kefu.chai@scylladb.com>
since we include cryptopp/ headers, we need find it and link against
it explicitly, instead of relying on seastar to do this.
Signed-off-by: Kefu Chai <kefu.chai@scylladb.com>
instead of using a while loop for finding the lower_bound,
just use std::lower_bound() for finding if current node owns given
token. this has two advantages:
* better readability: as lower_bound is exactly what this loop
calculates.
* lower_bound uses binary search for searching the element,
this algorithm should be faster than linear under most
circumstances.
* lower_bound uses std::advance() and prefix increment operator,
this should be more performant than the postfix increment operator.
as it does not create an temporary instance of iterator.
Signed-off-by: Kefu Chai <kefu.chai@scylladb.com>
Closes#13008
data_type_for() is a function template that converts a C++
type to a database dynamic type (data_type object).
Instead of implementing a function per type, implement a variable
template instance. This is shorter and nicer.
Since the original type variables (e.g. long_type) are defined separately,
use a reference instead of copying to avoid initialization order problems.
To catch misuses of data_type_for the general data_type_for_v variable
template maps to some unused tag type which will cause a build error
when instantiated.
The original motivation for this was to allow for partial
specialization of data_type_for() for tuple types, but this isn't
really workable since the native type for tuples is std::vector<data_value>,
not std::tuple, and I only checked this after getting the work done,
so this isn't helping anything; it's just a little nicer.
Closes#13043
This PR adds a note to the Alternator TTL section to specify in which Open Source and Enterprise versions the feature was promoted from experimental to non-experimental.
The challenge here is that OSS and Enterprise are (still) **documented together**, but they're **not in sync** in promoting the TTL feature: it's still experimental in 5.1 (released) but no longer experimental in 2022.2 (to be released soon).
We can take one of the following approaches:
a) Merge this PR with master and ask the 2022.2 users to refer to master.
b) Merge this PR with master and then backport to branch-5.1. If we choose this approach, it is necessary to backport https://github.com/scylladb/scylladb/pull/11997 beforehand to avoid conflicts.
I'd opt for a) because it makes more sense from the OSS perspective and helps us avoid mess and backporting.
Closes#12295
* github.com:scylladb/scylladb:
doc: fix the version in the comment on removing the note
doc: specify the versions where Alternator TTL is no longer experimental
This small series reorganizes the existing functional tests for aggregation (min, max, count) and adds additional tests for sum reproducing the strange (but Cassandra-compatible) behavior described in issue #13027.
Closes#13038
* github.com:scylladb/scylladb:
cql-pytest: add tests for sum() aggregate
test/cql-pytest: move aggregation tests to one file
query_id currently lives query-request.hh, a busy place
with lots of dependencies. In turn it gets pulled by
uuid.idl.hh, which is also very central. This makes
test/raft/randomized_nemesis_test.cc which is nominally
only dependent on Raft rebuild on random header file changes.
Fix by extracting into a new header.
Closes#13042
The series fixes the `make_nonforwardable` reader, it shouldn't emit `partition_end` for previous partition after `next_partition()` and `fast_forward_to()`
Fixes: #12249Closes#12978
* github.com:scylladb/scylladb:
flat_mutation_reader_test: cleanup, seastar::async -> SEASTAR_THREAD_TEST_CASE
make_nonforwardable: test through run_mutation_source_tests
make_nonforwardable: next_partition and fast_forward_to when single_partition is true
make_forwardable: fix next_partition
flat_mutation_reader_v2: drop forward_buffer_to
nonforwardable reader: fix indentation
nonforwardable reader: refactor, extract reset_partition
nonforwardable reader: add more tests
nonforwardable reader: no partition_end after fast_forward_to()
nonforwardable reader: no partition_end after next_partition()
nonforwardable reader: no partition_end for empty reader
row_cache: pass partition_start though nonforwardable reader
- treewide: do not define/capture unused variables
- sstables/sstables: mark dummy variable for loop [[maybe_unused]]
- util/result_try: reference this explicitly
- raft: reference this explicitly
- idl-compiler: mark captured this used
- build: reenable unused-{variable,lambda-capture} warnings
Closes#12915
* github.com:scylladb/scylladb:
build: reenable unused-{variable,lambda-capture} warnings
test: reader_concurrency_semaphore_test: define target_memory in debug mode
api::failure_detector: mark set_phi_convict_threshold unimplemented
test: memtable_test: mark dummy variable for loop [[maybe_unused]]
idl-compiler: mark captured this used
raft: reference this explicitly
util/result_try: reference this explicitly
sstables/sstables: mark dummy variable for loop [[maybe_unused]]
treewide: do not define/capture unused variables
service: storage_service: clear _node_ops in batch
Since commit 73e258fc34, Scylla has partial
verification for the CLUSTERING ORDER BY clause in CREATE MATERIALIZED
VIEW. Specifically, invalid column names are rejected. But for reasons
explained in issue #12936 and in the test in this patch, Cassandra
demands that if CLUSTERING ORDER BY appears it must list all the
clustering columns, with no duplicates, and do so in the right order.
This patch replaces an existing test which suggested it is fine
(an extention over Cassandra) to accept a partial list of clustering
columns, by a test that verifies that such a partial list, or an
incorrectly-ordered list, or list with duplicates, should be rejected.
The new test fails on Scylla, and passes on Cassandra, so marked as xfail.
Refs #12936.
Signed-off-by: Nadav Har'El <nyh@scylladb.com>
Closes#12938
This pr fixes the seastar::rpc::closed_error error in the test_topology suite and enables RBNO by default.
Closes#12970
* github.com:scylladb/scylladb:
Revert "Revert "storage_service: Enable Repair Based Node Operations (RBNO) by default for all node ops""
storage_service: Wait for normal state handler to finish in replace
storage_service: Wait for normal state handler to finish in bootstrap
Before this patch, all scripts which use test/cql-pytest/run.py
looked for the Scylla executable as their first step. This is usually
the right thing to do, except in two cases where Scylla is *not* needed:
1. The script test/cql-pytest/run-cassandra.
2. The script test/alternator/run with the "--aws" option.
So in this patch we change run.py to only look for Scylla when actually
needed (the find_scylla() function is called). In both cases mentioned
above, find_scylla() will never get called and the script can work even
if Scylla was never built.
Signed-off-by: Nadav Har'El <nyh@scylladb.com>
Closes#13010
- build: cmake: support thrift < 0.11.0
- build: cmake: find Thrift before using it
- build: cmake: find libxcrypt before using it
- build: cmake: expose scylla_gen_build_dir from "interface"
- build: cmake: extract thrift out
- build: cmake: link the whole auth
- build: cmake: extract mutation,db,replica,streaming out
Closes#12990
* github.com:scylladb/scylladb:
build: cmake: extract mutation,db,replica,streaming out
build: cmake: link the whole auth
build: cmake: extract thrift out
build: cmake: expose scylla_gen_build_dir from "interface"
build: cmake: find libxcrypt before using it
build: cmake: find Thrift before using it
build: cmake: support thrift < 0.11.0
now that all -Wunused-{variable,lambda-capture} warnings are taken
care of. let's reenable these warnings so they can help us to identify
potential issues.
Signed-off-by: Kefu Chai <kefu.chai@scylladb.com>
