Commit Graph

3242 Commits

Author SHA1 Message Date
Avi Kivity
77686ab889 Merge "Make SSTable cleanup run aware" from Raphael
"
Fixes #4663.
Fixes #4718.
"

* 'make_cleanup_run_aware_v3' of https://github.com/raphaelsc/scylla:
  tests/sstable_datafile_test: Check cleaned sstable is generated with expected run id
  table: Make SSTable cleanup run aware
  compaction: introduce constants for compaction descriptor
  compaction: Make it possible to config the identifier of the output sstable run
  table: do not rely on undefined behavior in cleanup_sstables
2019-07-31 19:10:22 +03:00
Avi Kivity
74349bdf7e Merge "Partially devirtualize CQL restrictions" from Piotr
"
This series is a batch of first small steps towards devirtualizing CQL restrictions:
 - one artificial parent class in the hierarchy is removed: abstract_restriction
 - the following functions are devirtualized:
    * is_EQ()
    * is_IN()
    * is_slice()
    * is_contains()
    * is_LIKE()
    * is_on_token()
    * is_multi_column()

Future steps can involve the following:
 - introducing a std::variant of restriction targets: it's either a column
   or a vector of columns
 - introducing a std::variant of restriction values: it's one of:
   {term, term_slice, std::vector<term>, abstract_marker}

The steps above will allow devirtualizing most of the remaining virtual functions
in favor of std::visit. They will also reduce the number of subclasses,
e.g. what's currently `token_restriction::IN_with_values` can be just an instance
of `restriction`, knowing that it's on a token, having a target of std::vector<column>
and a value of std::vector<term>.

Tests: unit(dev), dtest: cql_tests, cql_additional_tests
"

* 'refactor_restrictions_2' of https://github.com/psarna/scylla:
  cql3: devirtualize is_on_token()
  cql3: devirtualize is_multi_column()
  cql3: devirtualize is_EQ, is_IN, is_contains, is_slice, is_LIKE
  tests: add enum_set adding case
  cql3: allow adding enum_sets
  cql3: remove abstract_restriction class
2019-07-31 11:44:57 +03:00
Botond Dénes
cea3338e38 tests/mutation_source_tests: generate_mutation_sets() use larger ttl
Currently all cells generated by this method uses a ttl of 1. This
causes test flakyness as tests often compact the input and output
mutations to weed out artificial differences between them. If this
compaction is not done with the exact same query time, then some cells
will be expired in one compaction but not in the other.
733c68cb1 attempted to solve this by passing the same query time to
`flat_mutation_reader_assertions::produce_compacted()` as well as
`mutation_partition::compact_for_query()` when compacting the input
mutation. However a hidden compaction spot remained: the ka/la sstable
writer also does some compaction, and for this it uses the time point
passed to the `sstable` constructor, which defaults to
`gc_clock::now()`. This leads to false positive failures in
`sstable_mutation_test.cc`.
At this point I don't know what the original intent was behind this low
`ttl` value. To solve the immediate problem of the tests failing, I
increased it. If it turns out that this `ttl` value has a good reason,
we can do a more involved fix, of making sure all sstables written also
get the same query time as that used for the compaction.

Fixes: #4747

Signed-off-by: Botond Dénes <bdenes@scylladb.com>
Message-Id: <20190731081522.22915-1-bdenes@scylladb.com>
2019-07-31 11:44:57 +03:00
Piotr Sarna
e9798354ae tests: add enum_set adding case 2019-07-29 17:15:51 +02:00
Botond Dénes
733c68cb13 tests: flat_reader_assertions::produces_compacted(): add query_time param
`produces_compacted()` is usually used in tandem of another
compaction done on the expected output (`m` param). This is usually done
so that even though the reader works with an uncompacted stream, when
checking the checking of the result will not fail due to insignificant
changes to the data, e.g. expired collection cells dropped while merging
two collections. Currently, the two compactions, the one inside
`produce_compacted()` and the one done by the caller uses two separate
calls to `gc_clock::now()` to obtain the query time. This can lead to
off-by-one errors in the two query times and subsequently artificial
differences between the two compacted mutations, ultimately failing the
test due to a false-positive.
To prevent this allow callers to pass in a query time, the same they
used to compact the input mutation (`m`).

