* seastar 52d41277a...cb3aaf07e (2):
> tls: reloadable_credentials_base: add_dir_watch: fix root dir detection
> scripts/perftune.py: convert nic option in old perftune.yaml to list for compatibility
last fragment is unconditionally pushed to set of fragments, so if data
size is fragment-aligned, an empty fragment will be needlessly pushed to
the back of the fragment set.
note: i haven't tested if empty fragment at back of set will cause issues,
i think it won't, but this should be avoided anyway.
Signed-off-by: Raphael S. Carvalho <raphaelsc@scylladb.com>
Message-Id: <20210129231532.871405-3-raphaelsc@scylladb.com>
1) reuse default_fragment_size for knowledge of max fragment size
2) fragments_count is not a good name as it doesn't include last non-full
fragment (if present), so rename it.
3) simplify calculation of last fragment size
Signed-off-by: Raphael S. Carvalho <raphaelsc@scylladb.com>
Message-Id: <20210129231532.871405-1-raphaelsc@scylladb.com>
The idea of the monotonicity checking test is: try to apply
one one random partition to another random one sequentually
failing allocations. Each time allocation fails (with the
bad_alloc exception) -- check the exception guarantee is
respected, then apply (!) the very same two partitions to
each other. At the end of the test we make sure, that an
exception may pop up at any point of application and it
will be safe.
This idea is flawed currently. When verifying the guarantee
the test moves the 2nd partition and leaves it empty for the
next loop iteration. So right on the 2nd attempt to apply
partitions it becomes a no-op, doesn't fail and no more
exceptions arise.
Fix by restoring both partitions at the end of each check.
Broken since 74db08165d.
Signed-off-by: Pavel Emelyanov <xemul@scylladb.com>
Message-Id: <20210129153641.5449-1-xemul@scylladb.com>
This series contains an initial implementation of raft persistency module
that uses `raft` system table as the underlying storage model.
"system.raft" table will be used as a backend storage for implementing
raft persistence module in Scylla. It combines both raft log,
persisted vote and term, and snapshot info.
The table is partitioned by group id, thus allowing multi-raft
operation. The rest of the table structure mirrors the fields of
corresponding core raft structures defined in `raft.hh`, such as
`raft::log_entry`.
The raft table stores the only the latest snapshot id while
the actual snapshot will be available in a separate table
called `system.raft_snapshots`. The schema of `raft_snapshots`
mirrors the fields of `raft::snapshot` structure.
IDL definitions are also added for every raft struct so that we
automatically provide serialization and deserialization facilities
needed both for persistency module and for future RPC implmementation.
The first patch is a side-change needed to provide complete
serialization/deserialization for `bytes_ostream`, which we
need when persisting the raft log in the table (since `data`
is a variant containing `raft::command` (aka `bytes_ostream`)
among others).
`bytes_ostream` was lacking `deserialize` function, which is
added in the patch.
The second patch provides serializer for `lw_shared_ptr<T>`
which will be used for `raft::append_entries`, which has
a field with `std::vector<const lw_shared_ptr<raft::log_entry>>`
type.
There is also a patch to extend `fragmented_temporary_buffer`
with a static function `allocate_to_fit` that allocates an
instance of the fragmented buffer that has a specified size.
Individual fragment size is limited to 128kb.
The patch-set also contains the test suite covering basic
functionality of the persistency module.
* manmanson/raft-api-impl-v11:
raft/sys_table_storage: add basic tests for raft_sys_table_storage
raft: introduce `raft_sys_table_storage` class
utils: add `fragmented_temporary_buffer::allocate_to_fit`
raft: add IDL definitions for raft types
raft: create `system.raft` and `system.raft_snapshots` tables
serializer: add `serializer<lw_shared_ptr<T>>` specialization
serializer: add `deserialize` function overload for `bytes_ostream`
The test suite covers the most basic use cases for the system table
backed raft persistency module:
* store/load vote and term
* store/load snapshot
* store snapshot with log tail truncation
* store/load log entries
* log truncation
Tests: unit(dev)
Signed-off-by: Pavel Solodovnikov <pa.solodovnikov@scylladb.com>
This is the implementation of raft persistency module that
uses `raft` system table as the underlying storage model.
The instance is supposed to be bound to a single raft group.
Signed-off-by: Pavel Solodovnikov <pa.solodovnikov@scylladb.com>
Introduce `fragmented_temporary_buffer::allocate_to_fit` static
function returning an instance of the buffer of a specified size.
The allocated buffer fragments have a size of at most 128kb.
`bytes_ostream` has the same hard-coded limit, so just use the
same here.
This patch will be later needed for `raft::log_entry` raw data
serialization when writing to the underlying persistent storage.
Signed-off-by: Pavel Solodovnikov <pa.solodovnikov@scylladb.com>
Changes to the `configuration` and `tagged_uint64` classes are needed
to overcome limitations of the IDL compiler tool, i.e. we need to
supply a constructor to the struct initializing all the
members (raft::configuration) and also need to make an accessor
function for private members (in case of raft::tagged_uint64).
All other structs mirror raft definitions in exactly the same way
they are declared in `raft.hh`.
`tagged_id` and `tagged_uint64` are used directly instead of their
typedef-ed companions defined in `raft.hh` since we don't want
to introduce indirect dependencies. In such case it can be guaranteed
that no accidental changes made outside of the idl file will affect idl
definitions.
This patch also fixes a minor typo in `snapshot_id_tag` struct used
in `snapshot_id` typedef.
System raft table will be used as a backend storage for implementing
raft persistence module in Scylla. It combines both raft log,
persisted vote and term, and snapshot info.
The table is partitioned by group id, thus allowing multi-raft
operation. The rest of the table structure mirrors the fields of
corresponding core raft structures defined in `raft.hh`, such as
`raft::log_entry`.
The raft table stores the only the latest snapshot id while
the actual snapshot will be available in a separate table
called `system.raft_snapshots`. The schema of `raft_snapshots`
mirrors the fields of `raft::snapshot` structure.
Signed-off-by: Pavel Solodovnikov <pa.solodovnikov@scylladb.com>
This one works similar to `serializer<optional<T>>` and will be
later needed for serializing `raft::append_request`, which has
a field containing `lw_shared_ptr`.
Users to be warned, though: this code assumes that the pointer
is never null. This is done to mirror the serialize implementation
for `lw_shared_ptr:s` in the messaging_service.cc, which is
subject to being deleted in favor of the impl in the
`serializer_impl.hh`.
Signed-off-by: Pavel Solodovnikov <pa.solodovnikov@scylladb.com>
We use a custom sharder for all schema tables: every table under
the `system_schema` keyspace, plus `system.scylla_table_schema_history`.
This sharder puts all data on shard 0.
To achieve this, we hardcode the sharder in initial schema object
definitions. Furthermore - since the sharder is not stored inside schema
mutations yet - whenever we deserialize schema objects from mutations,
we modify the sharder based on the schema's keyspace and table names.
A regression test is added to ensure no one forgets to set the special
sharder for newly added schema tables. This test assumes that all newly
added schema tables will end up in the `system_schema` keyspace (other
tables may go unnoticed, unfortunately).
Closes#7947
"
Currently there are three different methods for creating an sstable
reader:
* one for single key reads
* one for ranged reads
* and one nobody uses
This patch-set consolidates all these into a single `make_reader()`
method, which behind the scenes uses the same logic to dispatch to the
right sstable reader constructor that `sstables::as_mutation_source()`
uses.
This patch-set is part of an effort to clean up the jungle that is the
various reader creation methods. The next step is to clean up the
sstable_set, which has even more methods.
One very sad discovery I made while working on this patch-set is that
we
still default `mutation_reader::forwarding` to `yes` in the sstable
range reader creator method and in the
`mutation_source::make_reader()`.
I couldn't assume that all callers are passing what they mean as the
value for that parameter. I found many sites in tests that create
forwardable single partition readers. This is also something we should
address soon.
Tests: unit(release, debug:v3)
"
* 'sstables-consolidate-reader-factory-methods-v4' of https://github.com/denesb/scylla:
cql_query_test: add unit test covering the non-optimal TWCS sstable read path
sstable_mutation_reader: consolidate constructors
tests: don't pass temporary ranges to readers
sstables: sstable_mutation_reader: remove now unused whole sstable constructor
sstables: stats: remove now unused sstable_partition_reads counter
sstable: remove read_.*row.*_flat() methods
tree-wide: use sstables::make_reader() instead of the read_.*row.*_flat() methods
sstables: pass partition_range to create_single_key_sstable_reader()
sstables: sstable: add make_reader()
The sstable read path for TWCS tables takes a different path when the
optimized read path cannot be used. This path was found to be not
covered at all by unit tests which allowed a trivial use-after-free to
slip in. Add a unit test to cover this path as well, so ASAN can catch
such bugs in the future.
* seastar a287bb1a3...52d41277a (8):
> fair_queue: Preempted requests got re-queued too far
> scripts/perftune.py: remove repeated items after merging options from file
> file.hh: Remove fair_queue.hh
> Merge "Reloadable TLS certificate tolerance" from Calle
> Merge "Cancellable IO" from Pavel E
> abort-source: Improve the subscriptions management
> fair_queue: Improve requests preemption while in pending state
> http: add support for Default handler (/*)
The two remaining sstable constructor are very similar apart from the
content of the initialize lambda. Speaking of which, the two remaining
initializer lambdas can be easily merged into one too. So this patch
does just that, consolidates the two constructors one and moves
consolidates as well as extracts the initializer method into a member
method. This means we have to store the previously captured variables as
members, but this is actually a good thing: when debugging we can see
the range and slice the reader is reading, and we are not actually
paying for it either -- they were already stored, just out of sight.
The sstable_mutation_reader, like all other mutation readers expects
that the partition-range passed to it is kept alive by its creator
for the duration of its lifetime. However, the single-key constructor
of the sstable reader was more tolerant, as it only extracted the key
from the range, essentially requiring only the key to be kept alive (but
not the containing range). Naturally in time some code come to rely on
it and ended up passing temporary ranges to the reader. This behaviour
will no longer be acceptable as we are about to consolidate the various
sstable reader constructors, uniformly requiring that the range is kept
alive. So this patch fixes up the tests so they work with this stricter
requirement. Only two occurences were found.
We want to unify the various sstable reader creation methods and this
method taking a ring position instead of a partition range like
everybody else stands in the way of that.
This is effect reverts 68663d0de.
Follow-up to #7917
The size of an cf::column_family_info is 224 bytes, so an std::vector that
contains one for each column family may be very large, causing allocations
of over 1MB.
Considering the vector is used only for iteration, it can be changed to
a non-contiguous list instead.
Signed-off-by: Wojciech Mitros <wojciech.mitros@scylladb.com>
Closes#7973
"
Currently we have two parallel query paths:
* database::query() -> table::query() -> data_query()
* mutation::query()
The former is used by single partition queries, the latter by range
scans, as mutation::query() is used to convert reconcilable_result to
query::result (which means it is also used in single partition queries
if it triggers read repair). This is a rather unfortunate situation as
we have two parallel implementation of the query code, which means they
are prone to diverge, and in fact they already have -- more on that
later.
This patchset aims to remedy this situation by retiring
`mutation::query()` and migrating users to an implementation based on
the "standard" query path, in other words one using the same building
blocks as the `database::query()` path. This means using
`compact_mutation` for compacting and `query_result_builder` for result
building. These components however were created to work with
`flat_mutation_reader`, however introducing a reader into this pipeline
would mean that we'd have to make all the related APIs asynchronous,
which would cause an insane amount of churn. To avoid this, this
patchset adds an API compatible `consume()` method to `mutation`, which
can accept a `compact_mutation` instance as-is. This allows an elegant
and succinct reimplementation. So far so good.
Like mentioned above, the two implementations have diverged in time, or
have been different from the start. The difference manifest when
calculating digests, more precisely in which tombstones are included in
the digest. The retired `mutation::query()` path incorporates only
non-purgeable tombstones in the digest. The standard query path however
incorporates all tombstones, even those that can be purged. After some
scrutiny however this difference proved to be completely theoretical,
as
the code path where this would matter -- converting reconcilable result
to query result -- passes min timestamp as the query time to the
compaction, so nothing is compacted and hence the difference has no
chance to manifest.
This patch-set was motivated by the desire to provide a single solution
to #7434, instead of two, one for each path.
Tests: unit(release:v2, debug:v2, dev:v3)
"
* 'unified-query-path/v3' of https://github.com/denesb/scylla:
mutation: remove now unused query() and query_compacted()
treewide: use query_mutations() instead of mutation::query()
mutation_test: test_query_digest: ensure digest is produced consistently
mutation_query: introduce query_mutation()
mutation_query: to_data_query_result(): migrate to standard query code
mutation_query: move to_data_query_result() to mutation_partition.cc
mutation: add consume()
flat_mutation_reader: move mutation consumer concepts to separate header
mutation compactor: query compaction: ignore purgeable tombstones
This will be the only method to create sstable readers with. For now we
leave the other variants, they as well as their users will be removed in
a following patch.
This patch adds a cql-pytest, test_json.py::test_tojson_double(),
which reproduces issue #7972 - where toJson() prints some doubles
incorrectly - truncated to integers, but some it prints fine (I
still don't know why, this will need to be debugged).
The test is marked xfail: It fails on Scylla, and passes on Cassandra.
Refs #7972.
Signed-off-by: Nadav Har'El <nyh@scylladb.com>
Message-Id: <20210127124338.297544-1-nyh@scylladb.com>
This patch introduces `schema_raft_state_machine` class
which is currently just a dummy implementation throwing a
"not implemented" exceptions for every call.
Will be needed later to construct an instance of `raft::server`.
Signed-off-by: Pavel Solodovnikov <pa.solodovnikov@scylladb.com>
Message-Id: <20210126193413.1520948-1-pa.solodovnikov@scylladb.com>
For some reason we had a distinct specialization of `serialize`
function to handle `bytes_ostream` but not `deserialize`.
This will be used in the following patches.
Signed-off-by: Pavel Solodovnikov <pa.solodovnikov@scylladb.com>
Currently the replacing node sets the status as STATUS_UNKNOWN when it
starts gossip service for the first time before it sets the status to
HIBERNATE to start the replacing operation. This introduces the
following race:
1) Replacing node using the same IP address of the node to be replaced
starts gossip service without setting the gossip STATUS (will be seen as
STATUS_UNKNOWN by other nodes)
2) Replacing node waits for gossip to settle and learns status and
tokens of existing nodes
3) Replacing node announces the HIBERNATE STATUS.
After Step 1 and before Step 3, existing nodes will mark the replacing
node as UP, but haven't marked the replacing node as doing replacing
yet. As a result, the replacing node will not be excluded from the read
replicas and will be considered a target node to serve CQL reads.
To fix, we make the replacing node avoid responding echo message when it is not
ready.
Fixes#7312Closes#7714
I see a miscompile on aarch64 where a call to format("{}", uuid)
translates a function pointer to -1. When called, this crashes.
Reduce the inline threshold from 2500 to 600. This doesn't guarantee
no miscompiles but all the tests pass with this parameter.
Closes#7953
The following patches fix issues seen occasionally in debug mode.
Notes:
- In debug mode there's still the UB nullptr arithmetic warning.
* https://github.com/alecco/scylla/tree/raft-ale-tests-07h-wait-propagation:
raft: replication test: wait for log propagation
raft: replication test: move wait for log to a function
raft: replication test: remove unused member
raft: replication test: use later()
raft: testing: remove election wait time and just yield
test_cell_external_memory_usage uses with_allocator() to observe how some
types allocate memory. However, compiler reordering (observed with clang 11
on aarch64) can move the various thread-local CQL type object initialization
into the with_allocator() scope; so any managed object allocated as part of
this initialization also gets measured, and the test fails. The code movement
is legal, as far as I can tell.
Fix this by initializing the type object early; use an atomic_thread_fence
as an optimization barrier so the compiler doesn't eliminate the or move
the early initialization.
Closes#7951
This patch adds a test for trying to set a tuple element to null with
fromJson(), which works on Cassandra but fails on Scylla. So the test
xfails on Scylla. Reproduces issue #7954.
Refs #7954.
Signed-off-by: Nadav Har'El <nyh@scylladb.com>
Message-Id: <20210124082311.126300-1-nyh@scylladb.com>
"
`next_partition()` used to return void, so readers that had to call
future returning code had to work around this. Now that
`next_partition()` returns a future, we can get rid of these
workarounds.
Tests: unit(release, debug)
"
* 'next-partition-cross-shard-readers/v1' of https://github.com/denesb/scylla:
mutation_reader: reader_lifecycle_policy::stopped_reader: drop pending_next_partition flag
mutation_reader: evictable_reader: remove next_partition() workaround
mutation_reader: shard_reader: remove next_partition() workaround
mutation_reader: foreign_reader: remove next_partition() workaround
If the message is larger than current buffer size, we need to consume
more data until we reach to tail of the message.
To do so, we need to return nullptr when it's not on the tail.
Fixes#7273Closes#7903
* github.com:scylladb/scylla:
redis: rename _args_size/_size_left There are two types of numerical parameter in redis protocol: - *[0-9]+ defined array size - $[0-9]+ defined string size
redis: fix large message handling
There are two types of numerical parameter in redis protocol:
- *[0-9]+ defined array size
- $[0-9]+ defined string size
Currently, array size is stored to args_count, and string size is
stored to _arg_size / _size_left.
It's bit hard to understand since both uses same word "arg(s)", let's
rename string size variables to _bytes_count / _bytes_left.
If the message is larger than current buffer size, we need to consume
more data until we reach to tail of the message.
To do so, we need to return nullptr when it's not on the tail.
Fixes#7273
Wait until entries propagate after adding and before changing leader
using the same code as done for partitioning.
This fixes occasional hangs in debug mode when a test switches to a
different leader without leaving enough time for full propagation.
Signed-off-by: Alejo Sanchez <alejo.sanchez@scylladb.com>
Instead of sleep 1us use later()
Also use later to yield after sending append entries in rpc test impl.
Signed-off-by: Alejo Sanchez <alejo.sanchez@scylladb.com>
After support for mixed cluster compatibility feature
DIGEST_MULTIPARTITION_READ was dropped in 854a44ff9b
range_slice_read_executor and never_speculating_read_executor become
identical, so remove the former for good.
Message-Id: <20210124122731.GA1122499@scylladb.com>
