Commit Graph

29611 Commits

Author SHA1 Message Date
Avi Kivity
b850b34bcc build: reduce inline threshold on aarch64 to 300
We see coroutine miscompiles with 600.

Fixes #9881.

Closes #9883
2022-01-06 15:13:27 +02:00
Nadav Har'El
6e2d29300c test/scylla-gdb: a rewrite, using pytest
This patch is an almost complete rewrite of the test/scylla-gdb
framework for testing Scylla's gdb commands.

The goals of this rewrite are described in issue #9864. In short, the
goals are:

1. Use pytest to define individual test cases instead one long Python
   script. This will make it easier to add more tests, to run only
   individual tests (e.g., test/scylla-gdb/run somefile.py::sometest),
   to understand which test failed when it fails - and a lot of other
   pytest conveniences.

2. Instead of an ad-hoc shell script to run Scylla, gdb, and the test,
   use the same Python code which is used in other test suites (alternator,
   cql-pytest, redis, and more). The resulting handling of the temporary
   resources (processes, directories, IP address) is more robust, and
   interrupting test/scylla-gdb/run will correctly kill its child
   processes (both Scylla and gdb).

All existing gdb tests (except one - more on this below...) were
easily rewritten in the new framework.

The biggest change in this patch is who starts what. Before this patch,
"run" starts gdb, which in turn starts Scylla, stops it on a breakpoint,
and then runs various tests. After this patch, "run" starts Scylla on
its own (like it does in test/cql-pytest/run, et al.), and then gdb runs
pytest - and in a pytest fixture attaches to the running Scylla process.
The biggest benefit of this approach is that "run" is aware of both gdb
and Scylla, and can kill both with abruptly with SIGKILL to end the test.
But there's also a downside to this change: One of the tests (of "scylla
fiber") needs access to some task object. Before this patch, Scylla was
stopped on a breakpoint, and a task was available at that point. After
this patch, we attach gdb to an idle Scylla, and the test cannot find
any task to use. So the test_fiber() test fails for now.

One way we could perhaps fix it is to add a breakpoint and "continue"
Scylla a bit more after attaching to it. However, I could find the right
breakpoint - and we may also need to send a request to Scylla to
get it to reach that breakpoint. I'm still looking for a better way
to have access to some "task" object we can test on.

Fixes #9864.

Signed-off-by: Nadav Har'El <nyh@scylladb.com>
Message-Id: <20220102221534.1096659-1-nyh@scylladb.com>
2022-01-06 11:29:55 +02:00
Nadav Har'El
d9fe6f4c96 Merge: main: improve tool integration
This set contains follow-up fixes to folding tools into the scylla
executable:
* Improve the app description of scylla w.r.t. tools
* Add a new --list-tools option
* Error out when the first argument is unrecognized

Tests: unit(dev)

Botond Dénes (3):
  main: rephrase app description
  main: add move tool listing to --list-tools
  main: improve handling of non-matching argv[1]

 main.cc | 29 +++++++++++++++++++----------
 1 file changed, 19 insertions(+), 10 deletions(-)
2022-01-06 10:06:28 +02:00
Botond Dénes
a37b4bbbaf main: improve handling of non-matching argv[1]
Be silent when argv[1] starts with "-", it is probably an option to
scylla (and "server" is missing from the cmd line).
Print an error and stop when argv[1] doesn't start with "-" and thus the
user assumably meant to start either the server or a tool and mis-typed
it. Instead of trying to guess what they meant stop with a clear error
message.
2022-01-06 06:59:59 +02:00
Botond Dénes
fe0bfa1d7b main: add move tool listing to --list-tools
And make it the central place listing available tools (to minimize the
places to update when adding a new one). The description is edited to
point to this command instead of listing the tools itself.
2022-01-06 06:58:44 +02:00
Botond Dénes
ab0e39503b main: rephrase app description
Remove "compatible with Apache Cassandra", scylla is much more than that
already.

Rephrase the part describing the included tools such that it is clear
that the scylla server is the main thing and the tools are the "extra"
additions. Also use the term "tool" instead of the term "app".
2022-01-06 06:37:32 +02:00
Botond Dénes
92727ac36c sstables/partition_index_cache: destroy entry ptr on error
The error-handling code removes the cache entry but this leads to an
assertion because the entry is still referenced by the entry pointer
instance which is returned on the normal path. To avoid this clear the
pointer on the error path and make sure there are no additional
references kept to it.

Signed-off-by: Botond Dénes <bdenes@scylladb.com>
Message-Id: <20220105140859.586234-2-bdenes@scylladb.com>
2022-01-05 19:03:24 +01:00
Nadav Har'El
6ebf32f4d7 types: deinline template throw_with_backtrace<marshal_exception, sstring>
When a template is instantiated in a header file which is included by many
source files, the compiler needs to compile it again and again.
ClangBuildAnalyzer helps find the worst cases of this happening, and one
of the worst happens to be

   seastar::throw_with_backtrace<marshal_exception, sstring>

