Currently, we have `real_db.tables` and `schemas`, the former containing
system tables needed to parse statements, and the latter accumulating
user tables parsed from CQL. This will be error-prone to maintain with
view/index support, so ditch `schemas` and instead add a `user` flag to
`table` and accumulate all tables in `real_db.tables`.
At the end, just return the schemas of all user tables.
Scylla's schema tables code determines which index was added, by diffing
index definitions with previous ones. This is clunky to use in
tools/schema_loader.cc, so also return the index metadata for the newly
created index.
The methods `validate_while_excuting()` and its only caller,
`build_index_schema()`, only use the query processor to get db from it.
So replace qp parameter with db one, relaxing requirements w.r.t.
callers.
If for some reason an exception is thrown in compaction_manager::remove,
it might leave behind stale table pointers in _compaction_state. Fix
that by setting up a deffered action to perform the cleanup.
Fixes#16635
Signed-off-by: Lakshmi Narayanan Sreethar <lakshmi.sreethar@scylladb.com>
Closesscylladb/scylladb#16632
Refer to the added comment for details.
This problem was found by a compiler warning, and I'm fixing
it mainly to silence the warning. I didn't give any thought
to its effects in practice.
Fixes#13077Closesscylladb/scylladb#16625
[avi: changed Refs to Fixes]
we format `std::variant<std::monostate, seastar::timed_out_error,
raft::not_a_leader, raft::dropped_entry, raft::commit_status_unknown,
raft::conf_change_in_progress, raft::stopped_error, raft::not_a_member>`
in this source file. and currently, we format `std::variant<..>` using
the default-generated `fmt::formatter` from `operator<<`, so in order to
format it using {fmt}'s compile-time check enabled, we have to make the
`operator<<` overload for `std::variant<...>` visible from the caller
sites which format `std::variant<...>` using {fmt}.
in this change, the `operator<<` for `std::variant<...>` is moved to
from the middle of the source file to the top of it, so that it can
be found when the compiler looks up for a matched `fmt::formatter`
for `std::variant<...>`.
please note, we cannot use the `fmt::formatter` provided by `fmt/std.h`,
as its specialization for `std::variant` requires that all the types
of the variant is `is_formattable`. but the default generated formatter
for type `T` is not considered as the proof that `T` is formattable.
this should address the FTBFS with the latest seastar like:
```
/usr/include/fmt/core.h:2743:12: error: call to deleted constructor of 'conditional_t<has_formatter<mapped_type, context>::value, formatter<mapped_type, char_type>, fallback_formatter<stripped_type, char_type>>' (aka 'fmt::detail::fallback_formatter<std::variant<std::monostate, seastar::timed_out_error, raft::not_a_leader, raft::dropped_entry, raft::commit_status_unknown, raft::conf_change_in_progress, raft::stopped_error, raft::not_a_member>>')
```
Signed-off-by: Kefu Chai <kefu.chai@scylladb.com>
Closesscylladb/scylladb#16616
we are using `seastar::format()` to format `append_entry` in
`append_reg_model`, so we have to provide a `fmt::formatter` for
these callers which format `append_entry`.
despite that, with FMT_DEPRECATED_OSTREAM, the formatter is defined
by fmt v9, we don't have it since fmt v10. so this change prepares us
for fmt v10.
Refs https://github.com/scylladb/scylladb/issues/13245Closesscylladb/scylladb#16614
* github.com:scylladb/scylladb:
test: randomized_nemesis_test: add formatter for append_entry
test: randomized_nemesis_test: move append_reg_model::entry out
it was a copy-pasta error introduced by 2508d339. the copyright
blob was copied from a C++ source code, but the CMake language
define the block comment is different from the C++ language.
let's use the line comment of CMake.
Signed-off-by: Kefu Chai <kefu.chai@scylladb.com>
Closesscylladb/scylladb#16615
In 7d5e22b43b ("replica: memtable: don't forget memtable
memory allocation statistics") we taught memtable_list to remember
learned memory allocation reserves so a new memtable inherits these
statistics from an older memtable. Share it now further across tablets
that belong to the same table as well. This helps the statistics be more
accurate for tablets that are migrated in, as they can share existing
tablet's memory allocation history.
Closesscylladb/scylladb#16571
* github.com:scylladb/scylladb:
table, memtable: share log-structured allocator statistics across all memtables in a table
memtable: consolidate _read_section, _allocating_section in a struct
Change the mutate_live_and_unreachable_endpoints procedure
so that the called `func` would mutate a cloned
`live_and_unreachable_endpoints` object in place.
Those are replicated to temporary copies on all shards
using `foreign<unique_ptr<>>` so that the would be
automatically freed on exception.
Only after all copies are made, they are applied
on all gossiper shards in a noexcept loop
and finally, a `on_success` function is called
to apply further side effects if everything else
was replicated successfully.
The latter is still susceptible to exceptions,
but we can live with those as long as `_live_endpoints`
and `_unreachable_endpoints` are synchronized on all shards.
With that, the read-only methods:
`get_live_members_synchronized` and
`get_unreachable_members_synchronized`
become trivial and they just return the required data
from shard 0.
Fixes#15089
Signed-off-by: Benny Halevy <bhalevy@scylladb.com>
Closesscylladb/scylladb#16597
remove the unused #include headers from repair.hh, as they are not
directly used. after this change, task_manager_module.hh fails to
have access to stream_reason, so include it where it is used.
Signed-off-by: Kefu Chai <kefu.chai@scylladb.com>
Closesscylladb/scylladb#16618
it's observed that the mock server could return something not decodable
as JSON. so let's print out the response in the logging message in this case.
this should help us to understand the test failure better if it surfaces again.
Refs #16542
Signed-off-by: Kefu Chai <kefu.chai@scylladb.com>
Closesscylladb/scylladb#16543
we are using `seastar::format()` to format `append_entry` in
`append_reg_model`, so we have to provide a `fmt::formatter` for
these callers which format `append_entry`.
despite that, with FMT_DEPRECATED_OSTREAM, the formatter is defined
by fmt v9, we don't have it since fmt v10. so this change prepares us
for fmt v10.
Refs #13245
Signed-off-by: Kefu Chai <kefu.chai@scylladb.com>
this change prepares for adding fmt::formatter for append_entry.
as we are using its formatter in the inline member functions of
`append_reg_model`. but its `fmt::formatter` can only be specialized out of
this class. and we don't have access to `format_as()` yet in {fmt} 9.1.0
which is shipped along with fedora38, which is in turn used for
our base build image.
so, in this change, `append_reg_model::entry` is extracted and renamed
to `append_entry`.
Signed-off-by: Kefu Chai <kefu.chai@scylladb.com>
Tablets metadata is quite expensive to generate (each data_value is
an allocation), so an old driver (without support for tablets) will
generate huge amounts of such notifications. This commit adds a way
to negotiate generation of the notification: a new driver will ask
for them, and an old driver won't get them. It uses the
OPTIONS/SUPPORTED/STARTUP protocol described in native_protocol_v4.spec.
Closesscylladb/scylladb#16611
seastar dropped the dependency to Crypto++, and it also removed
Findcryptopp.cmake from its `cmake` directory. but scylladb still
depends on this library. and it has been using the `Findcryptopp.cmake`
in seastar submodule for finding it.
after the removal of this file, scylladb would not be able to
use it anymore. so, we have to provide our own `Findcryptopp.cmake`.
Findcryptopp.cmake is copied from the Seastar project. So its
date of copyright is preserved. and it was licensed under Apache 2.0,
since we are creating a derivative work from it. let's relicense
it under Apache 2.0 and AGPL 3.0 or later.
Signed-off-by: Kefu Chai <kefu.chai@scylladb.com>
Closesscylladb/scylladb#16601
seastar::logger is using the compile-time format checking by default if
compiled using {fmt} 8.0 and up. and it requires the format string to be
consteval string, otherwise we have to use `fmt::runtime()` explicitly.
so adapt the change, let's use the consteval string when formatting
logging messages.
Signed-off-by: Kefu Chai <kefu.chai@scylladb.com>
Closesscylladb/scylladb#16612
in `alternator/auth.cc`, none of the symbols in "query" namespace
provided by the removed headers is used is used, so there is no
need to include this header file.
the same applies to other removed header files.
Signed-off-by: Kefu Chai <kefu.chai@scylladb.com>
Closesscylladb/scylladb#16603
In the Raft-based topology, we should never update token metadata
through gossip notifications. `storage_service::on_alive` and
`storage_service::on_remove` do it, so we ignore their parts that
touch token metadata.
Additionally, we improve some logs in other places where we ignore
the function because of using the Raft-based topology.
Fixesscylladb/scylladb#15732Closesscylladb/scylladb#16528
* github.com:scylladb/scylladb:
storage_service: handle_state_left, handle_state_normal: improve logs
raft topology: do not update token metadata in on_alive and on_remove
before this change, we rely on the default-generated fmt::formatter
created from operator<<, but fmt v10 dropped the default-generated
formatter.
in this change, we define a formatter for `atomic_cell_view::printer`
and `atomic_cell::printer` respectively, and remove their operator<<().
Refs #13245
Signed-off-by: Kefu Chai <kefu.chai@scylladb.com>
Closesscylladb/scylladb#16602
In #16102, we added a test for concurrent bootstrap in the raft-based
topology. This test was running in CI for some time and
never failed. Now, we can believe that concurrent bootstrap is not
bugged or at least the probability of a failure is very low. Therefore,
we can safely make use of it in all tests using the raft-based topology.
This PR:
- makes all initial servers start concurrently in topology tests,
- replaces all multiple `server_add` calls with a single `servers_add`
call in tests using the raft-based topology,
- removes no longer needed `test_concurrent_bootstrap`.
The changes listed above:
- make running tests a bit faster due to concurrent bootstraps,
- make multiple tests test concurrent bootstrap previously tested by
a single test.
Fixesscylladb/scylladb#15423Closesscylladb/scylladb#16384
* github.com:scylladb/scylladb:
test: test_different_group0_ids: fix comments
test: remove test_concurrent_bootstrap
test: replace multiple server_add calls with servers_add
test: ScyllaCluster: start all initial servers concurrently
test: ManagerClient: servers_add: specify consistent-topology-changes assumption
Previously, the tablet information was sent to the drivers
in two pieces within the custom_payload. We had information
about the replicas under the `tablet_replicas` key and token range
information under `token_range`. These names were quite generic
and might have caused problems for other custom_payload users.
Additionally, dividing the information into two pieces raised
the question of what to do if one key is present while the other
is missing.
This commit changes the serialization mechanism to pack all information
under one specific name, `tablets-routing-v1`.
From: Sylwia Szunejko <sylwia.szunejko@scylladb.com>
Closesscylladb/scylladb#16148
This test only adds 3 nodes concurrently to the empty cluster.
After making many other tests use ManagerClient.servers_add, it
serves no purpose.
We had added this test before we decided to use
ManagerClient.servers_add in many tests to avoid multiple failures
in CI if it turned out that the concurrent bootstrap is flaky with
high frequency there. This test was running in CI for some time and
never failed. Now, we can believe that concurrent bootstrap is not
bugged or at least the probability of a failure is very low.
ManagerClient.servers_add can be used in every test that uses
consistent topology changes. We replace all multiple server_add
calls in such tests with a single servers_add call to make these
tests faster and simplify their code. Additionally, these
servers_add calls will test concurrent bootstraps for free.
Starting all initial servers concurrently makes tests in suites
with initial_size > 1 run a bit faster. Additionally, these tests
test concurrent bootstraps for free.
add_servers can be called only if the cluster uses consistent
topology changes. We can use this function unconditionally in
install_and_start because every suite uses consistent topology
changes by default. The only way to not use it is by adding all
servers with a config that contains experimental_features without
consistent-topology-changes.
we create a default `scylla.yaml` on the fly in `install.sh`. but
the path to the temporary file holding the default yaml file is
hardwired to `/tmp/scylla.yaml`. this works fine if we only have a
single `install.sh` at a certain time point. but if we have multiple
`install.sh` process running in parallel, these packaging jobs could
step on each other when they create and remove the `scylla.yaml`.
in this change, because the limit of `installconfig`, it always consider
the "dest" parameter as a directory, `mktemp` is used for creating a
parent directory of the temporary file.
Fixes#16591
Signed-off-by: Kefu Chai <kefu.chai@scylladb.com>
Closesscylladb/scylladb#16592
we use "ue" for the short of "update_expressions", before we change
our minds and use a more readable name, let's add "ue" to the
"ignore_word_list" option of the codespell.
also, use the abslolute path in "skip" option. as the absolute paths
are also used by codespell's own github workflow. and we are still
observing codespell github workflow is showing the misspelling errors
in our "test/" directory even we have it listed in "skip". so this
change should silence them as well.
Signed-off-by: Kefu Chai <kefu.chai@scylladb.com>
Closesscylladb/scylladb#16593
The HOST_ID is already written to system.peers since inception pretty much (See https://github.com/scylladb/scylladb/pull/16376#discussion_r1429248185 for details).
However, it is written to the table using an individual CQL query and so it is not set atomically with other columns.
If scylla crashes or even hits an exception before updating the host_id, then system.peers might be left in an inconsistent state, and in particular without no HOST_ID value.
This series makes sure that HOST_ID is written to system.peers and use it to "seal" the record by upserting it in a single CQL BATCH query when adding the state for new nodes.
On the read side, skip rows that have no HOST_ID state in system.peers, assuming they are incomplete, i.e. scylla got an exception or crashed while writing them, so they can't be trusted.
With that change we can assume that endpoint state loaded from system.peers will always have a valid host_id.
Refs https://github.com/scylladb/scylladb/pull/15903Closesscylladb/scylladb#16376
* github.com:scylladb/scylladb:
gms: endpoint_state: change application_state_map to std::unordered_map
system_keyspace: update_peer_info: drop single-column overloads
storage_service: drop do_update_system_peers_table
storage_service: on_change: fixup indentation
endpoint_state subscriptions: batch on_change notification
everywhere: drop before_change subscription
system_keyspace: load_tokens/peers/host_ids: enforce presence of host_id
system_keyspace: drop update_tokens(endpoint, tokens) overload
storage_service: seal peer info with host_id
storage_service: update_peer_info: pass peer_info to sys_ks
gms: endpoint_state: define application_state_map
system_keyspace: update_peer_info: use struct peer_info for all optional values
query_processor: execute_internal: support unset values
types: add data_value_list
system_keyspace: get rid of update_cached_values
storage_service: do not update peer info for this node
State changes are processed as a batch and
there is no reason to maintain them as an ordered map.
Instead, use a std::unordered_map that is more efficient.
Signed-off-by: Benny Halevy <bhalevy@scylladb.com>
Rather than calling on_change for each particular
application_state, pass an endpoint_state::map_type
with all changed states, to be processed as a batch.
In particular, thise allows storage_service::on_change
to update_peer_info once for all changed states.
Signed-off-by: Benny Halevy <bhalevy@scylladb.com>
None of the subscribers is doing anything before_change.
This is done before changing `on_change` in the following patch.
Signed-off-by: Benny Halevy <bhalevy@scylladb.com>
When adding a peer via update_peer_info,
insert all columns in a single query
using system_keyspace::peer_info.
This ensures that `host_id` is inserted along with all
other app states, so we can rely on it
when loading the peer info after restart.
Signed-off-by: Benny Halevy <bhalevy@scylladb.com>
Use the newly added system_keyspace::peer_info
to pass a struct of all optional system.peea members
to system_keyspace::update_peer_info.
Add `get_peer_info_for_update` to construct said struct
from the endpoint state.
Signed-off-by: Benny Halevy <bhalevy@scylladb.com>
Have a central definition for the map held
in the endpoint_state (before changing it to
std::unordered_map).
Signed-off-by: Benny Halevy <bhalevy@scylladb.com>