This solves another source of flakyness in unit tests using the mutation
source test suite.

Refs: #4695
Fixes: #4747
Signed-off-by: Botond Dénes <bdenes@scylladb.com>
Message-Id: <20190726144032.3411-1-bdenes@scylladb.com>
2019-07-28 10:59:50 +03:00
Botond Dénes
f215286525 tests/mutation_reader_tests: move away from variadic futures
Signed-off-by: Botond Dénes <bdenes@scylladb.com>
Message-Id: <20190724101005.19126-1-bdenes@scylladb.com>
2019-07-27 13:21:24 +03:00
Botond Dénes
6dd8c4da83 test_multishard_combining_reader_non_strictly_monotonic_positions: use the same deletion_time for tombstones
Across all calls to `make_fragments_with_non_monotonic_positions()`, to
prevent off-by one errors between the separately generated test input
and expected output. This problem was already supposed to be fixed by
5f22771ea8 but for some reason that only
used the same deletion time *inside* a single call, which will still
fall short in some cases.
This should hopefully fix this problem for good.

Refs: #4695
Signed-off-by: Botond Dénes <bdenes@scylladb.com>
Message-Id: <20190724073240.125975-1-bdenes@scylladb.com>
2019-07-25 12:37:34 +02:00
Tomasz Grabiec
3af8431a40 Merge "compaction: allow collecting purged data" from Botond
compaction: allow collecting purged data

Allow the compaction initiator to pass an additional consumer that will
consume any data that is purged during the compaction process. This
allows the separate retention of these dead cells and tombstone until
some long-running process like compaction safely finishes. If the
process fails or is interrupted the purged data can be used to prevent
data resurrection.

This patch was developed to serve as the basis for a solution to #4531
but it is not a complete solution in and on itself.

This series is a continuation of the patch: "[PATCH v1 1/3] Introduce
Garbage Collected Consumer to Mutation Compactor" by Raphael S.
Carvalho <raphaelsc@scylladb.com>.

Refs: #4531

* https://github.com/denesb/scylla.git compaction_collect_purged_data/v8:
  Introduce compaction_garbage_collector interface
  collection_type_impl::mutation: compact_and_expire() add collector
    parameter
  row: add garbage_collector
  row_marker: de-inline compact_and_expire()
  row_marker: add garbage_collector
  Introduce Garbage Collected Consumer to Mutation Compactor
  tests: mutation_writer_test.cc/generate_mutations() ->
    random_schema.hh/generate_random_mutations()
  tests/random_schema: generate_random_mutations(): remove `engine`
    parameter
  tests/random_schema: add assert to make_clustering_key()
  tests/random_schema: generate_random_mutations(): allow customizing
    generated data
  tests: random_schema: futurize generate_random_mutations()
  random_schema: generate_random_mutations(): restore indentation
  data_model: extend ttl and expiry support
  tests/random_schema: generate_random_mutations(): generate partition
    tombstone
  random_schema: add ttl and expiry support
  tests/random: add get_bool() overload with random engine param
  random_schema: generate_random_mutations(): ensure partitions are
    unique
  tests: add unit tests for the data stream split in compaction
2019-07-23 17:12:28 +02:00
Avi Kivity
d730969278 Merge "make sure failure to create snapshots won't crash the node" from Glauber
"
Issue #4558 describes a situation in which failure to execute clearsnapshots
will hard crash the node. The problem is that clearsnapshots will internally use
lister::rmdir, which in turn has two in-tree users: clearing snapshots and clearing
temporary directories during sstable creation. The way it is currently
coded, it wraps the io functions in io_check, which means that failures
to remove the directory will crash the database.

We recently saw how benign failures crashed a database during
clearsnapshot: we had snapshot creation running in parallel, adding more
files to the directory that wasn't empty by the time of deletion. I
have also seen very often users add files to existing directories by
accident, which is another possibility to trigger that.

This patch removes the io_check from lister, and moves it to the caller
in which we want to be more strict. We still want to be strict about
the creation of temporary directories, since users shouldn't be touching
that in any way.

