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>
Nowadays they are all scattered along the .do_with() function. Keeping
them in one early place makes it possible to relocate them onto the
cql_test_env later
Signed-off-by: Pavel Emelyanov <xemul@scylladb.com>
It's now reference, but some time later it won't be able to get
initialized construction-time, so turn it into pointer
Signed-off-by: Pavel Emelyanov <xemul@scylladb.com>
The topology coordinator only marks a replaced node as LEFT
during the replace operation and actually removes it from the
group 0 config in cleanup_group0_config_if_needed. If this
function is called before raft has committed a replacing node as
a voter, it does not remove the replaced node from the group 0
config. Then, the coordinator can decide that it has no work to
do and starts sleeping, leaving us with an outdated config.
This behavior reduces group 0 availability and causes problems in
tests (see #14885). Also, it makes the coordinator's logic
confusing - it claims that it has no work to do when it has some
work to do. Therefore, we modify the coordinator so that it
removes the replaced node earlier in handle_topology_transition.
Fixes#14885Fixes#14975Closes#15009
This series fixes a couple of review comments on #14845Closes#14976
* github.com:scylladb/scylladb:
gossiper: lock_endpoint: fix comment regarding permit_id mismatch
gossiper: lock_endpoint: change assert to on_internal_error
there is chance that the default port of 9000 has been used on the host
running the test, in that case, we should try to use another available
port.
so, in this change, we try ports in the ranges of [9000, 9000+1000), and
use the first one which is not connectable.
Fixes#14985
Signed-off-by: Kefu Chai <kefu.chai@scylladb.com>
Closes#14997
* github.com:scylladb/scylladb:
test: stop using HostRegistry in MinioServer
s3/client: check for available port before starting minio server
test.py schedules calls to cluster .uninstall() and .stop() making
double calls to it running at the same time. Mark the cluster as not
running early on.
While there, do the same for .stop_gracefully() for consistency.
Signed-off-by: Alejo Sanchez <alejo.sanchez@scylladb.com>
Closes#14987
Currently, `mark_dead` is called with null_permit_id
from `convict`, and in this case, by contract,
it should lock the endpoint, same as `mark_as_shutdown`.
This change somehow escaped #14845 so it amends it.
Fixes#14838Closes#15001
* github.com:scylladb/scylladb:
gossiper: verify permit_id in all private functions
gossiper: convict: lock_endpoint
Instead of acquiring the permit is the permit_id arg is null,
like in mark_as_shutdown, just asssert that the permit_id is
non-null. The functions are both private and we control all callers.
Signed-off-by: Benny Halevy <bhalevy@scylladb.com>
Currently, `mark_dead` is called with null_permit_id
from `convict`, and in this case, by contract,
it should lock the endpoint, same as `mark_as_shutdown`.
This change somehow escaped #14845 so it amends it.
Fixes#14838
Signed-off-by: Benny Halevy <bhalevy@scylladb.com>
since MinioServer find a free port by itself, there is no need to
provide it an IP address for it anymore -- we can always use
127.0.0.1.
so, in this change, we just drop the HostRegistry parameter passed
to the constructor of MinioServer, and pass the host address in place
of it.
Signed-off-by: Kefu Chai <kefu.chai@scylladb.com>
The `system.group0_history` table provides useful descriptions for each
command committed to Raft group 0. One way of applying a command to
group 0 is by calling `migration_manager::announce`. This function has
the `description` parameter set to empty string by default. Some calls
to `announce` use this default value which causes `null` values in
`system.group0_history`. We want `system.group0_history` to have an
actual description for every command, so we change all default
descriptions to reasonable ones.
Going further, We remove the default value for the `description`
parameter of `migration_manager::announce` to avoid using it in the
future. Thanks to this, all commands in `system.group0_history` will
have a non-null description.
Fixes#13370Closes#14979
* github.com:scylladb/scylladb:
migration_manager: announce: remove the default value of description
test: always pass empty description to migration_manager::announce
migration_manager: announce: provide descriptions for all calls
there is chance that the default port of 9000 has been used on the
host running the test, in that case, we should try to use another
available port.
so, in this change, we try ports in the ranges of [9000, 9000+1000),
and use the first one which is not connectable.
Fixes#14985
Signed-off-by: Kefu Chai <kefu.chai@scylladb.com>
Currently `sstable_requiring_cleanup` is updated using `compacting_sstable_registration`, but that mechanism is not used by offstrategy compaction, leading to #14304.
This series introduces `compaction_manager::on_compaction_completion` that intercepts the call
to the table::on_compaction_completion. This allows us to update `sstable_requiring_cleanup` right before the compacted sstables are deleted, making sure they are no leaked to `sstable_requiring_cleanup`, which would hold a reference to them until cleanup attempts to clean them up.
`cleanup_incremental_compaction_test` was adjusted to observe the sstables `on_delete` (by adding a new observer event) to detect the case where cleanup attempts to delete the leaked sstables and fails since they were already deleted from the file system by offstrategy compaction. The test fails with the fix and passes with it.
Fixes#14304Closes#14858
* github.com:scylladb/scylladb:
compaction_manager: on_compaction_completion: erase sstables from sstables_requiring_cleanup
compaction/leveled_compaction_strategy: ideal_level_for_input: special case max_sstable_size==0
sstable: add on_delete observer
compaction_manager: add on_compaction_completion
sstable_compaction_test: cleanup_incremental_compaction_test: verify sstables_requiring_cleanup is empty
before this change, we use generator expression to initialize CMAKE_CXX_FLAGS_RELEASE, this has two problems:
1. the generator expression is not expanded when setting a regular variable.
2. the ending ">" is missing in one of the generator expression.
3. the parameters are not separated with ";"
so address them, let's just
* use `add_compile_options()` directly, as the corresponding `mode.${build_mode}.cmake` is only included when the "${build_mode}" is used.
* add comma in between the command line options.
* add the missing closing ">"
Closes#14996
* github.com:scylladb/scylladb:
build: cmake: pass --gc-sections to ld not ar
build: cmake: use add_compile_options() in release build
when it comes to `regular_compaction_task_executor`, we repeat the
compaction until the compaction can not proceed, so after an iteration
of compaction completes successfully, the task can still continue with
yet another round of the compaction as it sees appropriate. so let's
update the comment to reflect this fact.
Signed-off-by: Kefu Chai <kefu.chai@scylladb.com>
Closes#14891
quite a few member variables serves as the configuration for
a given compaction, they are immutable in the life cycle of it,
so for better readability, let's mark them `const`.
Signed-off-by: Kefu Chai <kefu.chai@scylladb.com>
Closes#14981
The only place that still calls it is static force_blocking_flush method. If can be made non-static already. Also, while at it, coroutinize some system_keyspace methods and fix a FIXME regarding replica::database access in it.
Closes#14984
* github.com:scylladb/scylladb:
code: Remove query-context.hh
code: Remove qctx
system_keyspace: Use system_keyspace's container() to flush
system_keyspace: Make force_blocking_flush() non-static
system_keyspace: Coroutinize update_tokens()
system_keyspace: Coroutinize save_truncation_record()
ar is not able to tell which sections to be GC'ed, hence it does
not care about --gc-sections, but ld does. let's add this option
to CMAKE_EXE_LINKER_FLAGS.
Signed-off-by: Kefu Chai <kefu.chai@scylladb.com>
before this change, we use generator expression to initialize
CMAKE_CXX_FLAGS_RELEASE, this has two problems:
1. the generator expression is not expanded when setting
a regular variable.
2. the ending ">" is missing in one of the generator
expression.
3. the parameters are not separated with ";"
so address them, let's just
* use `add_compile_options()` directly, as the corresponding
`mode.${build_mode}.cmake` is only included when the
"${build_mode}" is used.
* add comma in between the command line options.
* add the missing closing ">"
Signed-off-by: Kefu Chai <kefu.chai@scylladb.com>
In force_blocking_flush() there's an invoke-on-all invocation of
replica::database::flush() and a FIXME to get the replica database from
somewhere else rather than via query-processor -> data_dictionary.
Since now the force_blocking_flush() is non-static the invoke-on-all can
happen via system_keyspace's container and the database can be obtained
directly from the sys.ks. local instance
Signed-off-by: Pavel Emelyanov <xemul@scylladb.com>
Erase retired sstable from compaction_state::sstables_requiring_cleanup
also on_compaction_completion (in addition to
compacting_sstable_registration::release_compacting
for offstrategy compaction with piggybacked cleanup
or any other compaction type that doesn't use
compacting_sstable_registration.
Add cleanup_during_offstrategy_incremental_compaction_test
that is modeled after cleanup_incremental_compaction_test to check
that cleanup doesn't attempt to cleanup already-deleted
sstables that were left over by offstrategy compaction
in sstables_requiring_cleanup.
Fixes#14304
Signed-off-by: Benny Halevy <bhalevy@scylladb.com>
Prevent div-by-zero byt returning const level 1
if max_sstable_size is zero, as configured by
cleanup_incremental_compaction_test, before it's
extended to cover also offstrategy compaction.
Signed-off-by: Benny Halevy <bhalevy@scylladb.com>
Pass the call to the table on_compaction_completion
so we can manage the sstables requiring cleanup state
along the way.
Signed-off-by: Benny Halevy <bhalevy@scylladb.com>
We remove the default value for the description parameter of
migration_manager::announce to avoid using it in the future.
Thanks to this, all commands in system.group0_history will
have a non-null description.
In the next commit, we remove the default value for the
description parameter of migration_manager::announce to avoid
using it in the future. However, many calls to announce in tests
use the default value. We have to change it, but we don't really
care about descriptions in the tests, so we pass the empty string
everywhere.
The system.group0_history table provides useful descriptions
for each command committed to Raft group 0. One way of applying
a command to group 0 is by calling migration_manager::announce.
This function has the description parameter set to empty string
by default. Some calls to announce use this default value which
causes null values in system.group0_history. We want
system.group0_history to have an actual description for every
command, so we change all default descriptions to reasonable ones.
We can't provide a reasonable description to announce in
query_processor::execute_thrift_schema_command because this
function is called in multiple situations. To solve this issue,
we add the description parameter to this function and to
handler::execute_schema_command that calls it.
While in SQL DISTINCT applies to the result set, in CQL it applies
to the table being selected, and doesn't allow GROUP BY with clustering
keys. So reject the combination like Cassandra does.
While this is not an important issue to fix, it blocks un-xfailing
other issues, so I'm clearing it ahead of fixing those issues.
An issue is unmarked as xfail, and other xfails lose this issue
as a blocker.
Fixes#12479Closes#14970
Knowing that a server gained or lost leadership in group 0 is
sometimes useful for the purpose of debugging, so we log
information about these events on the INFO level.
Gaining and losing leadership are relatively rare events, so
this change shouldn't flood the logs.
Closes#14877
For `removenode`, we make a removed node a non-voter early. There is no
downside to it because the node is already dead. Moreover, it improves
availability in some situations.
For `decommission`, if we decommission a node when the number of nodes
is even, we make it a non-voter early to improve availability. All
majorities containing this node will remain majorities when we make this
node a non-voter and remove it from the set because the required size of
a majority decreases.
We don't change `decommission` when the number of nodes is odd since
this may reduce availability.
Fixes#13959Closes#14911
* github.com:scylladb/scylladb:
raft: make a decommissioning node a non-voter early
raft: topology_coordinator: implement step_down_as_nonvoter
raft: make a removed node a non-voter early
Rewrite test that checks whether task_manager/wait_task works properly.
The old version didn't work. Delete functions used in old version.
Closes#14959
* github.com:scylladb/scylladb:
test: rewrite wait_task test
test: move ThreadWrapper to rest_util.py
When deleting multiple sstables with the same prefix
the deletion atomicity is ensured by the pending_delete_log file,
so if scylla crashes in the middle, deletions will be replyed on
restart.
Therefore, we don't have to ensure atomicity of each individual
`unlink`. We just need to sync the directory once, before
removing the pending_delete_log file.
Signed-off-by: Benny Halevy <bhalevy@scylladb.com>
Closes#14967
This makes it possible to remove remaining users of the global qctx.
The thing is that db::schema_tables code needs to get wasm's engine, alien runner and instance cache to build wasm context for the merged function or to drop it from cache in the opposite case. To get the wasm stuff, this code uses global qctx -> query_processor -> wasm chain. However, the functions (un)merging code already has the database reference at hand, and its natural to get wasm stuff from it, not from the q.p. which is not available
So this PR packs the wasm engine, runner and cache on sharded<wasm::manager> instance, makes the manager be referenced by both q.p. and database and removes the qctx from schema tables code
Closes#14933
* github.com:scylladb/scylladb:
schema_tables: Stop using qctx
database: Add wasm::manager& dependency
main, cql_test_env, wasm: Start wasm::manager earlier
wasm: Shuffle context::context()
wasm: Add manager::remove()
wasm: Add manager::precompile()
wasm: Move stop() out of query_processor
wasm: Make wasm sharded<manager>
query_processor: Wrap wasm stuff in a struct
The metrics are registered on-demand when load-balancer is invoked, so that only leader exports the metrics. When leader changes, the old leader will stop exporting.
The metrics are divided into two levels: per-dc and per-node. In prometheus, they will have appropriate labels for dc and host_id values.
Closes#14962
* github.com:scylladb/scylladb:
tablet_allocator: unregister metrics when leadership is lost
tablets: load_balancer: Export metrics
service, raft: Move balance_tablets() to tablet_allocator
tablet_allocator: Start even if tablets feature is not enabled
main, storage_service: Pass tablet allocator to storage_service
Before the patch, tablet metadata update was processed on local schema merge
before table changes.
When table is dropped, this means that for a while table will exist
without a corresponding tablet map. This can cause memtable flush for
this table to fail, resulting in intentional abort(). That's because
sstable writing attempts to access tablet map to generate sharding
metadata.
If auto_snapshot is enabled, this is much more likely to happen,
because we flush memtables on table drop.
To fix the problem, process tablet metadata after dropping tables, but
before creating tables.
Fixes#14943Closes#14954
