The default target (i.e. what gets executed under "ninja") excludes
sanitize and coverage modes (since they're useful in special cases
only), but the other multi-mode targets such as "ninja build" do not.
This means that two extra modes are built.
Make things consistent by always using default_modes (which default to
release,dev,debug). This can be overriden using the --mode switch
to configure.py.
Closes#8775
We have been seeing rare failures of the cql-pytest (translated from
Cassandra's unit tests) for testing TTL in secondary indexes:
cassandra_tests/validation/entities/secondary_index_test.py::testIndexOnRegularColumnInsertExpiringColumn
The problem is that the test writes an item with 1 second TTL, and then
sleeps *exactly* 1.0 seconds, and expects to see the item disappear
by that time. Which doesn't always happen:
The problem with that assumption stems from Scylla's TTL clock ("gc_clock")
being based on Seastar's lowres clock. lowres_clock only has a 10ms
"granularity": The time Scylla sees when deciding whether an item expires
may be up to 10ms in the past - the arbitrary point when the lowres timer
happened to last run. In rare overload cases, the inaccuracy may be even
grater than 10ms (if the timer got delayed by other things running).
So when Scylla is asked to expire an item in 1 second - we cannot be
sure it will be expired in exactly 1 second or less - the expiration
can be also around 10ms later.
So in this patch we change the test to sleep with more than enough
margin - 1.1 seconds (i.e., 100ms more than 1 second). By that time
we're sure the item must have expired.
Before this patch, I saw the test failing once every few hundred runs,
after this patch I ran if 2,000 times without a single failure.
Fixes#9008
Signed-off-by: Nadav Har'El <nyh@scylladb.com>
Message-Id: <20210712100655.953969-1-nyh@scylladb.com>
Today our docker image is based on Centos7 ,Since centos will be EOL in
2024 and no longer has stable release stream. let's move our docker image to be based on ubuntu 20.04
Based on the work done in https://github.com/scylladb/scylla/pull/8730,
let's build our docker image based on local packages using buildah
Closes#8849
When a table with compact storage has no regular column (only primary
key columns), an artificial column of type empty is added. Such column
type can't be returned via CQL so CDC Log shouldn't contain a column
that reflects this artificial column.
This patch does two things:
1. Make sure that CDC Log schema does not contain columns that reflect
the artificial column from a base table.
2. When composing mutation to CDC Log, ommit the artificial column.
Fixes#8410
Test: unit(dev)
Signed-off-by: Piotr Jastrzebski <piotr@scylladb.com>
Closes#8988
This series fixes some issues that gcc 11 complains about. I believe all
are correct errors from the standard's view. Clang accepts the changed code.
Note that this is not enough to build with gcc 11, but it's a start.
Closes#9007
* github.com:scylladb/scylla:
utils: compact-radix-tree: detemplate array_of<>
utils: compact-radix-tree: don't redefine type as member
raft: avoid changing meaning of a symbol inside a class
cql3: lists: catch polymorphic exceptions by reference
The radix tree template defines a nested class template array_of;
both a generic template and a fully specialized version. However,
gcc (I believe correctly) rejects the fully specialized template
that happens to be a member of another class template.
As it happens, we don't really need a template here at all. Define
a non-template class for each of the cases we need, and use
std::conditional_t to select the type we need.
The `direct_layout` and `indirect_layout` template classes accept
a template parameter named `Layout` of type `layout`, and re-export
`Layout` as a static data member named `layout`. This redefinition
of `layout` is disliked by gcc. Fix by renaming the static data member
to `this_layout` and adjust all references.
The construct
struct q {
a a;
};
Changes the meaning of `a` from a type to a data member. gcc dislikes
it and I agree. Fully qualify the type name to avoid an error.
Returning a function parameter guarantees copy elision and does not
require a std::move(). Enable -Wredundant-move to warn us that the
move is unneeded, and gain slightly more readable code. A few violations
are trivially adjusted.
Closes#9004
When creating an index-table query, we form its clustering-key
restrictions by picking the right restrictions from the WHERE clause.
But we skip the indexed column, which isn't in the index-table clutering
key. This is, however, both incorrect and unnecessary:
It is incorrect because we compare the column IDs from different schemas
(indexed table vs. base table). We should instead be comparing column
names.
It is unnecessary because this code is only executed when the whole
partition key plus a clustering prefix is specified in the WHERE clause.
In such cases, the index cannot possibly be on a member of the
clustering prefix, as such a query would be satisfied out of the base
table. Therefore, it is redundant to check for the indexed table among
the CK prefix elements.
A careful reader will note that this check was first introduced to fix
the issue #7888 in commit 0bd201d. But it now seems to me that that fix
was misguided. The root problem was the old code miscalculating the
clustering prefix by including too many columns in it; it should have
stopped before reaching the indexed column. The new code, introduced by
commit 845e36e76, calculates the clustering prefix correctly, never
reaching the indexed column.
(Details, for the curious: the old code invoked
clustering_key_restrictions::prefix_size(), which is buggy -- it
doesn't check the restriction operator. It will, for instance,
calculate the prefix of `c1=0 AND c2 CONTAINS 0 AND c3=0` as 3, because
it restricts c1, c2, and c3. But the correct prefix is clearly 1,
because c2 is not restricted by equality.)
Tests: unit (dev, debug)
Signed-off-by: Dejan Mircevski <dejan@scylladb.com>
Closes#8993
This warning prevents using std::move() where it can hurt
- on an unnamed temporary or a named automatic variable being
returned from a function. In both cases the value could be
constructed directly in its final destination, but std::move()
prevents it.
Fix the handful of cases (all trivial), and enable the warning.
Closes#8992
"
After this series one can use perf_fast_forward to generate the data set.
It takes a lot less time this way than to use scylla-bench.
"
* 'perf-fast-forward-scylla-bench-dataset' of github.com:tgrabiec/scylla:
tests: perf_fast_forward: Use data_source::make_ck()
tests: perf_fast_forward: Move declaration of clustered_ds up
tests: perf_fast_forward: Make scylla_bench_small_part_ds1 not included by default
tests: perf_fast_forward: Add data sets which conform to scylla-bench schema
This patch adds to cql-pytest/README.md a paragraph on where run /
run-cassandra expect to find Scylla or Cassandra, and how to override
that choice.
Also make a couple of trivial formatting changes.
Signed-off-by: Nadav Har'El <nyh@scylladb.com>
Message-Id: <20210708142730.813660-1-nyh@scylladb.com>
"
When obtaining a valid permit was made mandatory, code which now had to
create reader permits but didn't have a semaphore handy suddenly found
itself in a difficult situation. Many places and most prominently tests
solved the problem by creating a thread-local semaphore to source
permits from. This was fine at the time but as usual, globals came back
to haunt us when `reader_concurrency_semaphore::stop()` was
introduced, as these global semaphores had no easy way to be stopped
before being destroyed. This patch-set cleans up this wart, by getting
rid of all global semaphores, replacing them with appropriately scoped
local semaphores, that are stopped after being used. With that, the
FIXME in `~reader_concurrency_semaphore()` can be resolved and we an
finally `assert()` that the semaphore was stopped before being
destroyed.
This series is another preparatory one for the series which moves the
semaphore in front of the cache.
tests: unit(dev)
"
* 'reader-concurrency-semaphore-mandatory-stop/v2' of https://github.com/denesb/scylla: (26 commits)
reader_concurrency_semaphore: assert(_stopped) in the destructor
test/lib: remove now unused reader_permit.{hh,cc}
test/boost: migrate off the global test reader semaphore
test/manual: migrate off the global test reader semaphore
test/unit: migrate off the global test reader semaphore
test/perf: migrate off the global test reader semaphore
test/perf: perf.hh: add reader_concurrency_semaphore_wrapper
test/lib: migrate off the global test reader semaphore
test/lib/simple_schema: migrate off the global test reader semaphore
test/lib/sstable_utils: migrate off the global test reader semaphore
test/lib/test_services: migrate off the global test reader semaphore
test/lib/sstable_test_env: add reader_concurrency_semaphore member
test/lib/cql_test_env: add make_reader_permit()
test/lib: add reader_concurrency_semaphore.hh
test/boost/sstable_test: migrate row counting tests to seastar thread
test/boost/sstable_test: test_using_reusable_sst(): pass env to func
test/lib/reader_lifecycle_policy: add permit parameter to factory function
test/boost/mutation_reader_test: share permit between readers in a read
memtable: migrate off the global reader concurrency semaphore
mutation_writer: multishard_writer: migrate off the global reader concurrency semaphore
...
Now that there are no more global semaphore which are impossible to stop
properly we can resolve the related FIXME and arm the assert in the
semaphore destructor.
We can also remove all the other cleanup code from the destructor as
they are taken care of by stop(), which we now assert to have been run.
After commit 76227fa ("cql-pytest: use NetworkTopologyStrategy, not
SimpleStrategy"), the cql-pytest tests now NetworkTopologyStrategy instead
of SimpleStrategy in the test keyspaces. The tests continued to use the
"replication_factor" option. The support for this option is a relatively
recent, and was only added to Cassandra in the 4.0 release series
(see https://issues.apache.org/jira/browse/CASSANDRA-14303). So users
who happen to have Cassandra 3 installed and want to run a cql-pytest
against it will see the test failing when it can't create a keyspace.
This patch trivially fixes the problem by using the name of the current
DC (automatically determined) instead of the word 'replication_factor'.
Almost all tests are fixed by a single fix to the test_keyspace fixture
which creates one keyspace used by most tests. Additional changes were
needed in test_keyspace.py, for tests which explicitly create keyspaces.
I tested the result on Cassandra 3.11.10, Cassandra 4 (git master) and
Scylla.
Fixes#8990
Signed-off-by: Nadav Har'El <nyh@scylladb.com>
Message-Id: <20210708123428.811184-1-nyh@scylladb.com>
We know that today in Scylla concurrent schema changes done on different
coordinators are not safe - and we plan to address this problem with Raft.
However, the test in this patch - reproducing issue #8968 - demonstrates
that even on a single node concurrent schema changes are not safe:
The test involves one thread which constantly creates a keyspace and
then a table in it - and a second thread which constantly deletes this
keyspace. After doing this for a while, the schema reaches an inconsistent
state: The keyspace is at a state of limbo where it cannot be dropped
(dropping it succeeds, but doesn't actually drop it), and a new keyspace
cannot be created under the same name).
Note that to reproduce this bug, it was important that the test create
both a keyspace and a table. Were the test to just create an empty keyspace,
without a table in it, the bug would not be reproduced.
Refs #8968.
Signed-off-by: Nadav Har'El <nyh@scylladb.com>
Message-Id: <20210704121049.662169-1-nyh@scylladb.com>
Supplying a convenience semaphore wrapper, which stops the contained
semaphore when destroyed. It also provides a more convenient
`make_permit()`. This class is intended to make the migration to local
semaphores less painful.
semaphore_timed_out errors should be ignored, similar to
rpc::timeout_error or seastar::timed_out_error, so that they
eventually be converted to `read_timeout_exception` via
the data/digest read resolver on_timeout() method.
Otherwise, the semaphore timeout is mistranslated to
read_failure_exception, via on_error().
Note that originally the intention was to change the exception
thrown by the reader_concurrency_semaphore expiry_handler, but
there are already several places in the code that catch and handle
the semaphore_timed_out exception that would need to be changed,
increasing the risk in this change.
Fixes#8958
Test: unit(dev)
Signed-off-by: Benny Halevy <bhalevy@scylladb.com>
Message-Id: <20210708083252.1934651-2-bhalevy@scylladb.com>
In older versions of Cassandra (such as 3.11.10 which I tried), the
CQL server is not turned on by default, unless the configuration file
explicitly has "start_native_transport: true" - without it only the
Thrift server is started.
So fix the cql-pytest/run-cassandra to pass this option. It also
works correctly in Cassandra 4.
Signed-off-by: Nadav Har'El <nyh@scylladb.com>
Message-Id: <20210708113423.804980-1-nyh@scylladb.com>
In order to avoid large allocations and too large mutations
generated from large view updates, granularity of the process
is broken down from per-partition to smaller chunks.
The view update builder now produces partial updates, no more
than 100 view rows at a time.
The series was tested manually with a particular scenario in mind -
deleting a large base partition, which results in creating
a view update per each deleted row - which, with sufficiently
large partitions, can reach millions. Before the series, Scylla
experienced an out-of-memory condition after the view update
generation mechanism tried to load too much data into a contiguous
buffer. Multiple large allocation warnings and reactor stalls were
observed as well. After the series, the operation is still rather slow,
but does not induce reactor stalls nor allocator problems.
A reduced version of the above test is added as a unit test -
it does not check for huge partitions, but instead uses a number
just large enough to cause the update generation process to be split
into multiple chunks.
Fixes#8852Closes#8906
* github.com:scylladb/scylla:
cql-pytest: add a test case for base range deletion
cql-pytest: add a test case for base partition deletion
table: elaborate on why exceptions are ignored for view updates
view: generate view updates in smaller parts
table: coroutinize generating view updates
db,view: move view_update_builder to the header
The test case checks that deleting a base table clustering range
works fine. This operation is potentially heavy, as it involves
generating a view update for every row. With large enough ranges,
the number can reach millions and beyond.
The test case checks that deleting a whole base table partition
works fine. This operation is potentially heavy, as it involves
generating a view update for every row. With large enough partitions,
the number can reach millions and beyond.
The factory method doesn't match the signature of
`reader_lifecycle_policy::make_reader()`, notably the permit is missing.
Add it as it is important that the wrapping evictable reader and
underlying reader share the permits.
Permits were designed such that there is one permit per read, being
shared by all readers in that read. Make sure readers created by tests
adhere to this.