Also while working on that, I realized we have no tests for snapshots of
any kind in tree, so let's write some
"

* 'snapshots' of https://github.com/glommer/scylla:
  tests: add tests for snapshots.
  lister: don't crash the node on failure to remove snapshot
2019-07-22 11:09:23 +03:00
Vlad Zolotarov
5632c0776e tests: fix the compilation with fmt v5.3.0
Compilation fails with fmt release 5.3.0 when we print a bytes_view
using "{}" formatter.

Compiler's complain is: "error: static assertion failed: mismatch
between char-types of context and argument"

Fix this by explicitly using to_hex() converter.

Signed-off-by: Vlad Zolotarov <vladz@scylladb.com>
Message-Id: <20190716221231.22605-3-vladz@scylladb.com>
2019-07-21 16:42:54 +03:00
Piotr Sarna
4d1eaf8478 tests: add checking computed columns in SI
The test case checks if token column generated for global indexing
is indeed only present in global indexes and is marked as a computed
column.
2019-07-19 11:58:42 +02:00
Piotr Sarna
1c0ef5f9e9 tests: generate new test case for schema change + computed cols
The original "test_schema_digest_does_not_change" test case ensures
that schema digests will match for older nodes that do not support
all the features yet (including computed columns).
The additional case uses sstables generated after computed columns
are allowed, in order to make sure that the digest computed
including computed columns does not change spuriously as well.
2019-07-19 11:58:42 +02:00
Piotr Sarna
589200f5a2 tests: add schema change test regeneration comment
Schema change test might need regenerating every time a system table
is added. In order to save future developer's time on debugging this
test, a short description of that requirement is added.
2019-07-19 11:58:42 +02:00
Piotr Sarna
03ade01db7 tests: disable computed columns feature in schema change test
In order to make sure that old schema digest is not recomputed
and can be verified - computed columns feature is initially disabled
in schema_change_test.
The reason for that is as follows: running CQL test env assumes that
we are running the newest cluster with all features enabled. However,
the mere existence of some features might influence digest calculation.
So, in order for the existing test to work correctly, it should have
exactly the same set of cluster supported features as it had during
its creation. It used to be "all features", but now it's "all features
except computed columns". One can think of that as running a cluster
with some nodes not yet knowing what computed columns are, so they
are not taken into account when computing digests.
Additionally, a separate test case that takes computed column digest
into account will be generated and added in this series.
2019-07-19 11:58:42 +02:00
Avi Kivity
51cff8ad23 Merge "Fix storage service for tests" from Botond
"
Fix another source of flakyness in mutation_reader_test. This one is caused by storage_service_for_tests lacking a config::broadcast_to_all_shards() call, triggering an invalid memory access (or SEGFAULT) when run on more than one shards.

Refs: #4695
"

* 'fix_storage_service_for_tests' of https://github.com/denesb/scylla:
  tests: storage_service_for_tests: broadcast config to all shards
  tests: move storage_service_for_tests impl to test_services.cc
2019-07-18 18:27:47 +03:00
Glauber Costa
be26cbd952 tests: add tests for snapshots.
While inspecting the snapshot code, I realized that we don't have any
tests for it. So I decided to add some.

Unfortunately I couldn't come up with a test of clearsnapshot reliably
failing to remove the directory: relying on create snapshot +
clearsnapshot is racy (not always happen), and other tricks that can be
used to reproduce this -- like creating a root-owned file inside the
snapshots directory -- is environment-dependent, and a bit ugly for unit
tests. Dtests would probably be a better place for that.

Signed-off-by: Glauber Costa <glauber@scylladb.com>
2019-07-16 13:35:53 -04:00
Kamil Braun
4417e78125 Fix timestamp_type_impl::timestamp_from_string.
Now it accepts the 'z' or 'Z' timezone, denoting UTC+00:00.
Fixes #4641.

