Now all the logged arguments are lazily evaluated (node* format string
and backtrace) so the preliminary log-level checks are not needed.
indentation is deliberately left broken
Signed-off-by: Pavel Emelyanov <xemul@scylladb.com>
This helper returns lazy_eval-ed current_backtrace(), so it will be
generated and printed only if logger is really going to do it with its
current log-level.
Signed-off-by: Pavel Emelyanov <xemul@scylladb.com>
Currently to print node information there's a debug_format(node*) helper
function that returns back an sstring object. Here's the formatter
that's more flexible and convenient, and a node_printer wrapper, since
formatters cannot format non-void pointers.
Signed-off-by: Pavel Emelyanov <xemul@scylladb.com>
Equip it with :v specifier that turns verbose mode on and prints much
more data about the node. Main user will appear in the next patch.
Signed-off-by: Pavel Emelyanov <xemul@scylladb.com>
Currently it waits for topology state machine to be idle, so it allows
one tablet to be moved at a time. We should allow it to start migration
if the current transition state is
- topology::transition_state::tablet_migration or
- topology::transition_state::tablet_draining
to allow starting parallel tablet movement. That will be useful when
scripting a custom rebalancing algorithm.
in this change, we wait until the topology state machine is idle or
it is at either of the above two states.
Fixes#16437
Signed-off-by: Kefu Chai <kefu.chai@scylladb.com>
Closesscylladb/scylladb#17203
codespell reports following warnings:
```
Error: ./docs/kb/flamegraph.svg:1: writen ==> written
Error: ./docs/kb/flamegraph.svg:1: writen ==> written
Error: ./docs/kb/flamegraph.svg:1: storag ==> storage
Error: ./docs/kb/flamegraph.svg:1: storag ==> storage
```
these misspellings come from the flamgraph, which can be viewed
at https://opensource.docs.scylladb.com/master/kb/flamegraph.html
they are very likely to be truncated function names displayed
in the frames. and the spelling of these names are not responsible
of the author of the article, neither can we change them in a
meaningful way. so add it to the skip list.
Signed-off-by: Kefu Chai <kefu.chai@scylladb.com>
Closesscylladb/scylladb#17215
we have three format()s in our arsenal:
* seastar::format()
* fmt::format()
* std::format()
the first one is used most frequently. but it has two limitations:
1. it returns seastar::sstring instead of std::string. under some
circumstances, the caller of the format() function actually
expects std::string, in that case a deep copy is performed to
construct an instance of std::string. this incurs unnecessary
performance overhead. but this limitation is a by-design behavior.
2. it does not do compile-time format check. this can be improved
at the Seastar's end.
to address these two problems, we switch the callers who expect
std::string to fmt::format(). to minimize the impact and to reduce
the risk, the switch will be performed piecemeal.
Signed-off-by: Kefu Chai <kefu.chai@scylladb.com>
Closesscylladb/scylladb#17212
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 formatters for `error_injection_at_startup`,
and drop its operator<<.
Refs #13245
Signed-off-by: Kefu Chai <kefu.chai@scylladb.com>
Closesscylladb/scylladb#17211
On top of the capabilities of the java-nodetool command, tablet support is also implemented: in addition to the existing keyspace parameter, an optional table parameter is also accepted and forwarded to the REST API. For tablet keyspaces this is required to get a ring description.
The command comes with tests and all tests pass with both the new and the current nodetool implementations.
Refs: https://github.com/scylladb/scylladb/issues/15588
Refs: https://github.com/scylladb/scylladb/issues/16846Closesscylladb/scylladb#17163
* github.com:scylladb/scylladb:
tools/scylla-nodetool: implement describering
tools/scylla-nodetool.cc: handle API request failures gracefully
test/nodetool: util.py: add check_nodetool_fails_with_all()
It holds back global token metadata barrier during streaming, which
limits parallelism of load balancing.
Topology transition is protected by the means of topology_guard.
Closesscylladb/scylladb#17230
Also implementing tablet support, which basically just means that a new
table parameter is also accepted and forwarded to the API, in addition
to the existing keyspace one.
Currently, error handling is done via catching
http::unexpected_status_error and re-throwing an std::runtime_error.
Turns out this no longer works, because this error will only be thrown
by the http client, if the request had an expected reply code set.
The scylla_rest_client doesn't set an expected reply code, so this
exception was never thrown for some time now.
Furthermore, even when the above worked, it was not too user-friendly as
the error message only included the reply-code, but not the reply
itself.
So in this patch this is fixed:
* The handling of http::unexpected_status_error is removed, we don't
want to use this mechanism, because it yields very terse error
messages.
* Instead, the status code of the request is checked explicitely and all
cases where it is not 200 are handled.
* A new api_request_failed exception is added, which is throw for all
non-200 statuses with the extracted error message from the server (if
any).
* This exception is caught by main, the error message is printed and
scylla-nodetool returns with a new distinct error-code: 4.
With this, all cases where the request fails on ScyllaDB are handled and
we shouldn't hit cases where a nodetool command fails with some
obscure JSON parsing error, because the error reply has different JSON
schema than the expected happy-path reply.
Similar to the existing check_nodetool_fails_with() but checks that all
error messages from expected_errors are contained in stderr.
While at it, use list as the typing hint, instead of typing.List.
both of its callers are passing parent_path() and filename() to
it. so let the callee to do this. simpler this way.
Signed-off-by: Kefu Chai <kefu.chai@scylladb.com>
Closesscylladb/scylladb#17225
before this change, when performing `stream_transfer_task`, if an
exception is raised, we check if the table being streamed is still
around, if it is missing, we just skip the table as it should be
dropped during streaming, otherwise we consider it a failure, and
report it back to the peer. this behavior was introduced by
953af382.
but we perform the streaming on all shards in parallel, and if any
of the shards fail because of the dropped table, the exception is
thrown. and the current shard is not necessarily the one which
throws the exception. actually, current shard might be still
waiting for a write lock for removing the table from the database's
table metadata. in that case, we consider the streaming RPC call a
failure even if the table is already removed on some shard(s). and
the peer would fail to bootstreap because of streaming failure.
in this change, before catching all exceptions, we handle
`no_such_column_family`, and do not fail the streaming in that case.
please note, we don't touch other tables, so we can just assume that
`no_such_column_family` is thrown only if the table to be transferred
is missing. that's why `assert()` is added.
Fixes#15370
Signed-off-by: Kefu Chai <kefu.chai@scylladb.com>
Closesscylladb/scylladb#17160
Due to the potentially large number of per-table metrics, ScyllaDB uses
configuration to determine what metrics will be reported. The user can
decide if they want per-table-per-shard metrics, per-table-per-instance
metrics, or none.
This patch uses the same logic for tablet metrics registration.
It adds a new metrics group tablets with one metric inside it - count.
So, scylla_tablets_count will report the number of tablets per shard.
The existing per-table metrics will be reported aggregated or not like
the other per-table metrics.
Signed-off-by: Amnon Heiman <amnon@scylladb.com>
Closesscylladb/scylladb#17182
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 formatters for `gms::gossip_digest_ack2`,
and drop its operator<<.
Refs #13245
Signed-off-by: Kefu Chai <kefu.chai@scylladb.com>
Closesscylladb/scylladb#17153
this change is a follow-up of 637dd730. the goal is to use
std::filesystem::path for manipulating paths, and to avoid the
converting between sstring and fs::path back and forth.
Signed-off-by: Kefu Chai <kefu.chai@scylladb.com>
Closesscylladb/scylladb#17214
state_to_dir(sstable_state) translate the enum to the corresponding
directory component. and it returns a `seastar::sstring`. not all
the callers of this function expect a full-blown sstring instance,
on the contrary, quite a few of them just want a string-alike object
which represents the directory component, so they can use it, for
instance to compose a path, or just format the given `state` enum.
so to avoid the overhead of creating/destroying the `seastar::sstring`
instance, let's switch to `std::string_view`. with this change, we
will be able to implement the fmt::formatter for `sstable_state`
without the help of the formatter of sstring.
Signed-off-by: Kefu Chai <kefu.chai@scylladb.com>
Closesscylladb/scylladb#17213
The table query param is added to get the describe_ring result for a
given table.
Both vnode table and tablet table can use this table param, so it is
easier for users to user.
If the table param is not provided by user and the keyspace contains
tablet table, the request will be rejected.
E.g.,
curl "http://127.0.0.1:10000/storage_service/describe_ring/system_auth?table=roles"
curl "http://127.0.0.1:10000/storage_service/describe_ring/ks1?table=standard1"
Refs #16509Closesscylladb/scylladb#17118
* github.com:scylladb/scylladb:
tablets: Convert to use the new version of for_each_tablet
storage_service: Add describe_ring support for tablet table
storage_service: Mark host2ip as const
tablets: Add for_each_tablet_gently
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 formatters for `dht::ring_posittion`,
and drop its operator<<.
Refs #13245
Signed-off-by: Kefu Chai <kefu.chai@scylladb.com>
Closesscylladb/scylladb#17194
It uses only compile-time constants to produce the value, so deserves
this marking
Signed-off-by: Pavel Emelyanov <xemul@scylladb.com>
Closesscylladb/scylladb#17181
Reduces footprint from hundreds of MB to a very few MB.
Issue could be reproduced with:
./build/dev/test/boost/mutation_writer_test --run_test=test_token_group_based_splitting_mutation_writer -- -m 500M --smp 1 --random-seed 1848215131
Fixes#17076.
Signed-off-by: Raphael S. Carvalho <raphaelsc@scylladb.com>
Closesscylladb/scylladb#17187
When creating a keyspace, scylla allows setting RF value smaller than there are nodes in the DC. With vnodes, when new nodes are bootstrapped, new tokens are inserted thus catching up with RF. With tablets, it's not the case as replica set remains unchanged.
With tablets it's good chance not to mimic the vnodes behavior and require as many nodes to be up and running as the requested RF is. This patch implementes this in a lazy manned -- when creating a keyspace RF can be any, but when a new table is created the topology should meet RF requirements. If not met, user can bootstrap new nodes or ALTER KEYSPACE.
closes: #16529Closesscylladb/scylladb#17079
* github.com:scylladb/scylladb:
tablets: Make sure topology has enough endpoints for RF
cql-pytest: Disable tablets when RF > nodes-in-DC
test: Remove test that configures RF larger than the number of nodes
keyspace_metadata: Include tablets property in DESCRIBE
despite that "welp" is more emotional expressive, it is considered
a misspelling of "whelp" by codespell. that's why this comment stands
out. but from a non-native speaker's point of view, probably we can
use more descriptive words to explain what "welp" is for in plain
English.
Signed-off-by: Kefu Chai <kefu.chai@scylladb.com>
Closesscylladb/scylladb#17183
In this PR we add the tests for two scenarios, related to the use of IPs in raft topology.
* When the replaced node transitions to the `LEFT` state we used to
remove the IP of such node from gossiper. If we replace with same IP,
this caused the IP of the new node to be removed from gossiper. This
problem was fixed by #16820, this PR adds a regression test for it.
* When a node is restarted after decommissioning some other node, the
restarting node tries to apply the raft log, this log contains a
record about the decommissioned node, and we got stuck trying to resolve
its IP. This was fixed by #16639 - we excluded IPs from the RAFt log
application code and moved it entirely to host_id-s. This PR adds a
regression test for this case.
Closesscylladb/scylladb#15967Closesscylladb/scylladb#14803Closesscylladb/scylladb#17180
* github.com:scylladb/scylladb:
test_topology_ops: check node restart after decommission
test_replace_reuse_ip: check other servers see the IP
For efficiency, if a base-table update generates many view updates that
go the same partition, they are collected as one mutation. If this
mutation grows too big it can lead to memory exhaustion, so since
commit 7d214800d0 we split the output
mutation to mutations no longer than 100 rows (max_rows_for_view_updates)
each.
This patch fixes a bug where this split was done incorrectly when
the update involved range tombstones, a bug which was discovered by
a user in a real use case (#17117).
Range tombstones are read in two parts, a beginning and an end, and the
code could split the processing between these two parts and the result
that some of the range tombstones in update could be missed - and the
view could miss some deletions that happened in the base table.
This patch fixes the code in two places to avoid breaking up the
processing between range tombstones:
1. The counter "_op_count" that decides where to break the output mutation
should only be incremented when adding rows to this output mutation.
The existing code strangely incrmented it on every read (!?) which
resulted in the counter being incremented on every *input* fragment,
and in particular could reach the limit 100 between two range
tombstone pieces.
2. Moreover, the length of output was checked in the wrong place...
The existing code could get to 100 rows, not check at that point,
read the next input - half a range tombstone - and only *then*
check that we reached 100 rows and stop. The fix is to calculate
the number of rows in the right place - exactly when it's needed,
not before the step.
The first change needs more justification: The old code, that incremented
_op_count on every input fragment and not just output fragments did not
fit the stated goal of its introduction - to avoid large allocations.
In one test it resulted in breaking up the output mutation to chunks of
25 rows instead of the intended 100 rows. But, maybe there was another
goal, to stop the iteration after 100 *input* rows and avoid the possibility
of stalls if there are no output rows? It turns out the answer is no -
we don't need this _op_count increment to avoid stalls: The function
build_some() uses `co_await on_results()` to run one step of processing
one input fragment - and `co_await` always checks for preemption.
I verfied that indeed no stalls happen by using the existing test
test_long_skipped_view_update_delete_with_timestamp. It generates a
very long base update where all the view updates go to the same partition,
but all but the last few updates don't generate any view updates.
I confirmed that the fixed code loops over all these input rows without
increasing _op_count and without generating any view update yet, but it
does NOT stall.
This patch also includes two tests reproducing this bug and confirming
its fixed, and also two additional tests for breaking up long deletions
that I wanted to make sure doesn't fail after this patch (it doesn't).
By the way, this fix would have also fixed issue #12297 - which we
fixed a year ago in a different way. That issue happend when the code
went through 100 input rows without generating *any* output rows,
and incorrectly concluding that there's no view update to send.
With this fix, the code no longer stops generating the view
update just because it saw 100 input rows - it would have waited
until it generated 100 output rows in the view update (or the
input is really done).
Fixes#17117
Signed-off-by: Nadav Har'El <nyh@scylladb.com>
Closesscylladb/scylladb#17164
The new rpc::optional parameter must come after any existing parameters,
including the rpc::source parameters, otherwise it will break
compatibility.
The regression was introduced in:
```
commit fd3c089ccc
Author: Tomasz Grabiec <tgrabiec@scylladb.com>
Date: Thu Oct 26 00:35:19 2023 +0200
service: range_streamer: Propagate topology_guard to receivers
```
We need to backport this patch ASAP before we release anything that
contains commit fd3c089ccc.
Refs: #16941Fixes: #17175Closesscylladb/scylladb#17176
RF values appear as strings and strategies classes convert them to integers. This PR removes some duplication of efforts in converting code.
Closesscylladb/scylladb#17132
* github.com:scylladb/scylladb:
network_topology_strategy: Do not walk list of datacenters twice
replication_strategy: Do not convert string RF into int twise
abstract_replication_strategy: Make validate_replication_factor return value
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 formatters for `data_dictionary::user_types_metadata`,
and drop its operator<<.
Refs #13245
Signed-off-by: Kefu Chai <kefu.chai@scylladb.com>
Closesscylladb/scylladb#17140
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 formatters for `exceptions::exception_code`,
and drop its operator<<.
Refs #13245
Signed-off-by: Kefu Chai <kefu.chai@scylladb.com>
Closesscylladb/scylladb#17151
C++20 introduced a new overload of std::ostringstream::str() that is selected when the mentioned member function is called on r-value.
The new overload returns a string, that is move-constructed from the underlying string instead of being copy-constructed.
This change applies std::move() on stringstream objects before calling str() member function to avoid copying of the underlying buffer.
It also removes a helper function `inet_addr_type_impl::to_sstring()` - it was used only in two places. It was replaced with `fmt::to_string()`.
Closesscylladb/scylladb#16991
* github.com:scylladb/scylladb:
use fmt::to_string() for seastar::net::inet_address
types/types.cc: move stringstream content instead of copying it
so we can tighten our dependencies a little bit. there are only three places where we are using the `date` library. also, there is no need to reinvent the wheels if there are ready-to-use ones.
Closesscylladb/scylladb#17177
* github.com:scylladb/scylladb:
types: use {fmt} to format boolean
types: use {fmt} to format time
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 formatters for `dht::sharder`, and drop
its operator<<.
Refs #13245
Signed-off-by: Kefu Chai <kefu.chai@scylladb.com>
Closesscylladb/scylladb#17178
* seastar 85359b28...289ad5e5 (19):
> net/dpdk: use user-defined literal when appropriate
> io_tester: Allow running on non-XFS fs
> io: Apply rate-factor early
> circular_buffer: make iterator default constructible
> net/posix: add a way to change file permissions of unix domain socket
> resource: move includes to the top of the source file
> treewide: replace calls to future::get0() by calls to future::get()
> core/future: add as_ready_future utility
> build: do not expose -Wno-error=#warnings
> coroutine: remove remnants of variadic futures
> build: prevent gcc -Wstringop-overflow workaround from affecting clang
> util/spinlock: use #warning instead of #warn
> io_tester: encapsulate code into allocate_and_fill_buffer()
> io_tester: make maybe_remove_file a function
> future: remove tuples from get0_return_type
> circular_buffer_fixed_capacity: use std::uninitialized_move() instead of open-coding
> rpc/rpc_types: do not use integer literal in preprocessor macro
> future: use "(T(...))" instead of "{T(...)}" in uninitialized_set()
> net/posix: include used header
Closesscylladb/scylladb#17179
Adds a missing logging import in the file scylladb_common_images extension, which prevents the enterprise build from building.
Additionally, it standardizes logging handling across the extensions and removes "ami" references in Azure and GCP extensions.
Closesscylladb/scylladb#17137
also disable some more warnings which are failing the build after `-Wextra` is enabled. we can fix them on a case-by-case basis, if they are geniune issues. but before that, we just disable them.
this goal of this change is to reduce the discrepancies between the compile options used by CMake and those used by configure.py. the side effect is that we enable some more warning enabeld by `-Wextra`, for instance, `-Wsign-compare` is enable now. for the full list of the enabled warnings when building with Clang, please see https://clang.llvm.org/docs/DiagnosticsReference.html#wextra.
Closesscylladb/scylladb#17131
* github.com:scylladb/scylladb:
configure.py: add -Wextra to cflags
test/tablets: do not compare signed and unsigned
There used to be a problem with restarting a node after
decommissioning some other node - the restarting node
tries to apply the raft log, this log contains a record
about the decommissioned node, and we got stuck trying
to resolve its IP.
This was fixed in #16639 - we excluded IPs from
the RAFt log application code and moved it entirely
to host_id-s.
In this commit we add a regression test
for this case. We move the decommission_node
call before server_stop/server_start. We need
to add one more server to retain majority when
the node is decommissioned, otherwise the topology
coordinator won't migrate from the stopped node
before replacing it, and we'll get an error.
closes#14803
The replaced node transitions to LEFT state, and
we used to remove the IPs of such nodes from gossiper.
If we replace with same IP, this caused the IP of the
new node to be removed from gossiper.
This problem was fixed by #16820, this commit
adds a regression test for it.
closes#15967
This PR removes the following pages:
- ScyllaDB Open Source Features
- ScyllaDB Enterprise Features
They were outdated, incomplete, and misleading. They were also redundant, as the per-release updates are added as Release Notes.
With this update, the features listed on the removed pages are added under the common page: ScyllaDB Features.
In addition, a reference to the Enterprise-only Features section is added.
Note: No redirections are added because no file paths or URLs are changed with this PR.
Fixes https://github.com/scylladb/scylladb/issues/13485
Refs https://github.com/scylladb/scylladb/issues/16496
(nobackport)
Closesscylladb/scylladb#17150
* github.com:scylladb/scylladb:
Update docs/using-scylla/features.rst
doc: remove the OSS and Enterprise Features pages
This PR:
- Adds the upgrade guide from ScyllaDB Open Source 5.4 to ScyllaDB Enterprise 2024.1. Note: The need to include the "Restore system tables" step in rollback has been confirmed; see https://github.com/scylladb/scylladb/issues/11907#issuecomment-1842657959.
- Removes the 5.1-to-2022.2 upgrade guide (unsupported versions).
Fixes https://github.com/scylladb/scylladb/issues/16445Closesscylladb/scylladb#16887
* github.com:scylladb/scylladb:
doc: fix the OSS version number
doc: metric updates between 2024.1. and 5.4
doc: remove the 5.1-to-2022.2 upgrade guide
doc: add the 5.4-to-2024.1 upgrade guide
