before this change, object_store/test_basic.py create a config file
for specifying the object storage settings, and pass the path of this
file as the argument of `--object-storage-config-file` option when
running scylla. we have the same requirement when testing scylla
with minio server, where we launch a minio server and manually
create a the config file and feed it to scylla.
to ease the preparation work, let's consolidate by creating the
config file in `minio_server.py`, so it always creates the config
file and put it in its tempdir. since object_store/test_basic.py
can also run against an S3 bucket, the fixture implemented
object_store/conftest.py is updated accordingly to reuse the
helper exposed by MinioServer to create the config file when it
is not available.
Signed-off-by: Kefu Chai <kefu.chai@scylladb.com>
If `live_updatable_config_params_changeable_via_cql` is set to true, configuration parameters defined with `liveness::LiveUpdate` option can be updated in the runtime with CQL, i.e. by updating `system.config` virtual table.
If we don't want any configuration parameter to be changed in the
runtime by updating `system.config` virtual table, this option should be
set to false. This option should be set to false for e.g. cloud users,
who can only perform CQL queries, and should not be able to change
scylla's configuration on the fly.
Current implemenatation is generic, but has a small drawback - messages
returned to the user can be not fully accurate, consider:
```
cqlsh> UPDATE system.config SET value='2' WHERE name='task_ttl_in_seconds';
WriteFailure: Error from server: code=1500 [Replica(s) failed to execute write] message="option is not live-updateable" info={'failures': 1, 'received_responses': 0, 'required_responses': 1, 'consistency': 'ONE'}
```
where `task_ttl_in_seconds` has been defined with
`liveness::LiveUpdate`, but because `live_updatable_config_params_changeable_via_cql` is set to
`false` in `scylla.yaml,` `task_ttl_in_seconds` cannot be modified in the
runtime by updating `system.config` virtual table.
Fixes#14355Closes#14382
Before compaction task executors started inheriting from
compaction_task_impl, they were destructed immediately after
compaction finished. Destructors of executors and their
fields performed actions that affected global structures and
statistics and had impact on compaction process.
Currently, task executors are kept in memory much longer, as their
are tracked by task manager. Thus, destructors are not called just
after the compaction, which results in compaction stats not being
updated, which causes e.g. infinite cleanup loop.
Add release_resources() method which is called at the end
of compaction process and does what destructors used to.
Fixes: #14966.
Fixes: #15030.
Closes#15005
should have been use `ignore_errors=True` to ignore
the error. this issue has not poped up, because
we haven't run into the case where the log file
does not exist.
this was a regression introduced by
d4ee84ee1e
Signed-off-by: Kefu Chai <kefu.chai@scylladb.com>
Closes#15063
An sstable can be in one of several states -- normal, quarantined, staging, uploading. Right now this "state" is hard-wired into sstable's path, e.g. quarantined sstable would sit in e.g. /var/lib/data/ks-cf-012345/quarantine/ directory. Respectively, there's a bunch of directory names constexprs in sstables.hh defining each "state". Other than being confusing, this approach doesn't work well with S3 backend. Additionally, there's snapshot subdir that adds to the confusion, because snapshot is not quite a state.
This PR converts "state" from constexpr char* directories names into a enum class and patches the sstable creation, opening and state-changing API to use that enum instead of parsing the path.
refs: #13017
refs: #12707Closes#14152
* github.com:scylladb/scylladb:
sstable/storage: Make filesystem storage with initial state
sstable: Maintain state
sstable: Make .change_state() accept state, not directory string
sstable: Construct it with state
sstables_manager: Remove state-less make_sstable()
table: Make sstables with required state
test: Make sstables with upload state in some cases
tools: Make sstables with normal state
table: Open-code sstables making streaming helpers
tests: Make sstables with normal state by default
sstable_directory: Make sstable with required state
sstable_directory: Construct with state
distributed_loader: Make sstable with desired state when populating
distributed_loader: Make sstable with upload state when uploading
sstable: Introduce state enum
sstable_directory: Merge verify and g.c. calls
distributed_loader: Merge verify and gc invocations
sstable/filesystem: Put underscores to dir members
sstable/s3: Mark make_s3_object_name() const
sstable: Remove filename(dir, ...) method
and use that in compaction_group, rather than
respective accumulators of its own.
This is part of of larger series to make cache updates exception safe.
Refs #14043Closes#15052
* github.com:scylladb/scylladb:
sstable_set: maintain total bytes_on_disk
sstable_set: insert, erase: return status
there is a small time window after we find a free port and before
the minio server listens on that port, if another server sneaked
in the time window and listen on that port, minio server can
still fail to start even there might be free port for it.
so, in this change, we just retry with a random port for a fixed
number of times until the minio server is able to serve.
Signed-off-by: Kefu Chai <kefu.chai@scylladb.com>
Closes#15042
The use-after-move is not very harmful as it's only used when
handling exception. So user would be left with a bogus message.
Signed-off-by: Raphael S. Carvalho <raphaelsc@scylladb.com>
Closes#15054
before this change, `scylla sstable dump-statistics` prints the
"regular_columns" as a list of strings, like:
```
"regular_columns": [
"name",
"clustering_order",
"type_name",
"org.apache.cassandra.db.marshal.UTF8Type",
"name",
"column_name_bytes",
"type_name",
"org.apache.cassandra.db.marshal.BytesType",
"name",
"kind",
"type_name",
"org.apache.cassandra.db.marshal.UTF8Type",
"name",
"position",
"type_name",
"org.apache.cassandra.db.marshal.Int32Type",
"name",
"type",
"type_name",
"org.apache.cassandra.db.marshal.UTF8Type"
]
```
but according
https://opensource.docs.scylladb.com/stable/operating-scylla/admin-tools/scylla-sstable.html#dump-statistics,
> $SERIALIZATION_HEADER_METADATA := {
> "min_timestamp_base": Uint64,
> "min_local_deletion_time_base": Uint64,
> "min_ttl_base": Uint64",
> "pk_type_name": String,
> "clustering_key_types_names": [String, ...],
> "static_columns": [$COLUMN_DESC, ...],
> "regular_columns": [$COLUMN_DESC, ...],
> }
>
> $COLUMN_DESC := {
> "name": String,
> "type_name": String
> }
"regular_columns" is supposed to be a list of "$COLUMN_DESC".
the same applies to "static_columnes". this schema makes sense,
as each column should be considered as a single object which
is composed of two properties. but we dump them like a list.
so, in this change, we guard each visit() call of `json_dumper()`
with `StartObject()` and `EndObject()` pair, so that each column
is printed as an object.
after the change, "regular_columns" are printed like:
```
"regular_columns": [
{
"name": "clustering_order",
"type_name": "org.apache.cassandra.db.marshal.UTF8Type"
},
{
"name": "column_name_bytes",
"type_name": "org.apache.cassandra.db.marshal.BytesType"
},
{
"name": "kind",
"type_name": "org.apache.cassandra.db.marshal.UTF8Type"
},
{
"name": "position",
"type_name": "org.apache.cassandra.db.marshal.Int32Type"
},
{
"name": "type",
"type_name": "org.apache.cassandra.db.marshal.UTF8Type"
}
]
```
Fixes#15036
Signed-off-by: Kefu Chai <kefu.chai@scylladb.com>
Closes#15037
This reverts commit 70b5360a73. It generates
a failure in group0_test .test_concurrent_group0_modifications in debug
mode with about 4% probability.
Fixes#15050
and use that in compaction_group, rather than
respective accumulators of its own.
bytes_on_disk is implemented by each sstable_set_impl
and is update on insert and erase (whether directly
into the sstable_set_impl or via the sstable_set).
Although compound_sstable_set doesn't implement
insert and erase, it override `bytes_on_disk()` to return
the sum of all the underlying `sstable_set::bytes_on_disk()`.
Also, added respective unit tests for `partitioned_sstable_set`
and `time_series_sstable_set`, that test each type's
bytes_on_disk, including cloning of the set, and the
`compound_sstable_set` bytes_on_disk semantics.
Signed-off-by: Benny Halevy <bhalevy@scylladb.com>
perform_offstrategy is called from try_perform_cleanup
when there are sstables in the maintenance set that require
cleanup.
The input sstables are inserted into the compaction_state
`sstables_requiring_cleanup` and `try_perform_cleanup`
expects offstrategy compaction to clean them up along
with reshape compaction.
Otherwise, the maintenance sstables that require cleanup
are not cleaned up by cleanup compaction, since
the reshape output sstable(s) are not analyzed again
after reshape compaction, where that would insert
the output sstable(s) into `sstables_requiring_cleanup`
and trigger their cleanup in the subsequent cleanup compaction.
The latter method is viable too, but it is less effficient
since we can do reshape+cleanup in one pass, vs.
reshape first and cleanup later.
Fixes scylladb/scylladb#15041
Signed-off-by: Benny Halevy <bhalevy@scylladb.com>
Closes#15043
Document how SELECT clauses are considered. For example, given the query
SELECT * FROM tab WHERE a = 3 LIMIT 1
We'll get different results if we first apply the WHERE clause then LIMIT
the result set, or if we first LIMIT there result set and then apply the
WHERE clause.
Closes#14990
discard_result ignores only successful futures. Thus, if
perform_compaction<regular_compaction_task_executor> call fails,
a failure is considered abandoned, causing tests to fail.
Explicitly ignore failed future.
Fixes: #14971.
Closes#15000
The filesystem storage driver uses different paths depending on sstable
state. It's possible to keep only table directory _and_ state on it and
construct this path on demand when needed, but it's faster to keep full
path onboard. All the more so it's only exported outside via .prefix()
call which is for logs only, but still
Signed-off-by: Pavel Emelyanov <xemul@scylladb.com>
This means -- keep state on sstable, change it with change_state() call
and (!) fix the is_<state>() helpers not to check storage->prefix()
nit: mark requires_view_building() noexcept while at it
Signed-off-by: Pavel Emelyanov <xemul@scylladb.com>
Pretty cosmetic change, but it will allow S3 to finally support moving
sstables between states (after this patch it still doesn't)
Signed-off-by: Pavel Emelyanov <xemul@scylladb.com>
This just moves make_path() call from outside of sstable::sstable()
inside it. Later it will be moved even further. Also, now sstable can
know its state and keep it (next patch)
Signed-off-by: Pavel Emelyanov <xemul@scylladb.com>
Now all callers specify the state they want their sstables in explicitly
and the old API can be removed
Signed-off-by: Pavel Emelyanov <xemul@scylladb.com>
By default it's created with normal state, but there are some places
that need to put it into staging. Do it with new state enum
Signed-off-by: Pavel Emelyanov <xemul@scylladb.com>
As was mentione in the previous patch, there are few places in tests
that put sstables in upload/ subdir and they really mean it. Those need
to use sstables manager/directory API directly (already) and specify the
state explicitly (this patch)
Signed-off-by: Pavel Emelyanov <xemul@scylladb.com>
Just like tests, tool open sstable by its full path and doesn't make any
assumptions about sstable state
Signed-off-by: Pavel Emelyanov <xemul@scylladb.com>
There are two of those that call each other to end up calling plain
make_sstable() one. It's simpler to patch both if they just call the
latter directly.
While at it -- drop the unused default argument.
Signed-off-by: Pavel Emelyanov <xemul@scylladb.com>
It's assumed that sstables are not very specific about which
subdirectory an sstable is, so they can use normal state. Places that
need to move sstables between states will use sstable manager API
explicitly
Signed-off-by: Pavel Emelyanov <xemul@scylladb.com>
The state is on sstable_directory, can switch to using the new manager
API. The full path is still there, will be dropped later
Signed-off-by: Pavel Emelyanov <xemul@scylladb.com>
This is to replace full path sitting on this object eventually. For now
they have to co-exist, but state will be used to make_sstable()-s from
manager with its new API
Signed-off-by: Pavel Emelyanov <xemul@scylladb.com>
This still needs to conver state to directory name internally as
sstable_directory instances are hashed on populator by subdir string.
Also the full string path is printed in logs. All this is now internal
to populate method and will be fixed later
Signed-off-by: Pavel Emelyanov <xemul@scylladb.com>
There are several states between which an sstable can migrate. Nowadays
the state is encoded into sstable directory, which is not nice. Also S3
backed sstables don't support states only keeping sstables in "normal".
This patch adds enum state in order to replace the path-encoded one
eventually. The new sstables_manager::make_sstable() method is added
that accepts table directory (without quarantine/ or staging/ component)
and the desired initial state (optional). Next patches will make use of
this maker and the existing one will be removed.
Signed-off-by: Pavel Emelyanov <xemul@scylladb.com>
It's only used by fs storage driver that can do dir/file concatenation
on its own. Moreover, this method is not welcome to be used even
internally
Signed-off-by: Pavel Emelyanov <xemul@scylladb.com>
Both, init_system_keyspace() and init_non_system_keyspaces() populate
the keyspaces with the help of distributed_loader::populate_keyspace().
That method, in turn, walks the list of keyspaces' tables to load
sstables from disk and attach to them.
After it both init_...-s take the 2nd pass over keyspaces' tables to
call the table::mark_ready_for_writes() on each. This marking can be
moved into populate_keyspace(), that's much easier and shorter because
that method already has the shard-wide table pointer and can just call
whatever it needs on the table.
This changes the initialization sequence, before the patch all tables
were populated before any of them was marked as ready for write. This
looks safe however, as marking a table for write meaks resetting its
generation generator and different tables' generators are independent
from each other.
Signed-off-by: Pavel Emelyanov <xemul@scylladb.com>
Closes#15026
To add a sharded service to the cql_test_env one needs to patch it in 5 or 6 places
- add cql_test_env reference
- add cql_test_env constructor argument
- initialize the reference in initializer list
- add service variable to do_with method
- pass the variable to cql_test_env constructor
- (optionally) export it via cql_test_env public method
Steps 1 through 5 are annoying, things get much simpler if look like
- add cql_test_env variable
- (optionally) export it via cql_test_env public method
This is what this PR does
refs: #2795Closes#15028
* github.com:scylladb/scylladb:
cql_test_env: Drop local *this reference
cql_test_env: Drop local references
cql_test_env: Move most of the stuff in run_in_thread()
cql_test_env: Open-code env start/stop and remove both
cql_test_env: Keep other services as class variables
cql_test_env: Keep services as class variables
cql_test_env: Construct env early
cql_test_env: De-static fdpinger variable
cql_test_env: Define all services' variables early
cql_test_env: Keep group0_client pointer
We see the abort_requested_exception error from time
to time, instead of sleep_aborted that was expected
and quietly ignored (in debug log level).
Treat abort_requested_exception the same way since
the error is expected on shutdown and to reduce
test flakiness, as seen for example, in
https://jenkins.scylladb.com/job/scylla-master/job/scylla-ci/3033/artifact/logs-full.release.010/1691896356104_repair_additional_test.py%3A%3ATestRepairAdditional%3A%3Atest_repair_schema/node2.log
```
INFO 2023-08-13 03:12:29,151 [shard 0] compaction_manager - Asked to stop
WARN 2023-08-13 03:12:29,152 [shard 0] gossip - failure_detector_loop: Got error in the loop, live_nodes={}: seastar::sleep_aborted (Sleep is aborted)
INFO 2023-08-13 03:12:29,152 [shard 0] gossip - failure_detector_loop: Finished main loop
WARN 2023-08-13 03:12:29,152 [shard 0] cdc - Aborted update CDC description table with generation (2023/08/13 03:12:17, d74aad4b-6d30-4f22-947b-282a6e7c9892)
INFO 2023-08-13 03:12:29,152 [shard 1] compaction_manager - Asked to stop
INFO 2023-08-13 03:12:29,152 [shard 1] compaction_manager - Stopped
INFO 2023-08-13 03:12:29,153 [shard 0] init - Signal received; shutting down
INFO 2023-08-13 03:12:29,153 [shard 0] init - Shutting down view builder ops
INFO 2023-08-13 03:12:29,153 [shard 0] view - Draining view builder
INFO 2023-08-13 03:12:29,153 [shard 1] view - Draining view builder
INFO 2023-08-13 03:12:29,153 [shard 0] compaction_manager - Stopped
ERROR 2023-08-13 03:12:29,153 [shard 0] view - start failed: seastar::abort_requested_exception (abort requested)
ERROR 2023-08-13 03:12:29,153 [shard 1] view - start failed: seastar::abort_requested_exception (abort requested)
```
Signed-off-by: Benny Halevy <bhalevy@scylladb.com>
Closes#15029
Currently we hold group0_guard only during DDL statement's execute()
function, but unfortunately some statements access underlying schema
state also during check_access() and validate() calls which are called
by the query_processor before it calls execute. We need to cover those
calls with group0_guard as well and also move retry loop up. This patch
does it by introducing new function to cql_statement class take_guard().
Schema altering statements return group0 guard while others do not
return any guard. Query processor takes this guard at the beginning of a
statement execution and retries if service::group0_concurrent_modification
is thrown. The guard is passed to the execute in query_state structure.
Fixes: #13942
Message-ID: <ZNSWF/cHuvcd+g1t@scylladb.com>
There are some asserting checks for keyspace and table existence on cql_test_env that perform some one-linee work in a complex manner, tests can do better on their own. Removing it makes cql_test_env simpler
refs: #2795Closes#15027
* github.com:scylladb/scylladb:
test: Remove require_..._exists from cql_test_env
test: Open-code ks.cf name parse into cdc_test
test: Don't use require_table_exists() in test/lib/random_schema
test: Use BOOST_REQUIRE(!db.has_schema())
test: Use BOOST_REQUIRE(db.has_schema())
test: Use BOOST_REQUIRE(db.has_keyspace())
test: Threadify cql_query_test::test_compact_storage case
test: Threadify some cql_query_test cases
The local auto& foo = env._foo references in run_in_thread() a no longer
needed, the code that uses foo can be switched to use _foo (this->_foo)
instead
Signed-off-by: Pavel Emelyanov <xemul@scylladb.com>
Thw do_with() method is static and cannot just access cql_test_env
variable's fields, using local references instead. To simplify this,
most of the method's content is moved to non-static run_in_thread()
method
Signed-off-by: Pavel Emelyanov <xemul@scylladb.com>
There are more services on do_with() stack that are not referenced from
the cql_test_env. Move them to be class variables too
Signed-off-by: Pavel Emelyanov <xemul@scylladb.com>
Now they are duplicated -- variables exist on do_with() stack and the
class references some of them. This patch makes is vice-versa -- all the
variables are on the cql_test_env and do_with() references them. The
latter will change soon
Signed-off-by: Pavel Emelyanov <xemul@scylladb.com>