Signed-off-by: Kamil Braun <kbraun@scylladb.com>
2019-07-16 19:16:56 +03:00
Botond Dénes
b40cf1c43d tests: storage_service_for_tests: broadcast config to all shards
Due to recent changes to the config subsystem, configuration has to be
broadcast to all shards if one wishes to use it on them. The
`storage_service_for_tests` has a `sharded<gms::gossiper>` member, which
reads config values on initialization on each shard, causing a crash as
the configuration was initialized only on shard 0. Add a call to
`config::broadcast_to_all_shards()` to ensure all shards have access to
valid config values.
2019-07-16 10:37:17 +03:00
Botond Dénes
fc9f46d7c1 tests: move storage_service_for_tests impl to test_services.cc
Let's make it easier to find.
2019-07-16 10:36:49 +03:00
Raphael S. Carvalho
7180731d43 tests/sstable_datafile_test: Check cleaned sstable is generated with expected run id
Signed-off-by: Raphael S. Carvalho <raphaelsc@scylladb.com>
2019-07-15 23:39:50 -03:00
Botond Dénes
5f22771ea8 tests/mutation_reader_test stabilize test_multishard_combining_reader_non_strictly_monotonic_positions
Currently the
test_multishard_combining_reader_non_strictly_monotonic_positions is
flaky. The test is somewhat unconventional, in that it doesn't use the
same instance of data as the input to the test and as it's expected
output, instead it invokes the method which generates this data
(`make_fragments_with_non_monotonic_positions()`) twice, first to
generate the input, and a secondly to generate the expected output. This
means that the test is prone to any deviation in the data generated by
said method. One such deviation, discovered recently, is that the method
doesn't explicitly specify the deletion time of the generated range
tombstones. This results in this deletion time sometimes differing
between the test input and the expected output. Solve by explicitly
passing the same deletion time to all created range tombstones.

Refs: #4695
2019-07-15 23:24:16 +02:00
Tomasz Grabiec
14700c2ac4 Merge "Fix the system.size_estimates table" from Kamil
Fixes a segfault when querying for an empty keyspace.

Also, fixes an infinite loop on smp > 1. Queries to
system.size_estimates table which are not single-partition queries
caused Scylla to go into an infinite loop inside
multishard_combining_reader::fill_buffer. This happened because
multishard_combinind_reader assumes that shards return rows belonging
to separate partitions, which was not the case for
size_estimates_mutation_reader.

Fixes #4689.
2019-07-15 22:09:30 +02:00
Botond Dénes
135c84c29a tests: add unit tests for the data stream split in compaction 2019-07-15 17:38:00 +03:00
Botond Dénes
719ad51bea random_schema: generate_random_mutations(): ensure partitions are unique
Duplicate partitions can appear as a result of the same partition key
generated more than once. For now we simply remove any duplicates. This
means that in some circumstances there will be less partitions generated
than asked.
2019-07-15 17:38:00 +03:00
Botond Dénes
eaedbed069 tests/random: add get_bool() overload with random engine param 2019-07-15 17:38:00 +03:00
Botond Dénes
057f9aa655 random_schema: add ttl and expiry support
When generating data, the user can now also generate ttls and
expiry for all generated atoms. This happens in a controlled way, via a
generator functor, very similar to how the timestamps are generated.
This functor is also used by `random_schema` to generate `deletion_time`
for all tombstones, so the user now has full control of when all of the
atoms can be GC'd.
2019-07-15 17:38:00 +03:00
Botond Dénes
76a853e345 tests/random_schema: generate_random_mutations(): generate partition tombstone 2019-07-15 17:38:00 +03:00
Botond Dénes
4d9f3e5705 data_model: extend ttl and expiry support 2019-07-15 17:38:00 +03:00
Botond Dénes
96d3c1efb1 random_schema: generate_random_mutations(): restore indentation 2019-07-15 17:38:00 +03:00
Botond Dénes
b26fe76fc1 tests: random_schema: futurize generate_random_mutations()
To avoid reactor stalls when generate many and/or large partitions.
2019-07-15 17:38:00 +03:00
Botond Dénes
cf135c6257 tests/random_schema: generate_random_mutations(): allow customizing generated data
Allow callers to specify the number of partitions generated, as well as
the number of clustering rows and range tombstones generated per
partition.
2019-07-15 17:38:00 +03:00
Botond Dénes
d2930ffa53 tests/random_schema: add assert to make_clustering_key()
Verify that the schema *does* indeed have clustering columns. Better an
assert than a cryptic "division by 0" exception deeper in the call stack.
2019-07-15 17:38:00 +03:00
Botond Dénes
d90ac6bd7b tests/random_schema: generate_random_mutations(): remove engine parameter
Use an internally create instance of random engine. Passing a readily
seeded engine from the outside is pointless now that we have a mechanism
to seed entire test suites with a command line algorithm: the internal
engine is seeded from tests::random, so the seed of the test suite
determines the internal seed as well.