This specific template function takes (according to ClangBuildAnalyzer)
362 milliseconds to instantiate, and this is done 312 (!) times, because
it reaches virtually every Scylla source file via either types.hh or
compound.hh which use this idiom.

Unfortunately, C++ as it exists today does not have a mechanism to
avoid compiling a specific template instantiation if this was already
done in some other source file. But we can do this manually using
the C++11 feature of "extern template":

1. For a specific template instance, in this case
   seastar::throw_with_backtrace<marhsal_exception, sstring>,
   all source files except one specify it as "extern template".
   This means that the code for it will NOT be built in this source
   file, and the compiler assumes the linker will eventually supply it.

2. At the same time, one source file instantiates this template
   instance once regularly, without "extern".

The numbers from ClangBuildAnalyzer suggest that this patch should
reduce total build time by 1% (in dev build mode), but this is hard to
measure in practice because the very long build time (210 CPU minutes on
my laptop) usually fluctuates by more than 1% in consecutive runs.
However, we've seen in the past that a good estimate of build time is
the total produced object size (du -bc build/dev/**/*.o). This patch
indeed reduces this total object size (in dev build mode) by exactly 1%.

Signed-off-by: Nadav Har'El <nyh@scylladb.com>
Message-Id: <20220105171453.308821-1-nyh@scylladb.com>
2022-01-05 19:23:40 +02:00
Avi Kivity
d01e1a774b Merge 'Build performance: do not include the entire <seastar/net/ip.hh>' from Nadav Har'El
The header file <seastar/net/ip.hh> is a large collection of unrelated stuff, and according to ClangBuildAnalyzer, takes 2 seconds to compile for every source file that included it - and unfortunately virtually all Scylla source files included it - through either "types.hh" or "gms/inet_address.hh". That's 2*300 CPU seconds wasted.

