`move_buffer_content_to()` makes it possible to implement more efficient
wrapping readers, readers that wrap another flat mutation reader but do
no transformation to the underlying fragment stream.
These readers, when filling their buffers, can simply fill the
underlying reader's buffer, then move its content into their own. When
the reader's own buffer is empty, this is very efficient, as it can be
done by simply swapping the buffers, avoiding the work of moving the
fragments one-by-one.
This patchset addresses multiple errors in normalizing_reader
implementation found during review.
I have decided to not make a clustering key full inside
before_key()/after_key() helpers. The reason is that for this they
would need schema to be passed as another parameter so existing
methods don't suit. OTOH, introducing new members for a class using
for testing purposes only seems an overkill.
* github.com/argenet/scylla.git projects/sstables-30/normalizing_reader_fixes/v1:
range_tombstone: Add constructor accepting position_in_partition_views
for range bounds.
tests: Make sure range tombstone is properly split over rows with
non-full keys.
tests: Multiple fixes for draining and clearing range tombstones in
normalizing_reader.
We only wait from the last test case, so if an individual test is executed,
a memory leak may be reported.
Fix by waiting from all test cases.
Message-Id: <20180926203723.18026-1-avi@scylladb.com>
When executing a prepared select statement with a multicolumn IN, the
system returned incorrect results due to a memory violation (a bytes view
referring to an out of scope bytes object).
Added test for the prepared statement results correctness.
Tests:
1. unit (release) with the new test.
2. Python script.
Signed-off-by: Eliran Sinvani <eliransin@scylladb.com>
Message-Id: <36c9cf9ed3fe72e3b4801e3cd120678429ce218a.1537947897.git.eliransin@scylladb.com>
"
This patchset fixes several issues in SSTables 3.x ('mc') writing and
parsing and extends existing SSTables unit tests to cover the new
format.
The only test enabled temporarily is check_multi_schema because it
turned out that reading SSTables 3.x with a different schema has not
been implemented in full. This will be addressed in a separate patchset.
This patchset depends on the "Support SSTables 3.x in Scylla runtime"
patchset.
Tests: unit {release}
"
* 'projects/sstables-30/unit-tests/v3' of https://github.com/argenet/scylla: (25 commits)
tests: Enable existing SSTables tests for 'mc' format.
tests: Fix test_wrong_range_tombstone_order for 'mc' format.
tests: Extend reader assertions to check clustering keys made full.
tests: Disable test_old_format_non_compound_range_tombstone_is_read for 'mc' format.
tests: Disable check_multi_schema for 'mc' format.
tests: Fix test_promoted_index_read for 'mc' format by using normalizing_reader.
tests: Fix promoted_index_read to not rely on a specific index length
tests: Add 'mc' files for test_wrong_range_tombstone_order
tests: Add 'mc' files for test_wrong_counter_shard_order
tests: Add 'mc' files for summary_test
tests: Add 'mc' files for test_promoted_index_read
tests: Add 'mc' files for test_partition_skipping
tests: Add 'mc' files for large_partition tests (promoted_index_read, sub_partition_read, sub_partitions_read
tests: Add 'mc' files for test_counter_read
tests: Add 'mc' files for test_broken_promoted_index_is_skipped
tests: SSTables 'mc' files for sliced_mutation_reads_test.
tests: Introduce normalizing_reader helper for SSTables tests.
mutation_fragment: Add range_tombstone_stream::empty() method.
sstables: Make key full when setting a range tombstone start from end open marker.
sstables: For 'mc' format, use excl_start when split an RT over a row with a full key.
...
"
This patchset makes it possible to use SSTables 'mc' format, commonly
referred to as 'SSTables 3.x', when running Scylla instance.
Several bugs found on this way are fixed. Also, a configuration option
is introduced to allow running Scylla either with 'mc' or 'la' format
as default.
Tests: unit {release}
+ tested Scylla with both 'la' and 'mc' formats to work fine:
cqlsh> CREATE KEYSPACE test WITH replication = {'class': 'SimpleStrategy', 'replication_factor': 1}; [3/1890]
cqlsh> USE test;
cqlsh:test> CREATE TABLE cfsst3 (pk int, ck int, rc int, PRIMARY KEY (pk, ck)) WITH compression = {'sstable_compression': ''};
cqlsh:test> INSERT INTO cfsst3 (pk, ck, rc) VALUES ( 4, 7, 8);
<<flush>>
cqlsh:test> DELETE from cfsst3 WHERE pk = 4 and ck> 3 and ck < 8;
<<flush>>
cqlsh:test> INSERT INTO cfsst3 (pk, ck) VALUES ( 2, 3);
cqlsh:test> INSERT INTO cfsst3 (pk, ck) VALUES ( 4, 6);
cqlsh:test> SELECT * FROM cfsst3 ;
pk | ck | rc
----+----+------
2 | 3 | null
4 | 6 | null
(2 rows)
<<Scylla restart>>
cqlsh:test> INSERT INTO cfsst3 (pk, ck) VALUES ( 5, 7);
cqlsh:test> INSERT INTO cfsst3 (pk, ck) VALUES ( 6, 8);
cqlsh:test> INSERT INTO cfsst3 (pk, ck) VALUES ( 7, 9);
cqlsh:test> INSERT INTO cfsst3 (pk, ck) VALUES ( 8, 10);
cqlsh:test> SELECT * from cfsst3 ;
pk | ck | rc
----+----+------
5 | 7 | null
8 | 10 | null
2 | 3 | null
4 | 6 | null
7 | 9 | null
6 | 8 | null
(6 rows)
"
* 'projects/sstables-30/try-runtime/v8' of https://github.com/argenet/scylla:
database: Honour enable_sstables_mc_format configuration option.
sstables: Support SSTables 'mc' format as a feature.
db: Add configuration option for enabling SSTables 'mc' format.
tests: Add test for reading a complex column with zero subcolumns (SST3).
sstables: Fix parsing of complex columns with zero subcolumns.
sstables: Explicitly cast api::timestamp_type to uint64_t when delta-encoding.
sstables: Use parser_type instead of abstract_type::parse_type in column_translation.
bytes: Add helper for turning bytes_view into sstring_view.
sstables: Only forward the call to fast_forwarding_to in mp_row_consumer_m if filter exists.
sstables: Fix string formatting for exception messages in m_format_read_helpers.
sstables: Don't validate timestamps against the max value on parsing.
sstables: Always store only min bases in serialization_header.
sstables: Support 'mc' version parsing from filename.
SST3: Make sure we call consume_partition_end
This test is not applicable to the 'mc' format as it covers a backward
compatibility case which may only occur with SSTables generated by older
Scylla versions in 'ka' format.
Signed-off-by: Vladimir Krivopalov <vladimir@scylladb.com>
This is a helper flat_mutation_reader that wraps another reader and
splits range tombstones over rows before emitting them.
It is used to produce the same mutation streams for both old (ka/la) and
new (mc) SSTables formats in unit tests.
Signed-off-by: Vladimir Krivopalov <vladimir@scylladb.com>
This fixes the monotonicity issue as otherwise the range tombstone
emitted after such clustering row has a start position that should be
ordered before that of the row.
Signed-off-by: Vladimir Krivopalov <vladimir@scylladb.com>
"
This series changes commitlog write path so that it uses fragmented
buffers and therefore avoids large allocations. This is done by first
switching the code to use seastar memory_output_stream interface, which
can handle fragmented buffer without any additional actions from the
user code needed and then making it use buffers of fixed size 128 kB.
Tests: unit(release, debug) dtest(commitlog_test.py:TestCommitLog.test_commitlog_replay_on_startup commitlog_test.py:TestCommitLog.test_commitlog_replay_with_alter_table)
"
* tag 'fragmented-commitlog-writes/v3' of https://github.com/pdziepak/scylla:
commitlog: switch to fragmented buffers
commitlog: drop buffer pools
commitlog: drop recovery from bad alloc
utils: drop data_output
commitlog: use memory_output_stream
serialization_visitors: add support for memory_output_stream
utils: fragmented_temporary_buffer::view: add remove_prefix()
utils: fragmented_temporary_buffer: add empty() and size_bytes()
utils: fragmented_temporary_buffer: add get_ostream()
idl: serializer: don't assume Iterator::value_type is bytes_view
idl: serializer: create buffer view from streams
utils: crc: accept FragmentRange
When an sstable is deleted, this work is done as a background task
since it cannot be done from the destructor. If we don't wait for
that background task, it is detected as a leak by ASAN.
Fix by waiting for background tasks in every test.
A more complete fix would involve having a factory class create
sstables and assume the responsibility for background tasks, and
something similar to with_cql_test_env(), but that is deferred until later.
Tests: sstable_3_x_test (debug).
Message-Id: <20180923111745.8313-1-avi@scylladb.com>
Currently timeout is opt-in, that is, all methods that even have it
default it to `db::no_timeout`. This means that ensuring timeout is used
where it should be is completely up to the author and the reviewrs of
the code. As humans are notoriously prone to mistakes this has resulted
in a very inconsistent usage of timeout, many clients of
`flat_mutation_reader` passing the timeout only to some members and only
on certain call sites. This is small wonder considering that some core
operations like `operator()()` only recently received a timeout
parameter and others like `peek()` didn't even have one until this
patch. Both of these methods call `fill_buffer()` which potentially
talks to the lower layers and is supposed to propagate the timeout.
All this makes the `flat_mutation_reader`'s timeout effectively useless.
To make order in this chaos make the timeout parameter a mandatory one
on all `flat_mutation_reader` methods that need it. This ensures that
humans now get a reminder from the compiler when they forget to pass the
timeout. Clients can still opt-out from passing a timeout by passing
`db::no_timeout` (the previous default value) but this will be now
explicit and developers should think before typing it.
There were suprisingly few core call sites to fix up. Where a timeout
was available nearby I propagated it to be able to pass it to the
reader, where I couldn't I passed `db::no_timeout`. Authors of the
latter kind of code (view, streaming and repair are some of the notable
examples) should maybe consider propagating down a timeout if needed.
In the test code (the wast majority of the changes) I just used
`db::no_timeout` everywhere.
Tests: unit(release, debug)
Signed-off-by: Botond Dénes <bdenes@scylladb.com>
Message-Id: <1edc10802d5eb23de8af28c9f48b8d3be0f1a468.1536744563.git.bdenes@scylladb.com>
"
In SSTables 3.x, the 'ancestors' field of compaction metadata is no
longer stored in the Statistics.db file
The newly added test has previously failed due to this inconsistency.
Tests: unit {release}
"
* 'projects/sstables-30/empty_clustering_key/v1' of https://github.com/argenet/scylla:
tests: Add test for reading table with empty clustering key from SSTables 3.x.
tests: Update Statistics.db files for SSTables 3.x write tests.
sstables: Do not parse ancestors from compaction metadata for SSTables 3.x
Those files have been generated with 'ancestors' field in compaction
metadata and so were invalid.
Signed-off-by: Vladimir Krivopalov <vladimir@scylladb.com>
"
Some of the write tests were missing the read after write validation
which has now been added for better coverage.
Tests: unit {release}
"
* 'projects/sstables-30/more-enriched-tests/v1' of https://github.com/argenet/scylla:
tests: Enrich test_write_adjacent_range_tombstones_with_rows with read after write
tests: Enrich test_write_many_range_tombstones with read after write
tests: Enrich test_write_mixed_rows_and_range_tombstones with read after write
tests: Enrich test_write_non_adjacent_range_tombstones with read after write
tests: Enrich test_write_adjacent_range_tombstones with read after write
tests: Enrich test_write_simple_range_tombstone with read after write.
tests: Enrich test_write_deleted_column with read after write.
"
This patchset fixes the bug in SSTables 3.x parser that did not properly
handle deleted counter cells.
A write test is enriched to validate read after write so that this case
is covered.
Tests: unit {release}
"
* 'projects/sstables-30/fix-deleted-counters-read/v1' of https://github.com/argenet/scylla:
tests: Read after write in test_write_counter_table.
sstables: Fix deleted counter cells processing in SSTables 3.x parser.
When a query with multicolumn inequality is issued on clustering columns
having mixed order (ASC and DESC together), if the ranges are not
broken to none overlapping lexicographically monotonic ones, the node
return incorrect rows. This is due to the search nature
(prefix comparison). The solution is to break the range imposed
by the restriction into several single column restrictions OR-ed
together that will be logically equivalent and preserve the
monotonicity assumption. This commit also fixes incorrect results
returned by a multicolumn query on an all descending columns.
A unit test have been added to account for both issues fixed.
Fixes#2050
Tests: Unit test, manual tests of the use case in the issue.
Signed-off-by: Eliran Sinvani <eliransin@scylladb.com>
Message-Id: <3b96620a3bd8b0614359a3b0757f324d45189dbb.1536478193.git.eliransin@scylladb.com>
The test in question is `restricted_reader_timeout`.
Use `eventually_true()` instead of `sleep()` to wait on the timeout
expiring, making the test more robust on overloaded machines.
Also fix graceful failing, another longstanding issue with this test.
The readers created for the test need different destruction logic
depending whether the test failed or succeeded. Previously this was
dealt with by using the logic that worked in case of success and using
asserts to abort when the test failed, thus avoiding developers
investigating the invalid memory accesses happening due to the wrong
destruction logic.
The solution is to use BOOST_CHECK() macro in the check that validates
whether timeout works as expected. This allows for execution to continue
even if the test failed, and thus allows for running the proper cleanup
code even when the test failed.
Fixes: #3719
Signed-off-by: Botond Dénes <bdenes@scylladb.com>
Message-Id: <911921dffc924f1b0a3e86408757467e9be2b65b.1537169933.git.bdenes@scylladb.com>
test_partial_delete_selected_column() does a long string of various
updates and deletes, each specifies a different timestamp. In one
of these updates, the timestamp was forgotten. This means that the
server picks the current time, a large number.
As the test is currently written, it doesn't matter which timestamp
was chosen, the test would still succeed (if timestamp >= 15, and it
must be since the timestamp is the time from the epoch).
But the intention was probably to use timestamp = 15, so let's make
this intention clear.
Signed-off-by: Nadav Har'El <nyh@scylladb.com>
Message-Id: <20180905095552.11883-2-nyh@scylladb.com>