Update the sole user of this method (mutation_writer_test.cc) to not
generate local seeds anymore.
2019-07-15 17:38:00 +03:00
Botond Dénes
fd2f53f292 tests: mutation_writer_test.cc/generate_mutations() -> random_schema.hh/generate_random_mutations()
We plan on allowing other tests to use this method. The first step is to
make it available in a header.
2019-07-15 17:38:00 +03:00
Botond Dénes
307b48794d collection_type_impl::mutation: compact_and_expire() add collector parameter
The new collector parameter is a pointer to a
`compaction_garbage_collector` implementation. This collector is passed
all atoms that are expired and would be discarded. The body of
`compact_and_expire()` was changed so that it checks cells' tombstone
coverage before it checks their expiry, so that cells that are both
covered by a tombstone and also expired are not passed to the collector.
The collector param is optional and defaults to nullptr. To accommodate
the collector, which needs to know the column id, a new `column_id`
parameter was added as well.
2019-07-15 17:37:55 +03:00
Rafael Ávila de Espíndola
4e7ffb80c0 cql: Fix use of UDT in reversed columns
We were missing calls to underlying_type in a few locations and so the
insert would think the given literal was invalid and the select would
refuse to fetch a UDT field.

Fixes #4672

Signed-off-by: Rafael Ávila de Espíndola <espindola@scylladb.com>
Message-Id: <20190708200516.59841-1-espindola@scylladb.com>
2019-07-12 19:21:26 +03:00
Kamil Braun
60a4867a5b Fix infinite looping when performing a range query on system.size_estimates.
Queries to system.size_estimates table which are not single parition queries
caused Scylla to go into an infinite loop inside multishard_combining_reader::fill_buffer.
This happened because multishard_combinind_reader assumes that shards return rows belonging
to separate partitions, which was not the case for size_estimates_mutation_reader.
This commit fixes the issue and closes #4689.
2019-07-12 18:09:15 +02:00
Kamil Braun
ba5a02169e Fix segmentation fault when querying system.size_estimates for an empty keyspace. 2019-07-12 18:02:10 +02:00
Kamil Braun
a1665b74a9 Refactor size_estimates_virtual_reader
Move the implementation of size_estimates_mutation_reader
to a separate compilation unit to speed up compilation times
and increase readability.

Refactor tests to use seastar::thread.
2019-07-12 17:53:00 +02:00
Avi Kivity
405fd517b0 Merge "IPv6 support" from Calle
"
Fixes #2027

Modifies inet address type in scylla to use seastar::net::inet_address,
and removes explicit use of ipv4_addr in various network code in favour
of socket_address. Thus capable of resolving and binding to ipv6.

Adds config option to enable/disable ipv6 (default enabled), so
upgrading cluster can continue to work while running mixed version
nodes (since gossip message address serialization becomes different).
"

* 'calle/ipv6' of https://github.com/elcallio/scylla:
  test-serialization: Add small roundtrip test for inet address (v4 + v6)
  inet_address/init: Make ipv6 default enabled
  db::config: Add enable ipv6 switch (default off)
  gms::inet_address: Make serialization ipv6 aware
  Remove usage of inet_address::raw_addr()
  Replace use of "ipv4_addr" with socket_address
  inet_address: Add optional family to lookup
  gms::inet_address: Change inet_address to wrap actual seastar::net::inet_address
  types: Add ipv6_address support
2019-07-10 15:07:56 +03:00
Benny Halevy
b4dc118639 tests: logalloc_test: scale down test_region_groups
Post commit b3adabda2d
(Reduce logalloc differences between debug and release)
logalloc_test's memory footprint has grown, in particular
in test_region_groups, and it triggers the oom killer on
our test automation machines.