In this two-patch series we completely eliminate the inclusion of <seastar/net/ip.hh> from Scylla. We still need the ipv4_address, ipv6_address types (e.g., gms/inet_address.hh uses it to hold a node's IP address) so those were split (in a Seastar patch that is already in) from ip.hh into separate small header files that we can include.

This patch reduces the entire build time (of build/dev/scylla) by 4% - reducing almost 10 sCPU minutes (!) from the build.

Closes #9875

* github.com:scylladb/scylla:
  build performance: do not include <seastar/net/ip.hh>
  build performance: speed up inclusion of <gm/inet_address.hh>
2022-01-05 17:55:07 +02:00
Nadav Har'El
6012f6f2b6 build performance: do not include <seastar/net/ip.hh>
In a previous patch, we noticed that the header file <gm/inet_address.hh>,
which is included, directly or indirectly, by most source files,
includes <seastar/net/ip.hh> which is very slow to compile, and
replaced it by the much faster-to-include <seastar/net/ipv[46]_address.hh>.

However, we also included <seastar/net/ip.hh> in types.hh - and that
too is included by almost every file, so the actual saving from the
above patch was minimal. So in this patch we replace this include too.
After this patch Scylla does not include <seastar/net/ip.hh> at all.

According to ClangBuildAnalyzer, this reduces the average time to include
types.hh (multiply this by 312 times!) from 4 seconds to 1.8 seconds,
and reduces total build time (dev mode) by about 3%.

Some of the source files were now missing some include directives, that
were previously included in ip.hh - so we need to add those explicitly.

Signed-off-by: Nadav Har'El <nyh@scylladb.com>
2022-01-05 17:29:21 +02:00
Tomasz Grabiec
382797a627 tests: perf: perf_fast_forward: Fix test_large_partition_slicing_clustering_keys for scylla_bench_large_part_ds1 schema
The test case assumed int32 partition key, but
scylla_bench_large_part_ds1 has int64 partition key. This resulted in
no results to be returned by the reader.

Fixs by introducing a partition key factory on the data source level.

Message-Id: <20220105150550.67951-1-tgrabiec@scylladb.com>
2022-01-05 17:18:06 +02:00
Nadav Har'El
788b9c7bc0 dbuild: better documentation for how to use with ccache
dbuild's README contained some vague and very partial hints on how to use
ccache with dbuild. Replace them with more concrete instructions.

Signed-off-by: Nadav Har'El <nyh@scylladb.com>
Message-Id: <20211229180433.781906-1-nyh@scylladb.com>
2022-01-05 16:53:08 +02:00
Botond Dénes
015d09a926 tools: utils: add configure_tool_mode()
Which configures seastar to act more appropriate to a tool app. I.e.
don't act as if it owns the place, taking over all system resources.
These tools are often run on a developer machine, or even next to a
running scylla instance, we want them to be the least intrusive
possible.
Also use the new tool mode in the existing tools.

Signed-off-by: Botond Dénes <bdenes@scylladb.com>
Message-Id: <20211220143104.132327-1-bdenes@scylladb.com>
2022-01-05 15:33:57 +02:00
Asias He
c5784c1149 repair: Sort follower nodes by proximity
Sort follower nodes by the proximity so that in the step where the
master node gets missing rows from repair follower nodes,the master
node has a chance to get the missing rows from a near node first (e.g.,
local dc node), avoding getting rows from a far node.

For example:

dc1: n1, n2
dc2: n3, n4
dc3: n5, n6

Run repair on n1, with this patch, n1 will get data from n2 which is in the same dc first.

[shard 0] repair - Repair 1 out of 1 ranges, id=[id=1, uuid=8b0040bd-5aa5-42e1-bb9f-58c5e7052aec],
shard=0, keyspace=ks, table={cf}, range=(-6734413101754081925, -6539883972247625343],
peers={127.0.39.5, 127.0.39.6, 127.0.39.2, 127.0.39.4, 127.0.39.3},
live_peers={127.0.39.5, 127.0.39.6, 127.0.39.2, 127.0.39.4, 127.0.39.3}

[shard 0] repair - Before sort = {127.0.39.5, 127.0.39.6, 127.0.39.2, 127.0.39.4, 127.0.39.3}

[shard 0] repair - After sort = {127.0.39.2, 127.0.39.5, 127.0.39.6, 127.0.39.4, 127.0.39.3}

[shard 0] repair - Started Row Level Repair (Master): local=127.0.39.1,
peers={127.0.39.2, 127.0.39.5, 127.0.39.6, 127.0.39.4, 127.0.39.3}

Closes #9769
2022-01-05 14:09:59 +02:00
Nadav Har'El
e7e9001808 test/alternator: add more tests for GSI "Projection"
We already have multiple tests for the unimplemented "Projection" feature
of GSI and LSI (see issue #5036). This patch adds seven more test cases,
focusing on various types of errors conditions (e.g., trying to project
the same attribute twice), esoteric corner cases (it's fine to list a key in
NonKeyAttributes!), and corner cases that I expect we will have in our
implementation (e.g., a projected attribute may either be a real Scylla
column or just an element in a map column).

All new tests pass on DynamoDB and fail on Alternator (due to #5036), so
marked with "xfail".

Refs #5036.

Signed-off-by: Nadav Har'El <nyh@scylladb.com>
Message-Id: <20211228193748.688060-1-nyh@scylladb.com>
2022-01-05 10:35:36 +02:00
Avi Kivity
53a83c4b1e Merge "flat_mutation_reader: convert flat_mutation_reader_from_mutations to v2" from Botond
"
Like flat_mutation_reader_from_fragments, this reader is also heavily
used by tests to compose a specific workload for readers above it. So
instead of converting it, we add a v2 variant and leave the v1 variant
in place.
The v2 variant was written from scratch to have built-in support for
reading in reverse. It is built-on `mutation::consume()` to avoid
duplicating the logic of consuming the contents of the mutation. To
avoid stalls, `mutation::consume()` gets support for pausing and
resuming consuming a mutation.

Tests: unit(dev)
"

* 'flat_mutation_reader_from_mutations_v2/v2' of https://github.com/denesb/scylla:
  flat_mutation_reader: convert make_flat_mutation_reader_from_mutation() v2
  flat_mutation_reader: extract mutation slicing into a function
  mutation: consume(): make it pausable/resumable
  mutation: consume(): restructure clustering iterator initialization
  test/boost/mutation_test: add rebuild test for mutation::consume()
2022-01-05 10:23:17 +02:00
Avi Kivity
2e958b3555 Merge "Coroutinization of compaction sstable rewrite procedure" from Raphael
"
Completes coroutinization of rewrite_sstables().

tests: UNIT(debug)
"

* 'rewrite_sstable_coroutinization' of https://github.com/raphaelsc/scylla:
  compaction_manager: coroutinize main loop in sstable rewrite procedure
  compaction_manager: coroutinize exception handling in sstable rewrite procedure
  compaction_manager: mark task::finish_compaction() as noexcept
  compaction_manager: make maybe_stop_on_error() more flexible
2022-01-05 10:15:19 +02:00
Raphael S. Carvalho
426450dc04 treewide: remove useless include of database.hh
Wrote a script based on cpp-include to find places that needlessly
included database.hh, which is expensive to process during
build time.

Signed-off-by: Raphael S. Carvalho <raphaelsc@scylladb.com>
Message-Id: <20220104204359.168895-1-raphaelsc@scylladb.com>
2022-01-05 10:15:19 +02:00
Nadav Har'El
dcc42d3815 configure.py: re-run configure.py if the build/ directory is gone
When you run "configure.py", the result is not only the creation of
./build.ninja - it also creates build/<mode>/seastar/build.ninja
and build/<mode>/abseil/build.ninja. After a "rm -r build" (or "ninja
clean"), "ninja" will no longer work because those files are missing
when Scylla's ninja tries to run ninja in those internal project.

So we need to add a dependency, e.g., that running ninja in Seastar
requires build/<mode>/seastar/build.ninja to exist, and also say
that the rule that (re)runs "configure.py" generates those files.

After this patch,

        configure.py --with-some-parameters --of-your-choice
        rm -r build
        ninja

works - "ninja" will re-run configure.py with the same parameters
when it needs Seastar's or Abseil's build.ninja.

Signed-off-by: Nadav Har'El <nyh@scylladb.com>
Message-Id: <20211230133702.869177-1-nyh@scylladb.com>
2022-01-05 10:15:19 +02:00
Nadav Har'El
5fbeae9016 cql-pytest: add a couple of default-TTL tests
This patch adds a new cql-pytest test file - test_ttl.py - with
currently just a couple of tests for the "with default_time_to_live"
feature. One is a basic test, and second reproduces issue #9842 -
that "using ttl 0" should override the default time to live, but
doesn't.

The test for #9842, test_default_ttl_0_override, fails on Scylla and
passes on Cassandra, and is marked "xfail".

Refs #9842.

Signed-off-by: Nadav Har'El <nyh@scylladb.com>
Message-Id: <20211227091502.553577-1-nyh@scylladb.com>
2022-01-05 10:15:19 +02:00
Benny Halevy
e0a351e0c6 compaction_manager: stop_compaction: disallow specific types
We can stop only specific compaction types.

Reshard should be excluded since it mustn't be stopped.

And other types of compaction types like "VALIDATION" or "INDEX_BUILD"
are valid in terms of their syntax but unsupported by scylla so we better
return an error rather than appear to support them.

Test: unit(dev)

Signed-off-by: Benny Halevy <bhalevy@scylladb.com>
Message-Id: <20211222133449.2177746-1-bhalevy@scylladb.com>
2022-01-05 09:32:20 +02:00
Botond Dénes
62d82b8b0e flat_mutation_reader: convert make_flat_mutation_reader_from_mutation() v2
Since this reader is also heavily used by tests to compose a specific
workload for readers above it, we just add a v2 variant, instead of
changing the existing v1 one.
The v2 variant was written from scratch to have built-in support for
reading in reverse. It is built-on `mutation::consume()` to avoid
duplicating the logic of consuming the contents of the mutation.

A v2 native unit test is also added.
2022-01-05 09:06:16 +02:00
Botond Dénes
2d1bb90c8e flat_mutation_reader: extract mutation slicing into a function 2022-01-05 09:06:16 +02:00
Botond Dénes
e8ca07abed mutation: consume(): make it pausable/resumable
To avoid stalls or overconsumption for consumers which have a limit on
how much they want to consume in one go, the mutation::consume() is made
pausable/resumable. This happens via a cookie which is now returned as
part of the returned result, and which can be passed to a later
consume call to resume the previous one.
2022-01-05 09:06:16 +02:00
Botond Dénes
f1391d5c27 mutation: consume(): restructure clustering iterator initialization
Instead of having a branch per each value of `consume_in_reverse`, have
just two ifs with two branches each for clustering rows and range
tombstones respectively, to facilitate further patching.
2022-01-05 07:29:36 +02:00
Nadav Har'El
3fbbad7d60 build performance: speed up inclusion of <gm/inet_address.hh>
The header file <gm/inet_address.hh> is included, directly or
indirectly, from 291 source files in Scylla. It is hard to reduce this
number because Scylla relies heavily on IP addresses as keys to
different things. So it is important that this header file be fast to
include. Unfortunately it wasn't... ClangBuildAnalyzer measurements
showed that each inclusion of this header file added a whopping 2 seconds
(in dev build mode) to the build. A total of 600 CPU seconds - 10 CPU
minutes - were spent just on this header file. It was actually worse
because the build also spent additional time on template instantiation
(more on this below).

So in this patch we:

1. Remove some unnecessary stuff from gms/inet_address.hh, and avoid
   including it in one place that doesn't need it. This is just
   cosmetic, and doesn't significantly speed up the build.

2. Move the to_sstring() implementation for the .hh to .cc. This saves
   a lot of time on template instantiations - previously every source
   file instantiated this to_sstring(), which was slow (that "format"
   thing is slow).

3. Do not include <seastar/net/ip.hh> which is a huge file including
   half the world. All we need from it is the type "ipv4_address",
   so instead include just the new <seastar/net/ipv4_address.hh>.
   This change brings most of the performance improvement.
   So source files forgot to include various Seastar header files
   because the includes-everything ip.hh did it - so we need to add
   these missing includes in this patch.

After this patch, ClangBuildAnalyzer's reports that the cost of
inclusion of <gms/inet_address.hh> is down from 2 seconds to 0.326
seconds. Additionally the format<inet_address> template instantiation
291 times - about half a second each - is also gone.

All in all, this patch should reduce around 10 CPU minutes from the build.

Refs #1

Signed-off-by: Nadav Har'El <nyh@scylladb.com>
2022-01-04 21:07:23 +02:00
Raphael S. Carvalho
f0b816d8e8 compaction_manager: coroutinize main loop in sstable rewrite procedure
with this patch, rewrite_sstables() is now fully coroutinized.

Signed-off-by: Raphael S. Carvalho <raphaelsc@scylladb.com>
2022-01-04 16:03:23 -03:00
Raphael S. Carvalho
c85ba1e694 compaction_manager: coroutinize exception handling in sstable rewrite procedure
Signed-off-by: Raphael S. Carvalho <raphaelsc@scylladb.com>
2022-01-04 15:39:54 -03:00
Raphael S. Carvalho
59a65742f9 compaction_manager: mark task::finish_compaction() as noexcept
As it's intended to be used in a deferred action.

Signed-off-by: Raphael S. Carvalho <raphaelsc@scylladb.com>
2022-01-04 15:30:04 -03:00
Raphael S. Carvalho
3fe4c2e517 compaction_manager: make maybe_stop_on_error() more flexible
It's hard to integrate maybe_stop_on_error() with coroutines as it
accepts a resolved future, not an exception pointer. Let's adjust
its interface, making it more flexible to work with.

Signed-off-by: Raphael S. Carvalho <raphaelsc@scylladb.com>
2022-01-04 15:28:30 -03:00
Raphael S. Carvalho
9a1fdb0635 sstables: stop including unused expensive headers
database.hh is expensive to include, and turns out it's no longer
needed. also stop including other unused ones.

build time of sstables.o reduces by ~3% (cleared all caches and set
cpu frequency to a fixed value before building sstables.o from
scratch)

Signed-off-by: Raphael S. Carvalho <raphaelsc@scylladb.com>
Message-Id: <20220104175908.98833-1-raphaelsc@scylladb.com>
2022-01-04 20:14:01 +02:00
Asias He
25b036f35b repair: Improve memory usage tracking and oom protection
Currently, the repair parallelism is calculated by the number of memory
allocated to repair and memory usage per repair instance. However, the
memory usage per repair instance does not take the max possible memory
usage caused by repair followers.

As a result, when repairing a table with more replication factors, e.g.,
3 DCs, each has 3 replicas, the repair master node would use 9X repair
buffer size in worse cases. This would cause OOM when the system is
under pressure.

This patch introduces a semaphore to cap the max memory usage.

Each repair instance takes the max possible memory usage budget before
it starts. This ensures repair would never use more than the memory
allocated to repair.

Fixes #9817

Closes #9818.
2022-01-04 20:11:36 +02:00
Asias He
a8ad385ecd repair: Get rid of the gc_grace_seconds
The gc_grace_seconds is a very fragile and broken design inherited from
Cassandra. Deleted data can be resurrected if cluster wide repair is not
performed within gc_grace_seconds. This design pushes the job of making
the database consistency to the user. In practice, it is very hard to
guarantee repair is performed within gc_grace_seconds all the time. For
example, repair workload has the lowest priority in the system which can
be slowed down by the higher priority workload, so that there is no
guarantee when a repair can finish. A gc_grace_seconds value that is
used to work might not work after data volume grows in a cluster. Users
might want to avoid running repair during a specific period where
latency is the top priority for their business.

To solve this problem, an automatic mechanism to protect data
resurrection is proposed and implemented. The main idea is to remove the
tombstone only after the range that covers the tombstone is repaired.

In this patch, a new table option tombstone_gc is added. The option is
used to configure tombstone gc mode. For example:

1) GC a tombstone after gc_grace_seconds

cqlsh> ALTER TABLE ks.cf WITH tombstone_gc = {'mode':'timeout'} ;

This is the default mode. If no tombstone_gc option is specified by the
user. The old gc_grace_seconds based gc will be used.

2) Never GC a tombstone

cqlsh> ALTER TABLE ks.cf WITH tombstone_gc = {'mode':'disabled'};

3) GC a tombstone immediately

cqlsh> ALTER TABLE ks.cf WITH tombstone_gc = {'mode':'immediate'};

4) GC a tombstone after repair

