When converting a value to its Lua representation, we choose
an integer type if it fits. If it doesn't, we fall back to a
more expensive type. So we explicitly try to trigger an overflow.
However, clang's ubsan doesn't like the overflow, and kills the
test. Tell it that the overflow is expected here.
Closes#7374
Our AVX2 implementation cannot load a partial vector,
or mask unused elements (that can be done with AVX-512/SVE2),
so it has some restrictions. Document them.
Closes#7385
Offsetting a null pointer is undefined, and clang's ubsan complains.
Rearrange the arithmetic so we never offset a null pointer. A function
is introduced for the remaining contiguous bytes so it can cast the result
to size_t, avoiding a compare-of-different-signedness warning from gcc.
Closes#7373
When the number of nanosecond digits is greater than 9, the std::pow()
expression that corrects the nanosecond value becomes infinite. This
is because sstring::length() is unsigned, and so negative values
underflow and become large.
Following Cassandra, fix by forbidding more than 9 digits of
nanosecond precision.
Found by clang's ubsan.
Closes#7371
Following 5d249a8e27, apply the same
fix for user_type_impl.
This works around https://bugs.llvm.org/show_bug.cgi?id=47747
Depending on this might be unstable, as the bug bug can show up at any
corner, but this is sufficient right now to get
test_user_function_disabled to pass.
Closes#7370
Our cql parser uses large amounts of stack, and can overflow it
in debug mode with clang. To prevent this stack overflow,
temporarily use a larger (1MB) stack.
Closes#7369
* github.com:scylladb/scylla:
cql3: use larger stack for do_with_cql_parser() in debug mode
cql3: deinline do_with_cql_parser()
Clang has a bug processing inline ifuncs with intrinsics[1].
Since ifuncs can't be inlined anyway (they are always dispatched
via a function pointer that is determined based on the CPU
features present), nothing is gained by inlining them. Deinlining
therefore reduces compile time and works around the clang bug.
[1] https://bugs.llvm.org/show_bug.cgi?id=47691Closes#7358
Our cql parser uses large amounts of stack, and can overflow it
in debug mode with clang. To prevent this stack overflow,
temporarily use a larger (1MB) stack.
We can't use seastar::thread(), since do_with_cql_parser() does
not yield. We can't use std::thread(), since lw_shared_ptr()'s
debug mode will scream murder at an lw_shared_ptr used across
threads (even though it's perfectly safe in this case). We
can't use boost::context2 since that requires the library to
be compiled with address sanitizer support, which it isn't on
Fedora. So we use a fiber switch using the getcontext() function
familty. This requires extra annotations for debu mode, which are
added.
The cql parser causes trouble with the santizers and clang,
since it consumes a large amount of stack space (it does so
with gcc too, but does not overflow our 128k stacks). In
preparation for working around the problem, deinline it
so the hacks need not spread to the entire code base
via #include.
There is no performance impact from the virtual function,
as cql parsing will dominate the call.
Raft tests with declarative structure instead of procedural.
* https://github.com/alecco/scylla/tree/raft-ale-tests-03d:
raft: log failed test case name
raft: test add hasher
raft: declarative tests
raft: test make app return proper exit int value
raft: test add support for disconnected server
raft: tests use custom server ids for easier debugging
raft: make election_elapsed public for testing
raft: test remove unnecessary header
raft: fix typo snaphot snapshot
Values seen by nodes were so far added but this does not provide a
guarantee the order of these values was respected.
Use a digest to check output, implicitly checking order.
On the other hand, sum or a simple positional checksum like Fletcher's
is easier to debug as rolling sum is evident.
Signed-off-by: Alejo Sanchez <alejo.sanchez@scylladb.com>
For convenience making Raft tests, use declarative structures.
Servers are set up and initialized and then updates are processed.
For now, updates are just adding entries to leader and change of leader.
Updates and leader changes can be specified to run after initial test setup.
An example test for 3 nodes, node 0 starting as leader having two entries
0 and 1 for term 1, and with current term 2, then adding 12 entries,
changing leader to node 1, and adding 12 more entries. The test will
automatically add more entries to the last leader until the test limit
of total_values (default 100).
{.name = "test_name", .nodes = 3, .initial_term = 2,
.initial_states = {{.le = {{1,0},{1,1}}},
.updates = {entries{12},new_leader{1},entries{12}},},
Leader is isolated before change via is_leader returning false.
Initial leader (default server 0) will be set with this method, too.
Signed-off-by: Alejo Sanchez <alejo.sanchez@scylladb.com>
The get_range_to_endpoint_map method, takes a keyspace and returns a map
between the token ranges and the endpoint.
It is used by some external tools for repair.
Token ranges are codes as size-2 array, if start or end are empty, they will be
added as an empty string.
The implementation uses get_range_to_address_map and re-pack it
accordingly.
The use of stream_range_as_array it to reduce the risk of large
allocations and stalls.
Relates to scylladb/scylla-jmx#36
Signed-off-by: Amnon Heiman <amnon@scylladb.com>
Closes#7329
shutil.rmtree(ignore_errors=True) was for ignores error when directory not exist,
but it also ignores permission error, so we shouldn't use that.
Run os.path.exists() before shutil.rmtree() instead.
Fixes#7337Closes#7338
This series prepares the build system for clang support. It deals with
the different sets of warnings accepted by clang and gcc, and with
detecting clang 10 as a supported compiler.
It's still not possible to build with clang after this, but we're
another step closer.
Closes#7269
* github.com:scylladb/scylla:
build: detect and allow clang 10 as a compiler
build: detect availablity of -Wstack-usage=
build: disable many clang-specific warnings
rapidjson has a harmless (but true) ubsan violation. It was fixed
in 16872af889.
Since rapidjson has't released since 2016, we're unlikely to see
the fix, so suppress it to prevent the tests failing. In any case
the violation is harmless.
gcc's ubsan doesn't object to the addition.
Closes#7357
Send more that one entry in single append_entry message but
limit one packets size according to append_request_threshold parameter.
Message-Id: <20201007142602.GA2496906@scylladb.com>
Updated for the ability to add group names to SMP service groups
(https://github.com/scylladb/seastar/pull/809).
* seastar 8c8fd3ed...c62c4a3d (3):
> smp service group: add optional group name
> dpdk: mark link_ready() function override
> Merge "sharded: make start, stop, and invoke_on methods noexcept" from Benny
Fix a race condition in on_compaction_completion() that can prevent shutdown,
as well as an exception handling error. See individual patches for details.
Fixes#7331.
Closes#7334
* github.com:scylladb/scylla:
table: fix mishandled _sstable_deleted_gate exception in on_compaction_completion
table: fix on_compaction_completion corrupting _sstables_compacted_but_not_deleted during self-race
Instead of eagerly linearizing all values as they are passed to
validate(), defer linearization to those validators that actually need
linearized values. Linearizing large values puts pressure on the memory
allocator with large contiguous allocation requests. This is something
we are trying to actively avoid, especially if it is not really neaded.
Turns out the types, whose validators really want linearized values are
a minority, as most validators just look at the size of the value, and
some like bytes don't need validation at all, while usually having large
values.
This is achieved by templating the validator struct on the view and
using the FragmentedRange concept to treat all passed in views
(`bytes_view` and `fragmented_temporary_buffer_view`) uniformly.
This patch makes no attempt at converting existing validators to work
with fragmented buffers, only trivial cases are converted. The major
offenders still left are ascii/utf8 and collections.
Fixes: #7318
Tests: unit(dev)
Signed-off-by: Botond Dénes <bdenes@scylladb.com>
Message-Id: <20201007054524.909420-1-bdenes@scylladb.com>
After adding a new node to the cluster, Scylla sends a NEW_NODE event
to CQL clients. Some clients immediately try to connect to the new node,
however it fails as the node has not yet started listening to CQL
requests.
In contrast, Apache Cassandra waits for the new node to start its CQL
server before sending NEW_NODE event. In practice this means that
NEW_NODE and UP events will be sent "jointly" after new node is UP.
This change is implemented in the same manner as in Apache Cassandra
code.
Fixes#7301.
Closes#7306
"
There are few places left that call for global query processor instance,
the tracing is one of them.
The query pressor is used mainly in table_helper, so this set mostly
shuffles its methods' arguments to deliver the needed reference. At the
end the main.cc code is patched to provide the query processor, which
is still global and not stopped, and is thus safe to be used anywhere.
tests: unit(dev), dtest(cql_tracing:dev)
"
* 'br-tracing-vs-query-processor' of https://github.com/xemul/scylla:
tracing: Keep qp anchor on backend
tracing: Push query processor through init methods
main: Start tracing in main
table_helper: Require local query processor in calls
table_helper: Use local qp as setup_table argument
table_helper: Use local db variable
"
The querier cache has a memory based eviction mechanism, which starts
evicting freshly inserted queriers once their collective memory
consumption goes above the configured limit. For determining the memory
consumption of individual queriers, the querier cache uses
`flat_mutation_reader::buffer_size()`. But we now have a much more
comprehensive accounting of the memory used by queriers: the reader
permit, which also happens to be available in each querier. So use this
to determine the querier's memory consumption instead.
Tests: unit(dev)
"
* 'querier-cache-use-permit-for-memory-accounting/v1' of https://github.com/denesb/scylla:
flat_mutation_reader: de-virtualize buffer_size()
querier_cache: use the reader permit for memory accounting
querier_cache_test: use local semaphore not the test global one
reader_permit: add consumed_resources() accessor
The query processor is required in table_helper's used by tracing. Now
everything is ready to push the query processor reference from main down
to the table helpers.
Because of the current initialization sequence it's only possible to have
the started query processor at the .start_tracing() time. Earlier, when
the sharded<tracing> is started the query processor is not yet started,
so tracing keeps a pointer on local query processor.
When tracing is stopped, the pointer is null-ed. This is safe (but an
assert is put when dereferencing it), because on stop trace writes' gate
is closed and the query processor is only used in them.
Also there's still a chance that tracing remains started in case of start
abort, but this is on-par with the current code -- sharded query processor
is not stopped, so the memory is not freed.
Signed-off-by: Pavel Emelyanov <xemul@scylladb.com>
The goal is to make tracing keyspace helper reference query processor, so this
patch adds the needed arguments through the initialization stack.
Signed-off-by: Pavel Emelyanov <xemul@scylladb.com>
Move the tracing::start_tracing() out of the storage_service::join_cluster. It
anyway happens at the end of the join, so the logic is not changed, but it
becomes possible to patch tracing further.
Signed-off-by: Pavel Emelyanov <xemul@scylladb.com>
Keeping the query processor reference on the table_helper in raii manner
seems waistful, the only user of it -- the trace_keyspace_helper -- has
a bunch of helpers on board, each would then keep its own copy for no
gain.
At the same time the trace_keyspace_helper already gets the query processor
for its needs, so it can share one with table_helper-s.
Signed-off-by: Pavel Emelyanov <xemul@scylladb.com>
The goal is to make table_helper API require the query_processor
reference and use it where needed. The .setup_table() is private
method, and still grabs the query processor reference itself. Since
its futures do noth reshard, it's safe to carry the query processor
reference through.
Signed-off-by: Pavel Emelyanov <xemul@scylladb.com>
The test was broken by recent sstables manager rework. In the middle
the sstables::test_env is destroyed without being closed which leads
to broken _closing assertion inside ~sstables_manager().
Fix is to use the test_env::do_with helper.
tests: perf.memory_footprint
* https://github.com/xemul/scylla/tree/br-memory-footprint-test-fix:
test/perf/memory_footprint: Fix indentation after previous patch
test/perf/memory_footprint: Don't forget to close sstables::test_env after usage
After recent sstables manager rework the sstables::test_env must be .close()d
after usage, otherwise the ~sstables_mananger() hits the _closing assertion.
Do it with the help of .do_with(). The execution context is already seastar::async
in this place, so .get() it explicitly.
Signed-off-by: Pavel Emelyanov <xemul@scylladb.com>
on_compaction_completion tries to handle a gate_closed_exception, but
with_gate() throws rather than creating an exceptional future, so
the extra handling is lost. This is relatively benign since it will
just fail the compaction, requiring that work to be redone later.
Fix by using the safer try_with_gate().
on_compaction_completion() updates _sstables_compacted_but_not_deleted
through a temporary to avoid an exception causing a partial update:
1. copy _sstables_compacted_but_not_deleted to a temporary
2. update temporary
3. do dangerous stuff
4. move temporary to _sstables_compacted_but_not_deleted
This is racy when we have parallel compactions, since step 3 yields.
We can have two invocations running in parallel, taking snapshots
of the same _sstables_compacted_but_not_deleted in step 1, each
modifying it in different ways, and only one of them winning the
race and assigning in step 4. With the right timing we can end
with extra sstables in _sstables_compacted_but_not_deleted.
Before a5369881b3, this was a benign race (only resulting in
deleted file space not being reclaimed until the service is shut
down), but afterwards, extra sstable references result in the service
refusing to shut down. This was observed in database_test in debug
mode, where the race more or less reliably happens for system.truncated.
Fix by using a different method to protect
_sstables_compacted_but_not_deleted. We unconditionally update it,
and also unconditionally fix it up (on success or failure) using
seastar::defer(). The fixup includes a call to rebuild_statistics()
which must happen every time we touch the sstable list.
Fixes#7331.
