in 7952200c, we changed the `selected_format` from `mc` to `me`,
but to be backward compatible the cluster starts with "md", so
when the nodes in cluster agree on the "ME_SSTABLE_FORMAT" feature,
the format selector believes that the node is already using "ME",
which is specified by `_selected_format`. even it is actually still
using "md", which is specified by `sstable_manager::_format`, as
changed by 54d49c04. as explained above, it was specified to "md"
in hope to be backward compatible when upgrading from an existign
installation which might be still using "md". but after a second
thought, since we are able to read sstables persisted with older
formats, this concern is not valid.
in other words, 7952200c introduced a regression which changed the
"default" sstable format from `me` to `md`.
to address this, we just change `sstable_manager::_format` to "me",
so that all sstables are created using "me" format.
a test is added accordingly.
Fixes#18995
Signed-off-by: Kefu Chai <kefu.chai@scylladb.com>
Closesscylladb/scylladb#19293
This patch adds a check if aggregation query is doing single-partition read and if so, makes the query to not use forward_service and do not parallelize the request.
Fixesscylladb/scylladb#19349Closesscylladb/scylladb#19350
* github.com:scylladb/scylladb:
test/boost/cql_query_test: add test for single-partition aggregation
cql3/select_statement: do not parallelize single-partition aggregations
flat_mutation_reader_v2 was introduced in a pair of commits in 2021:
e3309322c3 "Clone flat_mutation_reader related classes into v2 variants"
08b5773c12 "Adapt flat_mutation_reader_v2 to the new version of the API"
as a replacement for flat_mutation_reader, using range_tombstone_change
instead of range_tombstone to represent represent range tombstones. See
those commits for more information.
The transition was incremental; the last use of the original
flat_mutation_reader was removed in 2022 in commit
026f8cc1e7 "db: Use mutation_partition_v2 in mvcc"
In turn, flat_mutation_reader was introduced in 2017 in commit
748205ca75 "Introduce flat_mutation_reader"
To transition from a mutation_reader that nested rows within
a partition in a separate stream, to a flat reader that streamed
partitions and rows in the same stream.
Here, we reclaim the original name and rename the awkward
flat_mutation_reader_v2 to mutation_reader.
Note that mutation_fragment_v2 remains since we still use the original
for compatibilty, sometimes.
Some notes about the transition:
- files were also renamed. In one case (flat_mutation_reader_test.cc), the
rename target already existed, so we rename to
mutation_reader_another_test.cc.
- a namespace 'mutation_reader' with two definitions existed (in
mutation_reader_fwd.hh). Its contents was folded into the mutation_reader
class. As a result, a few #includes had to be adjusted.
Closesscylladb/scylladb#19356
Normally, the space overhead for TWCS is 1/N, where is number of windows. But during off-strategy, the overhead is 100% because input sstables cannot be released earlier.
Reshaping a TWCS table that takes ~50% of available space can result in system running out of space.
That's fixed by restricting every TWCS off-strategy job to 10% of free space in disk. Tables that aren't big will not be penalized with increased write amplification, as all input (disjoint) sstables can still be compacted in a single round.
Fixes#16514.
Closesscylladb/scylladb#18137
* github.com:scylladb/scylladb:
compaction: Reduce twcs off-strategy space overhead to 10% of free space
compaction: wire storage free space into reshape procedure
sstables: Allow to get free space from underlying storage
replica: don't expose compaction_group to reshape task
so that "wasm" target is built. "wasm" generates the text format
of wasm code. and these wasm applications are used by the test_wasm
tests.
the rules generated by `configure.py` adds these .wat files as a
dependency of `{mode}-build`, which is in turn a dependency of `{mode}`.
in this change, let's mirror this behavior by making `wasm` ALL,
so it is built by the default target.
Signed-off-by: Kefu Chai <kefu.chai@scylladb.com>
Closesscylladb/scylladb#19391
since we've switched almost all callers of the operator<< to {fmt}, let's drop the unused operator<<:s.
there are more occurrences of unused operator<< in the tree, but let's do the cleanup piecemeal.
---
this is a cleanup, so no need to backport
Closesscylladb/scylladb#19346
* github.com:scylladb/scylladb:
types: remove unused operator<<
node_ops: remove unused operator<<
lang: remove unused operator<<
gms: remove unused operator<<
dht: remove unused operator<<
test: do not use operator<< for std::optional
Currently if task_manager::task::impl::abort preempts before children are recursively aborted and then the task gets unregistered, we hit use after free since abort uses children vector which is no longer alive.
Modify abort method so that it goes over all tasks in task manager and aborts those with the given parent.
Fixes: #19304.
Requires backport to all versions containing task manager
Closesscylladb/scylladb#19305
* github.com:scylladb/scylladb:
test: add test for abort while a task is being unregistered
tasks: fix tasks abort
Before these changes, compilation was failing with the following
error:
In file included from test/boost/hint_test.cc:12:
/usr/include/fmt/ranges.h:298:7: error: no member named 'parse' in 'fmt::formatter<db::hints::sync_point::host_id_or_addr>'
298 | f.parse(ctx);
| ~ ^
We add the missing callback.
Closesscylladb/scylladb#19375
Replace the reserve_partial loop in large_bitset constructor with a new function - reserve_gently() that can reserve memory without stalling by repeatedly calling reserve_partial() method of the passed container.
Closesscylladb/scylladb#19361
* github.com:scylladb/scylladb:
utils/large_bitset: replace reserve_partial with utils::reserve_gently
utils/stall_free: introduce reserve_gently
Add reserve_gently() that can reserve memory without stalling by
repeatedly calling reserve_partial() method of the passed container.
Update the comments of existing reserve_partial() methods to mention
this newly introduced reserve_gently() wrapper.
Also, add test to verify the functionality.
Signed-off-by: Lakshmi Narayanan Sreethar <lakshmi.sreethar@scylladb.com>
Related: https://github.com/scylladb/scylladb/issues/17851
Fix the issue that test logs were not deleted
Fix the issue that the URL to the failed test directory was incorrectly shown even when artifacts_dir_url option was not provided
Fix the issue that there were no node logs when it failed to join the cluster
Closesscylladb/scylladb#19115
* github.com:scylladb/scylladb:
[test.py] Fix logs had multiplication of lines
[test.py] Fix log not deleted
[test.py] Fix log for failed node was nod added to failed directory
[test.py] Fix URl for failed logs directory in CI
Since the test name was not unique across the run and when we were using a --repeat option, there were several handlers for the same file. With this change test name and accordingly, the log name will be different for the same test but different repeat case. Remove mode from the test name since it's already in mode directory.
One of the created log files was not deleted at all, because there was no delete command. Unlink moved on later stage explicitly after removing the handler that writing to this file to avoid the possibility that something will be added after removing the file.
in 94cdfcaa94, we added commitlog_cleanup_test to `configure.py`,
but didn't add it to the CMake building system.
in this change, let's add it to the CMake building system.
Signed-off-by: Kefu Chai <kefu.chai@scylladb.com>
Closesscylladb/scylladb#19314
In issue #15561 some doubts were raised regarding the way ScyllaDB sorts
UUID values. This patch adds a heavily-commented cql-pytest test that
helps understand - and verify that understanding - of the way Scylla sorts
UUIDs, and shows there is some reason in the madness (in particular,
Version 1 UUIDs (time uuids) are sorted like timeuuids, and not as byte
arrays.
The new tests check the different cases (see the comments in the test),
and as usual for cql-pytest tests - they passes also on Cassandra, which
allows us to confirm that the sort order we used is identical to the one
used by Cassandra and not something that Scylla mis-implemented.
Having this test in our suite will also ensure that the UUID ordering
never changes accidentally in the future. If it ever changes, it can
break access to existing tables that use UUID clustering keys, so
it shouldn't change.
Fixes#15561
Signed-off-by: Nadav Har'El <nyh@scylladb.com>
Closesscylladb/scylladb#19343
Making the count resources on the maintenance (streaming) semaphore live update via config. This will allow us to improve repair speed on mixed-shard clusters, where we suspect that reader trashing -- due to the combination of high number of readers on each shard and very conservative reader count limit (10) -- is the main cause of the slowness.
Making this count limit confgurable allows us to start experimenting with this fix, without committing to a count limit increase (or removal), addressing the pain in the field.
Refs: #18269
No OSS backport needed.
Closesscylladb/scylladb#19248
* github.com:scylladb/scylladb:
replica/database: wire in maintenance_reader_concurrency_semaphore_count_limit
db/config: introduce maintenance_reader_concurrency_semaphore_count_limit
reader_concurrency_semaphore: make count parameter live-update
since we've switched almost all callers of the operator<< to {fmt},
let's drop the unused operator<<:s.
Signed-off-by: Kefu Chai <kefu.chai@scylladb.com>
since we've switched almost all callers of the operator<< to {fmt},
let's drop the unused operator<<:s.
Signed-off-by: Kefu Chai <kefu.chai@scylladb.com>
we don't provide it anymore, and if any of existing type provides
constructor accepting an `optional<>`, and hence can be formatted
using operator<< after converting it, neither shall we rely on this
behavior, as it is fragile.
so, in this change, we switch to `fmt::print()` to use {fmt} to
print `optional<>`.
Signed-off-by: Kefu Chai <kefu.chai@scylladb.com>
thrift support was deprecated since ScyllaDB 5.2
> Thrift API - legacy ScyllaDB (and Apache Cassandra) API is
> deprecated and will be removed in followup release. Thrift has
> been disabled by default.
so let's drop it. in this change,
* thrift protocol support is dropped
* all references to thrift support in document are dropped
* the "thrift_version" column in system.local table is preserved for backward compatibility, as we could load from an existing system.local table which still contains this clolumn, so we need to write this column as well.
* "/storage_service/rpc_server" is only preserved for backward compatibility with java-based nodetool.
Fixes#3811Fixes#18416
Signed-off-by: Kefu Chai <kefu.chai@scylladb.com>
- [x] not a fix, no need to backport
Closesscylladb/scylladb#18453
* github.com:scylladb/scylladb:
config: expand on rpc_keepalive's description
api: s/rpc/thrift/
db/system_keyspace: drop thrift_version from system.local table
transport: do not return client_type from cql_server::connection::make_client_key()
treewide: drop thrift support
cql3: always return created event in create ks/table/type/view statement
In case multiple clients issue concurrently CREATE KEYSPACE IF NOT EXISTS
and later USE KEYSPACE it can happen that schema in driver's session is
out of sync because it synces when it receives special message from
CREATE KEYSPACE response.
Similar situation occurs with other schema change statements.
In this patch we fix only create keyspace/table/type/view statements
by always sending created event. Behavior of any other schema altering
statements remains unchanged.
Fixes https://github.com/scylladb/scylladb/issues/16909
**backport: no, it's not a regression**
Closesscylladb/scylladb#18819
* github.com:scylladb/scylladb:
cql3: always return created event in create ks/table/type/view statement
cql3: auth: move auto-grant closer to resource creation code
cql3: extract create ks/table/type/view event code
in 51c53d8db6, we check `self.old_env[env]` for None, but there
are chances `self.old_env` does not contain a value with `env`.
in that case, we'd have following failure:
```
Traceback (most recent call last):
File "/home/kefu/dev/scylladb/test/pylib/minio_server.py", line 307, in <module>
asyncio.run(main())
File "/usr/lib64/python3.12/asyncio/runners.py", line 194, in run
return runner.run(main)
^^^^^^^^^^^^^^^^
File "/usr/lib64/python3.12/asyncio/runners.py", line 118, in run
return self._loop.run_until_complete(task)
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
File "/usr/lib64/python3.12/asyncio/base_events.py", line 687, in run_until_complete
return future.result()
^^^^^^^^^^^^^^^
File "/home/kefu/dev/scylladb/test/pylib/minio_server.py", line 304, in main
await server.stop()
File "/home/kefu/dev/scylladb/test/pylib/minio_server.py", line 274, in stop
self._unset_environ()
File "/home/kefu/dev/scylladb/test/pylib/minio_server.py", line 211, in _unset_environ
if self.old_env[env] is not None:
~~~~~~~~~~~~^^^^^
KeyError: 'S3_CONFFILE_FOR_TEST'
```
this happens if we run `pylib/minio_server.py` as a standalone
application.
in this change, instead of getting the value with index, we use
`dict.get()`, so that it does not throw when the dict does not
have the given key.
Signed-off-by: Kefu Chai <kefu.chai@scylladb.com>
Closesscylladb/scylladb#19291
If something happens during nod adding to the cluster, it will not be registered as a part of the cluster. This leads to situations during log gathering that logs for a such node will be missing.
Incorrect passing of the artifacts_dir_url parameter from test.py to pytest leads to the situation when it will pass None as a string and pytest will generate incorrect URL.
This test in topology_experimental_raft/test_alternator.py wants to
check that during Alternator TTL's expiration scans, ALL of the CPU was
used in the "streaming" scheduling group and not in the "statement"
scheduling group. But to allow for some fluke requests (e.g., from the
driver), the test actually allows work in the statement group to be
up to 1% of the work.
Unfortunately, in one test run - a very slow debug+aarch64 run - we
saw the work on the statement group reach 1.4%, failing the test.
I don't know exactly where this work comes from, perhaps the driver,
but before this bug was fixed we saw more than 58% of the work in the
wrong scheduling group, so neither 1% or 1.4% is a sign that the bug
came back. In fact, let's just change the threshold in the test to 10%,
which is also much lower than the pre-fix value of 58%, so is still a
valid regression test.
Fixes#19307
Signed-off-by: Nadav Har'El <nyh@scylladb.com>
Closesscylladb/scylladb#19323
In CI test always executed with option --repeat=3 that leads to generate 3 test results with the same name. Junit plugin in CI cannot distinguish correctly the difference between these results. In case when we have two passes and one fail, the link to test result will sometimes be redirected to the incorrect one because the test name is the same. To fix this ReportPlugin added that will be responsible to modify the test case name during junit report generation adding to the test name mode and run id.
Fixes: https://github.com/scylladb/scylladb/issues/17851
Fixes: https://github.com/scylladb/scylladb/issues/15973Closesscylladb/scylladb#19235
* github.com:scylladb/scylladb:
[test.py] Add uniqueness to the test name
[test.py] Refactor alternator, nodetool, rest_api
utils/chunked_vector::reserve_partial: fix usage in callers
The method reserve_partial(), when used as documented, quits before the
intended capacity can be reserved fully. This can lead to overallocation
of memory in the last chunk when data is inserted to the chunked vector.
The method itself doesn't have any bug but the way it is being used by
the callers needs to be updated to get the desired behaviour.
Instead of calling it repeatedly with the value returned from the
previous call until it returns zero, it should be repeatedly called with
the intended size until the vector's capacity reaches that size.
This PR updates the method comment and all the callers to use the
right way.
Fixes#19254Closesscylladb/scylladb#19279
* github.com:scylladb/scylladb:
utils/large_bitset: remove unused includes identified by clangd
utils/large_bitset: use thread::maybe_yield()
test/boost/chunked_managed_vector_test: fix testcase tests_reserve_partial
utils/lsa/chunked_managed_vector: fix reserve_partial()
utils/chunked_vector: return void from reserve_partial and make_room
test/boost/chunked_vector_test: fix testcase tests_reserve_partial
utils/chunked_vector::reserve_partial: fix usage in callers
This PR fixes two problems with the `expected_error`
parameter in `server_add` and `servers_add`.
1. It didn't work in `server_add` if the cluster was empty
because of an incorrect attempt to connect the driver.
2. It didn't work in `servers_add` completely because the
`seeds` parameter was handled incorrectly.
This PR only adds improvements in the testing framework,
no need to backport it.
Closesscylladb/scylladb#19255
* github.com:scylladb/scylladb:
test: manager_client, scylla_cluster: fix type annotations in add_servers
test: manager_client: don't connect driver after failed server_{add, start}
test: scylla_cluster: pass seeds to add_servers
In CI test always executed with option --repeat=3 that leads to generate 3 test results with the same name. Junit plugin in CI cannot distinguish correctly the difference between these results. In case when we have two passes and one fail, the link to test result will sometimes be redirected to the incorrect one because the test name is the same.
To fix this ReportPlugin added that will be responsible to modify the test case name during junit report generation adding to the test name mode and run id.
Fixes: https://github.com/scylladb/scylladb/issues/17851
Fixes: https://github.com/scylladb/scylladb/issues/15973
For various reasons, a view building write may fail. When that
happens, the view building should not finish until these writes
are successfully retried and they should not interfere with any
writes that are performed to the base table while the view is
building.
The test introduced in this patch confirms that this is the case.
Refs scylladb/scylladb#19261Closesscylladb/scylladb#19263
Update the maximum size tested by the testcase. The test always created
only one chunk as the maximum size tested by it (1 << 12 = 4KB) was less
than the default max chunk size (12.8 KB). So, use twice the
max_chunk_capacity as the test size distribution upper limit to verify
that partial_reserve can reserve multiple chunks.
Signed-off-by: Lakshmi Narayanan Sreethar <lakshmi.sreethar@scylladb.com>
Fix the method comment and return types of chunked_managed_vector's
reserve_partial() similar to chunked_vector's reserve_partial() as it
has the same issues mentioned in #19254. Also update the usage in the
chunked_managed_vector_test.
Signed-off-by: Lakshmi Narayanan Sreethar <lakshmi.sreethar@scylladb.com>
Fix the usage of reserve_partial in the testcase. Also update the
maximum chunk size used by the testcase. The test always created only
one chunk as the maximum size tested by it (1 << 12 = 4KB) was less
than the default max chunk size (128 KB). So, use smaller chunk size,
512 bytes, to verify that partial_reserve can reserve multiple chunks.
Signed-off-by: Lakshmi Narayanan Sreethar <lakshmi.sreethar@scylladb.com>
Before work on tablets was completed, it was noticed that — due to some missing pieces of implementation — Scylla doesn't properly close sstables for migrated-away tablets. Because of this, disk space wasn't being reclaimed properly.
Since the missing pieces of implementation were added, the problem should be gone now. This patch adds a test which was used to reproduce the problem earlier. It's expected to pass now, validating that the issue was fixed.
Should be backported to branch-6.0, because the tested problem was also affecting that branch.
Fixes#16946Closesscylladb/scylladb#18906
* github.com:scylladb/scylladb:
test_tablets: add test_tablet_storage_freeing
test: pylib: add get_sstables_disk_usage()
TWCS off-strategy suffers with 100% space overhead, so a big TWCS table
can cause scylla to run out of disk space during node ops.
To not penalize TWCS tables, that take a small percentage of disk,
with increased write ampl, TWCS off-strategy will be restricted to
10% of free disk space. Then small tables can still compact all
disjoint sstables in a single round.
Fixes#16514.
Signed-off-by: Raphael S. Carvalho <raphaelsc@scylladb.com>
That will be used in turn to restrict reshape to 10% of available space
in underlying storage.
Signed-off-by: Raphael S. Carvalho <raphaelsc@scylladb.com>
As it turns out, each sstable carries its own schema in its serialization header (Statistics component). This schema is incomplete -- the names of the key columns are not stored, just their type. Static and regular columns do have names and types stored however. This bare-bones schema is enough to parse and display the content of the sstable. Another thing missing is schema options (the stuff after the `WITH` keyword, except the clustering order). The only options stored are the compression options (in the CompressionInfo component), this is actually needed to read the Data component.
This series adds a new method to `tools/schema_loader.cc` to extract the schema stored in the sstable itself. This new schema load method is used as the last fall-back for obtaining the schema, in case scylla-sstable is trying to autodetect the schema of the sstable. Although, right now this bare-bones schema is enough for everything scylla-sstable does, it is more future proof to stick to the "full" schema if possible, so this new method is the last resort for now.
Fixes: https://github.com/scylladb/scylladb/issues/17869
Fixes: https://github.com/scylladb/scylladb/issues/18809
New functionality, no backport needed.
Closesscylladb/scylladb#19169
* github.com:scylladb/scylladb:
tools/scylla-sstable: log loaded schema with trace level
tools/scylla-sstable: load schema from the sstable as fallback
tools/schema_loader: introduce load_schema_from_sstable()
test/lib/random_schema: remove assert on min number of regular columns
sstables: introduce load_metadata()
So that the amount of count resources can be changed at run-time,
triggered by a e.g. a config change.
Previous constant-count based constructor is left intact, to avoid
patching all clients, as only a small subset will want the new
functionality.
This patch includes extensive testing for what happens to an ongoing
paged query when a secondary index is suddenly added or dropped.
Issue #18992 was opened suggesting that this would be broken, and indeed
the tests included here show that it is indeed broken.
The four tests included in this patch are heavily commented to explain
what they are testing and why, but here is a short summary of what is
being tested by each of them:
1. A paged query filtering on v=17 continues correctly even if an
index is created on v.
2. A paged query filtering on v1 and v2 where v2 is indexed,
continues correctly even if an index is created on v1 (remember
that Scylla prefers to use the first index mentioned in the query).
3. A paged query using an index on v continues correctly even if that
index is deleted.
4. However, if the query doesn't say "ALLOW FILTERING", it cannot
be continued after the index is deleted.
All these tests pass on Cassandra, but all of them except the fourth
fail on Scylla, reproducing issue #18992. Somewhat to my suprise, the
failure of the query in all the failed tests is silent (i.e., trying to
fetch the next page just fetches nothing and says the iteration is done).
I was expecting more dramatic failures ("marshaling error" messages,
crashes, etc.) but didn't get them.
Refs #18992
Signed-off-by: Nadav Har'El <nyh@scylladb.com>
Closesscylladb/scylladb#19000
When auto-detecting the schema of the sstable, if all other methods
failed, load the schema from the sstable's serialization header. This
schema is incomplete. It is just enough to parse and display the content
of the sstable. Although parsing and displaying the content of the
sstable is all scylla-sstable does, it is more future-compatible to us
the full schema when possible. So the always-available but minimal
schema that each sstable has on itself, is used just as a fallback.
The test which tested the case when all schema load attempts fail,
doesn't work now, because loading the serialization header always
succeeds. So convert this test into two positive tests, testing the
serialization header schema fallback instead.