cqlsh> ALTER TABLE ks.cf WITH tombstone_gc = {'mode':'repair'};

In addition to the 'mode' option, another option 'propagation_delay_in_seconds'
is added. It defines the max time a write could possibly delay before it
eventually arrives at a node.

A new gossip feature TOMBSTONE_GC_OPTIONS is added. The new tombstone_gc
option can only be used after the whole cluster supports the new
feature. A mixed cluster works with no problem.

Tests: compaction_test.py, ninja test

Fixes #3560

[avi: resolve conflicts vs data_dictionary]
2022-01-04 19:48:14 +02:00
Avi Kivity
5eccb42846 Merge "Host tool executables in the scylla main executable" from Botond
"
A big problem with scylla tool executables is that they include the
entire scylla codebase and thus they are just as big as the scylla
executable itself, making them impractical to deploy on production
machines. We could try to combat this by selectively including only the
actually needed dependencies but even ignoring the huge churn of
sorting out our depedency hell (which we should do at one point anyway),
some tools may genuinely depend on most of the scylla codebase.

A better solution is to host the tool executables in the scylla
executable itself, switching between the actual main function to run
some way. The tools themselves don't contain a lot of code so
this won't cause any considerable bloat in the size of the scylla
executable itself.
This series does exactly this, folds all the tool executables into the
scylla one, with main() switching between the actual main it will
delegate to based on a argv[1] command line argument. If this is a known
tool name, the respective tool's main will be invoked.
If it is "server", missing or unrecognized, the scylla main is invoked.

