Many sstable test cases create tempdir on their own to create sstables with. Sometimes it's justified when the test needs to check files on disk by hand for some validation, but often all checks are fs-agnostic. The latter case(s) can be patched to work on top of any storage, in particular -- on top of object storage. To make it work tests should stop creating sstables explicitly in tempdir and this PR does exactly that.
All relevant occurrences of tempdir are removed from test cases, instead the sstable::test_env's tempdir is used. Next, the test_env::{create_sstable|reusable_sst} are patched not to accept the `fs::path dir` argument and pick the env's tempdir. Finally, the `make_sstable_easy` helper is patched to use path-less env methods too.
refs: #13015Closes#13116
* github.com:scylladb/scylladb:
test,sstables: Remove path from make_sstable_easy()
test,lib: Remove wrapper over reusable_sst and move the comment
test: Make "compact" test case use env dir
test,compaction: Use env tempdir in some more cases
test,compaction: Make check_compacted_sstables() use env's dir
test: Relax making sstable with sequential generation
test/sstable::test_env: Keep track of auto-incrementing generation
test/lib: Add sstable maker helper without factory
test: Remove last occurrence of test_env::do_with(rval, ...)
test,sstables: Dont mess with tempdir where possible
test/sstable::test_env: Add dir-less sstables making helpers
test,sstables: Use sstables::test_env's tempdir with sweeper
test,sstables: Use sstables::test_env's tempdir
test/lib: Add tempdir sweeper
test/lib: Open-code make_sstabl_easy into make_sstable
test: Remove vector of mutation interposer from test_key_count_estimation
- sstables: mark param of sstable::*_from_sstring() const
- sstables: mark param of reverse_map() const
- sstables: mark static lookup table const
Closes#13115
* github.com:scylladb/scylladb:
sstables: mark static lookup table const
sstables: mark param of reverse_map() const
sstables: mark param of sstable::*_from_sstring() const
This reverts commit 49e0d0402d, reversing
changes made to 25cf325674.
An old version of PR #13115 was accidentally merged into `master` (it
was dequeued concurrently while a running next promotion job included
it).
Revert the merge. We'll merge the new version as a follow-up.
Major compaction can be started from both storage_service and column_family
api. The first allows to compact a subset of tables in given keyspace,
while the latter - given table in given keyspace.
As major compaction started from storage_service has a wider scope,
we use its mechanisms for column_family's one. That makes it more consistent
and reduces number of classes that would be needed to cover the major
compaction with task manager's tasks.
I've no idea why the quotes are there at all, it works even without
them. However, with quotes gdb-13 fails to find the _all_threads static
thread-local variable _unless_ it's printed with gdb "p" command
beforehand.
fixes: #13125
Signed-off-by: Pavel Emelyanov <xemul@scylladb.com>
Closes#13132
these tables are mappings from symbolic names to their string
representation. we don't mutate them. so mark them const.
Signed-off-by: Kefu Chai <kefu.chai@scylladb.com>
it does not mutate the map in which the value is looked up, so let's
mark map const. also, take this opportunity to use structured binding
for better readability.
Signed-off-by: Kefu Chai <kefu.chai@scylladb.com>
The components_writer class from this list doesn't even exist
Also drop the forward declaration of mx::partition_reversing_data_source_impl
Signed-off-by: Pavel Emelyanov <xemul@scylladb.com>
Closes#13097
Related: https://github.com/scylladb/scylladb/issues/13119
This commit removes the pages that describe Enterprise only features
from the Open Source documentation:
- Encryption at Rest
- Workload Prioritization
- LDAP Authorization
- LDAP Authentication
- Audit
In addition, it removes most of the information about Incremental
Compaction Strategy (ICS), which is replaced with links to the
Enterprise documentation.
The changes above required additional updates introduced with this
commit:
- The links to Enterprise-only features are replaced with the
corresponding links in the Enterprise documentation.
- The redirections are added for the removed pages to be redirected to
the corresponding pages in the Enterprise documentation.
This commit must be reverted in the scylla-enterprise repository to
avoid deleting the Enterprise-only content from the Enterprise docs.
Closes#13123
- sstables: remove unused function
- sstables: mark param of sstable::*_from_sstring() const
- sstables: mark param of reverse_map() const
- sstables: mark static lookup table const
Closes#13115
* github.com:scylladb/scylladb:
sstables: mark static lookup table const
sstables: mark param of reverse_map() const
sstables: mark param of sstable::*_from_sstring() const
sstables: remove unused function
The method in question is only called with env's tempdir, so there's no
point in explicitly passing it.
Signed-off-by: Pavel Emelyanov <xemul@scylladb.com>
There's a wonderful comment describing what the reusable_sst is for near
one of its wrappers. It's better to drop the wrapper and move the
comment to where it belongs.
Signed-off-by: Pavel Emelyanov <xemul@scylladb.com>
Both already do so, but get the tempdir explicitly. It's possible to
make them much shorter by not carrying this variable over the code.
Signed-off-by: Pavel Emelyanov <xemul@scylladb.com>
It's in fact using it already via argument. Next patch will do the same
with another call, but having this change separately makes the next
patch shorter and easier to review.
Signed-off-by: Pavel Emelyanov <xemul@scylladb.com>
Many test cases populate sstable with a factory that at the same time
serves as a stable maintainer of a monitomic generation. Those can be
greately relaxed by re-using the recently introduced generation from the
test_env.
Signed-off-by: Pavel Emelyanov <xemul@scylladb.com>
Lots of test cases make sstables with monotonically incrementing
generation values. In Scylla code this counter is maintained in class
table, but sstable tests not always have it. To mimic this behavior, the
test_env can keep track of the generation, so that callers just don't
mess with it (next patch).
Signed-off-by: Pavel Emelyanov <xemul@scylladb.com>
There's a make_sstable_containing() helper that creates sstable and
populates it with mutations (and makes some post validation). The helper
accepts a factory function that should make sstable for it.
This patch shuffles this helper a bit by introducing an overload that
populates (and validates) the already existing sstable.
Signed-off-by: Pavel Emelyanov <xemul@scylladb.com>
There's the lonely test case that uses the mentioned template to carry
its own instance of tempdir over its lifetime. Patch the case to re-use
the already existing env's tempdir and drop the template.
Signed-off-by: Pavel Emelyanov <xemul@scylladb.com>
Beneficiary of the previuous patch -- those cases that make sstables in
env's tempdir can now enjoy not mentioning this explicitly and letting
the env specify the sstable making path itself.
Signed-off-by: Pavel Emelyanov <xemul@scylladb.com>
Lots of (most of) test cases out there generate sstables inside env's
temporary directory. This patch adds some sugar to env that will allow
test cases omit explicit env.tempdir() call.
Signed-off-by: Pavel Emelyanov <xemul@scylladb.com>
Continuation of the previous patch. Some test cases are sensitive to
having the temp directory clean, so patch them similarly, but equip with
the sweeper on entry instead of their own temprid instance.
Signed-off-by: Pavel Emelyanov <xemul@scylladb.com>
The one is maintained by the env throughout its lifetime. For many test
cases there's no point in generating tempdir on their own, so just
switch to using env's one.
The code gets longer lines, but this is going to change really soon.
Signed-off-by: Pavel Emelyanov <xemul@scylladb.com>
This is a RAII-sh helper that cleans temp directory on destruction. To
be used in cases when a test needs to do several checks over clean
temporary directory (future patches).
Signed-off-by: Pavel Emelyanov <xemul@scylladb.com>
The former helper is going to get rid of the fs::path& dir argument,
but the latter cannot yet live without it. The simplest solution is to
open-code the helper until better times.
Signed-off-by: Pavel Emelyanov <xemul@scylladb.com>
The test generates a vector of mutation to be later passed into
make_sstable() helper which just applies them to memtable. The test case
can generate memtable directly. This makes it possible to stop using the
local tempdir in this test case by future patches.
Signed-off-by: Pavel Emelyanov <xemul@scylladb.com>
these tables are mappings from symbolic names to their string
representation. we don't mutate them. so mark them const.
Signed-off-by: Kefu Chai <kefu.chai@scylladb.com>
it does not mutate the map in which the value is looked up, so let's
mark map const. also, take this opportunity to use structured binding
for better readability.
Signed-off-by: Kefu Chai <kefu.chai@scylladb.com>
- api: reference httpd::* symbols like 'httpd::*'
- alternator: using chrono_literals before using it
- api: s/request/http::request/
the last two commits were inspired Pavel's comment of
> It looks like api/ code was caught by some using namespace seastar::httpd shortcut.
they should be landed before we merge and include https://github.com/scylladb/seastar/pull/1536 in Scylla.
Closes#13095
* github.com:scylladb/scylladb:
api: reference httpd::* symbols like 'httpd::*'
alternator: using chrono_literals before using it
api: s/request/http::request/
- distributed_loader: print log without using fmt::format()
- distributed_loader: correct a typo in comment
Closes#13108
* github.com:scylladb/scylladb:
distributed_loader: correct a typo in comment
distributed_loader: print log without using fmt::format()
Our documentation states that writing an item with "USING TTL 0" means it
should never expire. This should be true even if the table has a default
TTL. But Scylla mistakenly handled "USING TTL 0" exactly like having no
USING TTL at all (i.e., it took the default TTL, instead of unlimited).
We had two xfailing tests demonstrating that Scylla's behavior in this
is different from Cassandra. Scylla's behavior in this case was also
undocumented.
By the way, Cassandra used to have the same bug (CASSANDRA-11207) but
it was fixed already in 2016 (Cassandra 3.6).
So in this patch we fix Scylla's "USING TTL 0" behavior to match the
documentation and Cassandra's behavior since 2016. One xfailing test
starts to pass and the second test passes this bug and fails on a
different one. This patch also adds a third test for "USING TTL ?"
with UNSET_VALUE - it behaves, on both Scylla and Cassandra, like a
missing "USING TTL".
The origin of this bug was that after parsing the statement, we saved
the USING TTL in an integer, and used 0 for the case of no USING TTL
given. This meant that we couldn't tell if we have USING TTL 0 or
no USING TTL at all. This patch uses an std::optional so we can tell
the case of a missing USING TTL from the case of USING TTL 0.
Fixes#6447
Signed-off-by: Nadav Har'El <nyh@scylladb.com>
Closes#13079
logger.info() is able to format the given arguments with the format
string, so let's just let it do its job.
Signed-off-by: Kefu Chai <kefu.chai@scylladb.com>
In issue #5283 we noted that the auto_snapshot option is not useful
in Alternator (as we don't offer any API to restore the snapshot...),
and suggested that we should automatically disable this option for
Alternator tables. However, this issue has been open for more than three
years, and we never changed this default.
So until we solve that issue - if we ever do - let's add a paragraph
in docs/alternator/alternator.md recommending to the user to disable
this option in the configuration themselves. The text explains why,
and also provides a link to the issue.
Refs #5283
Signed-off-by: Nadav Har'El <nyh@scylladb.com>
Closes#13103
Add a test which performs an UPDATE and
tries to pass an UNSET_VALUE as a value
for the primary key.
There is also an LWT variant of this test
that tries to set an UNSET_VALUE
in the IF condition.
These two tests are analogous to
test_insert_update_where and
test_insert_update_where_lwt,
but use an UPDATE instead of INSERT.
It's useful to test UPDATE as well as INSERT.
When I was developing a fix for #13001
I initially added the condition for unset value
inside insert_statement, but this didn't handle
update statements. These two tests allowed me
to see that UPDATE still causes a crash.
Signed-off-by: Jan Ciolek <jan.ciolek@scylladb.com>
Closes#13058
Undefined behavior because the evaluation order is undefined.
With GCC, where evaluation is right-to-left, schema will be moved
once it's forwarded to make_flat_mutation_reader_from_mutations_v2().
The consequence is that memory tracking of mutation_fragment_v2
(for tracking only permit used by view update), which uses the schema,
can be incorrect. However, it's more likely that Scylla will crash
when estimating memory usage for row, which access schema column
information using schema::column_at(), which in turn asserts that
the requested column does really exist.
Fixes#13093.
Signed-off-by: Raphael S. Carvalho <raphaelsc@scylladb.com>
Closes#13092
When Fedora 37 came out, we discovered that its "pytest" script started
to run Python with the "-s" option, which caused problems for packages
installed personally via pip. We fixed this by adding our own wrapper
script test/pytest.
But this bug (https://bugzilla.redhat.com/show_bug.cgi?id=2152171) was
already fixed in Fedora 37, and the new version already reached our
dbuild. So we no longer need this wrapper script. Let's remove it.
Fixes#12412
Signed-off-by: Nadav Har'El <nyh@scylladb.com>
Closes#13083
We have seen users unintentionally use RF=1 or RF=2 for a keyspace.
We would like to have an option for a minimal RF that is allowed.
Cassandra recently added, in Cassandra 4.1 (see apache/cassandra@5fdadb2
and https://issues.apache.org/jira/browse/CASSANDRA-14557), exactly such
a option, called "minimum_keyspace_rf" - so we chose to use the same option
name in Scylla too. This means that unlike the previous "safe mode"
options, the name of this option doesn't start with "restrict_".
The value of the minimum_keyspace_rf option is a number, and lower
replication factors are rejected with an error like:
cqlsh> CREATE KEYSPACE x WITH REPLICATION = { 'class' : 'SimpleStrategy',
'replication_factor': 2 };
ConfigurationException: Replication factor replication_factor=2 is
forbidden by the current configuration setting of minimum_keyspace_rf=3.
Please increase replication factor, or lower minimum_keyspace_rf set in
the configuration.
This restriction applies to both CREATE KEYSPACE and ALTER KEYSPACE
operations. It applies to both SimpleStrategy and NetworkTopologyStrategy,
for all DCs or a specific DC. However, a replication factor of zero (0)
is *not* forbidden - this is the way to explicitly request not to
replicate (at all, or in a specific DC).
For the time being, minimum_keyspace_rf=0 is still the default, which
means that any replication factor is allowed, as before. We can easily
change this default in a followup patch.
Note that in the current implementation, trying to use RF below
minimum_keyspace_rf is always an error - we don't have a syntax
to make into just a warning. In any case the error message explains
exactly which configuration option is responsible for this restriction.
Fixes#8891.
Signed-off-by: Nadav Har'El <nyh@scylladb.com>
Closes#9830
This patch fixes a problem which affects decommission and removenode
which may lead to data consistency problems under conditions which
lead one of the nodes to unliaterally decide to abort the node
operation without the coordinator noticing.
If this happens during streaming, the node operation coordinator would
proceed to make a change in the gossiper, and only later dectect that
one of the nodes aborted during sending of decommission_done or
removenode_done command. That's too late, because the operation will
be finalized by all the nodes once gossip propagates.
It's unsafe to finalize the operation while another node aborted. The
other node reverted to the old topolgy, with which they were running
for some time, without considering the pending replica when handling
requests. As a result, we may end up with consistency issues. Writes
made by those coordinators may not be replicated to CL replicas in the
new topology. Streaming may have missed to replicate those writes
depending on timing.
It's possible that some node aborts but streaming succeeds if the
abort is not due to network problems, or if the network problems are
transient and/or localized and affect only heartbeats.
There is no way to revert after we commit the node operation to the
gossiper, so it's ok to close node_ops sessions before making the
change to the gossiper, and thus detect aborts and prevent later aborts
after the change in the gossiper is made. This is already done during
bootstrap (RBNO enabled) and replacenode. This patch canges removenode
to also take this approach by moving sending of remove_done earlier.
We cannot take this approach with decommission easily, because
decommission_done command includes a wait for the node to leave the
ring, which won't happen before the change to the gossiper is
made. Separating this from decommission_done would require protocol
changes. This patch adds a second-best solution, which is to check if
sessions are still there right before making a change to the gossiper,
leaving decommission_done where it was.
The race can still happen, but the time window is now much smaller.
The PR also lays down infrastructure which enables testing the scenarios. It makes node ops
watchdog periods configurable, and adds error injections.
Fixes#12989
Refs #12969Closes#13028
* github.com:scylladb/scylladb:
storage_service: node ops: Extract node_ops_insert() to reduce code duplication
storage_service: Make node operations safer by detecting asymmetric abort
storage_service: node ops: Add error injections
service: node_ops: Make watchdog and heartbeat intervals configurable
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.
Closes#13096
* github.com:scylladb/scylladb:
build: enable more warnings
test: do not initialize plain number with {}
test: do not initialize a time_t with braces
The code for compare_endpoints originates at the dawn of time (bc034aeaec)
and is called on the fast path from storage_proxy via `sort_by_proximity`.
This series considerably reduces the function's footprint by:
1. carefully coding the many comparisons in the function so to reduce the number of conditional banches (apparently the compiler isn't doing a good enough job at optimizing it in this case)
2. avoid sstring copy in topology::get_{datacenter,rack}
Closes#12761
* github.com:scylladb/scylladb:
topology: optimize compare_endpoints
to_string: add print operators for std::{weak,partial}_ordering
utils: to_sstring: deinline std::strong_ordering print operator
move to_string.hh to utils/
test: network_topology: add test_topology_compare_endpoints
One test in test/cql-pytest/test_batch.py accidentally had the asyncio
marker, despite not using any async features. Remove it. The test still
runs fine.
Signed-off-by: Nadav Har'El <nyh@scylladb.com>
Closes#13002
- build: cmake: use different names for output of check_cxx_compiler_flag
- build: cmake: only add supported warning flags to CMAKE_CXX_FLAGS
- build: cmake: limit the number of link job
Closes#13098
* github.com:scylladb/scylladb:
build: cmake: limit the number of link job
build: cmake: only add supported warning flags to CMAKE_CXX_FLAGS
build: cmake: use different names for output of check_cxx_compiler_flag
it turns out we have `using namespace httpd;` in seastar's
`request_parser.rl`, and we should not rely on this statement to
expose the symbols in `seatar::httpd` to `seastar` namespace.
in this change,
* api/*.hh: all httpd symbols are referenced by `httpd::*`
instead of being referenced as if they are in `seastar`.
* api/*.cc: add `using namespace seastar::httpd`.
Signed-off-by: Kefu Chai <kefu.chai@scylladb.com>
we should assume that some included header does this for us.
we'd have following compiling failure if seastar's
src/http/request_parser.rl does not `using namespace httpd;` anymore.
```
/home/kefu/dev/scylladb/alternator/streams.cc:433:55: error: no matching literal operator for call to 'operator""h' with argument of type 'unsigned long long' or 'const char *', and no matching literal operator template
static constexpr auto dynamodb_streams_max_window = 24h;
^
```
Signed-off-by: Kefu Chai <kefu.chai@scylladb.com>