This patch scales down this test case so it requires less memory.

Fixes #4669

Signed-off-by: Benny Halevy <bhalevy@scylladb.com>
2019-07-10 12:06:10 +02:00
Botond Dénes
ce647fac9f timestamp_based_splitting_writer: fix the handling of partition tombstone
Currently the handling of partition tombstones is broken in multiple
ways:
* The partition-tombstone is lost when the bucket is calculated for its
timestamp (due to a misplaced `std::exchange()`).
* When the `partition_start` fragment (containing the partition
tombstone) is actually written to the bucket we emit another
`partition_start` fragment before it because the bucket has not seen
that partition before and we fail to notice that we are actually writing
the partition header.

This bug was allowed to fly under the radar because the unit test was
accidentally not creating partition tombstones in the generated data
(due to a mistake). It was discovered while working on unit tests for
another test and fixing the data generation function to actually
generate partition tombstones.

This patch fixes both problems in the handling of partition tombstones
but it doesn't yet fixes the test. That is deferred until the patch
series which uncovered this bug is merged to avoid merge conflicts.
The other series mentioned here is: [PATCH v6 00/15] compaction: allow
collecting purged data

Fixes: #4683

Signed-off-by: Botond Dénes <bdenes@scylladb.com>
Message-Id: <20190710092427.122623-1-bdenes@scylladb.com>
2019-07-10 12:36:57 +03:00
Piotr Sarna
ebbe038d19 test: add 'eventually' block to index paging test
Without 'eventually', the test is flaky because the index can still
be not up to date while checking its conditions.

Fixes #4670
2019-07-09 17:07:16 +02:00
Calle Wilund
5dfc356380 test-serialization: Add small roundtrip test for inet address (v4 + v6)
Verify we get back what we put in.
2019-07-08 15:28:21 +00:00
Avi Kivity
63edd46562 Merge "Expand big decimal with arithmetic operators" from Piotr
"
This miniseries expands big_decimal interface with convenience operators
(-=, +, -), provides test cases for it and makes one of the constructors
explicit.

Tests: unit(dev)
"

* 'expand_big_decimal_interface' of https://github.com/psarna/scylla:
  utils: make string-based big decimal constructor explicit
  tests: add more operators to big decimal tests
  utils: add operators to big_decimal
2019-07-06 12:26:08 +03:00
Avi Kivity
24caf0824d Merge "Complete the LIKE operator" from Dejan
"
Implement LIKE parsing, intermediate representation, and query processing. Add tests
for this implementation (leaving the LIKE functionality tests in
tests/like_matcher_test.cc).

Refs #4477.
"

* 'finish-like' of https://github.com/dekimir/scylla:
  cql3: Add LIKE operator to CQL grammar
  cql3: Ensure LIKE filtering for partition columns
  cql3: Add LIKE restriction
  cql3: Add LIKE relation
2019-07-06 12:26:08 +03:00
kbr-
8995945052 Implement tuple_type_impl::to_string_impl. (#4645)
Resolves #4633.

Signed-off-by: Kamil Braun <kbraun@scylladb.com>
2019-07-06 12:26:08 +03:00
Piotr Sarna
7e722f8dd5 tests: add more operators to big decimal tests 2019-07-04 11:32:57 +02:00
Dejan Mircevski
6727e8f073 cql3: Add LIKE operator to CQL grammar
Extend the grammar with LIKE and add CQL query tests for it.

Signed-off-by: Dejan Mircevski <dejan@scylladb.com>
2019-07-04 11:01:13 +02:00
Tomasz Grabiec
eb496b5eae Merge "Allow changing configuration at runtime" from Avi
This patchset allows changing the configuration at runtime, The user
triggers this by editing the configuration file normally, then
signalling the database with SIGHUP (as is traditional).

The implementation is somewhat complicated due the need to store
non-atomic mutable state per-shard and to synchronize the values in
all shards. This is somewhat similar to Seastar's sharded<>, but that
cannot be used since the configuration is read before Seastar is
initialized (due to the need to read command-line options).

Tests: unit (dev, debug), manual test with extra prints (dev)

Ref #2689
Fixes #2517.
2019-07-01 15:04:59 +02:00