Originally this series used argv[0] as the mean to switch between the
main to run. This approach was abandoned for the approach mentioned above
for the following reasons:
* No launcher script, hard link, soft link or similar games are needed to
  launch a specific tool.
* No packaging needed, all tools are automatically deployed.
* Explicit tool selection, no surprises after renaming scylla to
  something else.
* Tools are discoverable via scylla's description.
* Follows the trend set by modern command line multi-command or multi-app
  programs, like git.

Fixes: #7801

Tests: unit(dev)
"

* 'tools-in-scylla-exec-v5' of https://github.com/denesb/scylla:
  main,tools,configure.py: fold tools into scylla exec
  tools: prepare for inclusion in scylla's main
  main: add skeleton switching code on argv[1]
  main: extract scylla specific code into scylla_main()
2022-01-04 17:55:07 +02:00
Botond Dénes
5e547dcc8a test/boost/mutation_test: add rebuild test for mutation::consume()
In the next patches we will refactor mutation::consume(). Before doing
that add another test, which rebuilds the consumed mutation, comparing
it with the original.
2022-01-04 11:43:46 +02:00
Nadav Har'El
e0ebde0f4f Update seastar submodule
The split of <seastar/net/ip.hh> will be useful for reducing the build
time (ip.hh is huge and we don't need to include most of it)
Refs #1

* seastar 8d15e8e6...655078df (13):
  > net: split <seastar/net/ip.hh>
  > Merge "Rate-limited IO capacity management" from Pavel E
  > util: closeable/stoppable: Introduce cancel()
  > loop: Improve concepts to match requirements
  > Merge "scoped_critical_alloc_section make conditional and volatile" from Benny
  > Added variadic version of when_any
  > websocket: define CryptoPP::byte for older cryptopp
  > tests: fix build (when libfmt >= 8) by adding fmt::runtime()
  > foreign_ptr: destroy_on: fixup indentation
  > foreign_ptr: expose async destroy method
  > when_all: when_all_state::wait_all move scoped_critical_alloc_section to captures
  > json: json_return_type: provide copy constructor and assignment operator
  > json: json_element: mark functions noexcept
2022-01-03 22:52:24 +02:00
Calle Wilund
3c02cab2f7 commitlog: Don't allow error_handler to swallow exception
Fixes #9798

If an exception in allocate_segment_ex is (sub)type of std::system_error,
commit_error_handler might _not_ cause throw (doh), in which case the error
handling code would forget the current exception and return an unusable
segment.

Now only used as an exception pointer replacer.

Closes #9870
2022-01-03 22:46:31 +02:00
Nadav Har'El
8774fc83d3 test/rest_api: fix "--ssl" option
test/rest_api has a "--ssl" option to use encrypted CQL. It's not clear
to me why this is useful (it doesn't actually test encryption of the
REST API!), but as long as we have such an option, it should work.

And it didn't work because of a typo - we set a "check_cql" variable to the
right function, but then forgot to use it and used run.check_cql instead
(which is just for unencrypted cql).

Signed-off-by: Nadav Har'El <nyh@scylladb.com>
Message-Id: <20220102123202.1052930-1-nyh@scylladb.com>
2022-01-02 15:53:25 +02:00
Benny Halevy
fc729a804b shard_reader: Continue after read_ahead error
If read ahead failed, just issue a log warning
and proceed to close the reader.

Currently co_await will throw and the evictable reader
won't be closed.

This is seen occasionally in testing, e.g.
https://jenkins.scylladb.com/view/master/job/scylla-master/job/dtest-debug/1010/artifact/logs-all.debug.2/1640918573898_lwt_banking_load_test.py%3A%3ATestLWTBankingLoad%3A%3Atest_bank_with_nemesis/node2.log
```
ERROR 2021-12-31 02:40:56,160 [shard 0] mutation_reader - shard_reader::close(): failed to stop reader on shard 1: seastar::named_semaphore_timed_out (Semaphore timed out: _system_read_concurrency_sem)
```

Fixes #9865.

Signed-off-by: Benny Halevy <bhalevy@scylladb.com>
Message-Id: <20220102124636.2791544-1-bhalevy@scylladb.com>
2022-01-02 15:52:09 +02:00
Pavel Emelyanov
36905ce19d scylla-gdb: Do not try to unpack None-s
When 'scylla fiber' calls _walk the latter can validly return back None
pointer (see 74ffafc8a7 scylla-gdb.py: scylla fiber: add actual return
to early return). This None is not handled by the caller but is unpacked
as if it was a valid tuple.

fixes: #9860
tests: scylla-gdb(release, failure not reproduced though)

Signed-off-by: Pavel Emelyanov <xemul@scylladb.com>
Message-Id: <20211231094311.2495-1-xemul@scylladb.com>
2021-12-31 22:21:58 +02:00
Pavel Emelyanov
946e03351e scylla-gdb: Handle rate-limited IO scheduler groups
The capacity accounting was changed, scylla-gdb.py should know
the new layout. On error -- fall back to current state.

tests: scylla-gdb(release, current and patched seastar)

Signed-off-by: Pavel Emelyanov <xemul@scylladb.com>
Message-Id: <20211231073427.32453-1-xemul@scylladb.com>
2021-12-31 22:20:45 +02:00
Pavel Solodovnikov
904de0a094 gms: introduce two gossip features for raft-based cluster management
The patch adds the `SUPPORTS_RAFT_CLUSTER_MANAGEMENT`
and `USES_RAFT_CLUSTER_MANAGEMENT` gossiper features.

These features provide a way to organize the automatic
switch to raft-based cluster management.

The scheme is as follows:
 1. Every new node declares support for raft-based cluster ops.
 2. At the moment, no nodes in the cluster can actually use
    raft for cluster management, until the `SUPPORTS*` feature is enabled
    (i.e. understood by every node in the cluster).
 3. After the first `SUPPORTS*` feature is enabled, the nodes
    can declare support for the second, `USES*` feature, which
    means that the node can actually switch to use raft-based cluster
    ops.

The scheme ensures that even if some nodes are down while
transitioning to new bootstrap mechanism, they can easily
switch to the new procedure, not risking to disrupt the
cluster.

The features are not actually wired to anything yet,
providing a framework for the integration with `raft_group0`
code, which is subject for a follow-up series.

Signed-off-by: Pavel Solodovnikov <pa.solodovnikov@scylladb.com>
Message-Id: <20211220081318.274315-1-pa.solodovnikov@scylladb.com>
2021-12-30 11:05:45 +02:00
Tomasz Grabiec
7038dc7003 lsa: Fix segment leak on memory reclamation during alloc_buf
alloc_buf() calls new_buf_active() when there is no active segment to
allocate a new active segment. new_buf_active() allocates memory
(e.g. a new segment) so may cause memory reclamation, which may cause
segment compaction, which may call alloc_buf() and re-enter
new_buf_active(). The first call to new_buf_active() would then
override _buf_active and cause the segment allocated during segment
compaction to be leaked.

This then causes abort when objects from the leaked segment are freed
because the segment is expected to be present in _closed_segments, but
isn't. boost::intrusive::list::erase() will fail on assertion that the
object being erased is linked.

Introduced in b5ca0eb2a2.

Fixes #9821
Fixes #9192
Fixes #9825
Fixes #9544
Fixes #9508
Refs #9573

Message-Id: <20211229201443.119812-1-tgrabiec@scylladb.com>
2021-12-30 11:02:08 +02:00
Piotr Jastrzebski
85f5277a05 max_result_size: Expand the comment
Add describtion about how SEPARATE_PAGE_SIZE_AND_SAFETY_LIMIT cluster
feature is used and note that only coordinators check it. Decision made
by a coordinator is immutable for the whole request and can be checked
by looking at page_size field. If it's set to 0 or unset then we're
handling the struct in the old way. Otherwise, new way is used.

Signed-off-by: Piotr Jastrzebski <piotr@scylladb.com>

Closes #9855
2021-12-29 17:34:15 +02:00
Avi Kivity
e6f7ade60c Update tools/java submodule (python2 dependency)
* tools/java 8fae618f7f...6249bfbe2f (1):
  > dist/debian: replace "python (>=2.7)" with "python2"

Ref #9498.
2021-12-29 17:31:53 +02:00
Avi Kivity
9e74556413 Merge 'Support reverse reads in the row cache natively' from Tomasz Grabiec
This change makes row cache support reverse reads natively so that reversing wrappers are not needed when reading from cache and thus the read can be executed efficiently, with similar cost as the forward-order read.

The database is serving reverse reads from cache by default after this. Before, it was bypassing cache by default after 703aed3277.

Refs: #1413

Tests:

  - unit [dev]
  - manual query with build/dev/scylla and cache tracing on

Closes #9454

* github.com:scylladb/scylla:
  tests: row_cache: Extend test_concurrent_reads_and_eviction to run reverse queries
  row_cache: partition_snapshot_row_cursor: Print more details about the current version vector
  row_cache: Improve trace-level logging
  config: Use cache for reversed reads by default
  config: Adjust reversed_reads_auto_bypass_cache description
  row_cache: Support reverse reads natively
  mvcc: partition_snapshot: Support slicing range tombstones in reverse
  test: flat_mutation_reader_assertions: Consume expected range tombstones before end_of_partition
  row_cache: Log produced range tombstones
  test: Make produces_range_tombstone() report ck_ranges
  tests: lib: random_mutation_generator: Extract make_random_range_tombstone()
  partition_snapshot_row_cursor: Support reverse iteration
  utils: immutable-collection: Make movable
  intrusive_btree: Make default-initialized iterator cast to false
2021-12-29 16:53:25 +02:00
Avi Kivity
4a323772c1 Merge 'Use the same page size limit in reverse queries as in forward reads' from Piotr Jastrzębski
The default for get_unlimited_query_max_result_size() is 100MB (adjustable through config), whereas query::result_memory_limiter::maximum_result_size is 1MB (hard coded, should be enough for everybody)

This limit is then used by the replica to decide when to break pages and, in case of reversed clustering order reads, when to fail the read when accumulated data crosses the threshold. The latter behavior stems from the fact that reversed reads had to accumulate all the data (read in forward order) before they can reverse it and return the result. Reverse reads thus need a higher limit so that they have a higher chance of succeeding.

Most readers are now supporting reading in reverse natively, and only reversing wrappers (make_reversing_reader()) inserted on top of ka/la sstable readers need to accumulate all the data. In other cases, we could break pages sooner. This should lead to better stability (less memory usage) and performance (lower page build latency, higher read concurrency due to less memory footprint).

Tests: unit(dev)

Closes #9815

* github.com:scylladb/scylla:
  storage_proxy: Send page_size in the read_command
  gms: add SEPARATE_PAGE_SIZE_AND_SAFETY_LIMIT feature
  result_memory_accounter: use new max_result_size::get_page_size in check_local_limit
  max_result_size: Add page_size field
2021-12-29 15:04:01 +02:00
Nadav Har'El
4374c73d82 Merge 'Fix bad lowres_clock::duration assumptions' from Avi Kivity
Some code assumes that lowres_clock::duration is milliseconds, but public
documentation never claimed that. Harden the code for a change in the
definition by removing the assumptions.

Closes #9850

* github.com:scylladb/scylla:
  loading_cache: fix mixup of std::chrono::milliseconds and lowres_clock::duration
  service: storage_proxy: fix lowres_clock::duration assumption
  service: misc_services: fix lowres_clock::duration assumption
  gossip: fix lowres_clock::duration assumption
2021-12-28 23:32:26 +02:00
Avi Kivity
d40722d598 loading_cache: fix mixup of std::chrono::milliseconds and lowres_clock::duration
lowres_clock uses the two types interchangably, although they are not
defined to be the same. Fix by using only lowres_clock::duration.
2021-12-28 21:19:08 +02:00
Avi Kivity
966bb3c8f0 service: storage_proxy: fix lowres_clock::duration assumption
calculate_delay() implicitly converts a lowres_clock::duration to
std::chrono::microseconds. This fails if lowres_clock::duration has
higher resolution than microseconds.

Fix by using an explicit conversion, which always works.
2021-12-28 21:17:14 +02:00