Row cache needs to be invalidated whenever data in sstables
changes. Cleanup removes data from sstables which doesn't belong to
the node anymore, which means cache must be invalidated on cleanup.
Currently, stale data can be returned when a node re-owns ranges which
data are still stored in the node's row cache, because cleanup didn't
invalidate the cache."
Fixes#4446.
tests:
- unit tests (dev mode)
- dtests:
update_cluster_layout_tests.py:TestUpdateClusterLayout.simple_decommission_node_2_test
cleanup_test.py
This patch causes inclusive and exclusive range deletes to be
distinguished in cdc log. Previously, operations `range_delete_start`
and `range_delete_end` were used for both inclusive and exclusive bounds
in range deletes. Now, old operations were renamed to
`range_delete_*_inclusive`, and for exclusive deletes, new operations
`range_delete_*_exclusive` are used.
Tests: unit(dev)
Previously we required MODIFY permissions on all materialized views in
order to modify a table. This is wrong, because the views should be
synced to the table unconditionally. For the same reason,
users *shouldn't* be granted MODIFY on views, to prevent them manually
changing (and breaking) a view.
This patch removes an explicit permissions check in
modification_statement introduced by 65535b3. It also tests that a
user can indeed modify a table they are allowed to modify, regardless
of lacking permissions on the table's views and indices.
Fixes#5205.
Signed-off-by: Dejan Mircevski <dejan@scylladb.com>
This descriptor contain all information needed for table to be properly
updated on compaction completion. A new member will be added to it soon,
which will store ranges to be invalidated in row cache on behalf of
cleanup compaction.
Signed-off-by: Raphael S. Carvalho <raphaelsc@scylladb.com>
"
This change set is comprised of several unrelated patches regarding
some cleanups in cql3 layer code.
Most of the changes are aimed at eliminating superfluous `shared_ptr`
usages. In places where it can be safely assumed that objects passed
to the function are considered non-null and constant, these places
were adjusted to use passing as const ref instead.
Other changes incude eliminating unused arguments at some functions
and replacing usages of `shared_ptr<service::pager::paging_state>`
to use `lw_shared_ptr` instead, since `pager::paging_state` is final.
Tests: unit(dev, debug)
"
* 'feature/cql_cleanups_4' of https://github.com/ManManson/scylla:
cql3: minor sweeps through the cql layer code to reduce shared_ptrs count
cql3: change some function signatures to accept const references
cql3: change signatures of several functions to return crefs instead of pointers
cql3: remove unused argument at functions::castas_functions::get
paging_state: switch from shared_ptr to lw_shared_ptr
" from Botond
Nodetool scrub rewrites all sstables, validating their data. If corrupt
data is found the scrub is aborted. If the skip-corrupted flag is set,
corrupt data is instead logged (just the keys) and skipped.
The scrubbing algorithm itself is fairly simple, especially that we
already have a mutation stream validator that we can use to validate the
data. However currently scrub is piggy-backed on top of cleanup
compaction. To implement this flag, we have to make scrub a separate
compaction type and propagate down the flag. This required some
massaging of the code:
* Add support for more than two (cleanup or not) compaction types.
* Allow passing custom options for each compaction type.
* Allow stopping a compaction without the manager retrying it later.
Additionally the validator itself needed some changes to allow different
ways to handle errors, as needed by the scrub.
Fixes: #5487
* https://github.com/denesb/nodetool-scrub-skip-corrupted/v7:
table: cleanup_sstables(): only short-circuit on actual cleanup
compaction: compaction_type: add Upgrade
compaction: introduce compaction_options
compaction: compaction_descriptor: use compaction options instead of
cleanup flag
compaction_manager: collect all cleanup related logic in
perform_cleanup()
sstables: compaction_stop_exception: add retry flag
mutation_fragment_stream_validator: split into low-level and
high-level API
compaction: introduce scrub_compaction
compaction_manager: scrub: don't piggy-back on upgrade_sstables()
test: sstable_datafile_test: add scrub unit test
All the places that use partitioner have been switched
to not use global partitioner any more and we can stop
setting it in this test.
Signed-off-by: Piotr Jastrzebski <piotr@scylladb.com>
random_schema already has a _schema field which in turn
has a get_partitioner() function. Store partitioner
in random_schema is redundant.
At the moment all uses of random_schema are based on
default partitioner so it is not necessary to set it
explicitly. If in the future we need random_schema to
work with other partitioners we will add the constructor
back and fix the creation of _schema to contain it. It's
not needed now though.
Signed-off-by: Piotr Jastrzebski <piotr@scylladb.com>
and replace all dht::global_partitioner().decorate_key
with dht::decorate_key
It is an improvement because dht::decorate_key takes schema
and uses it to obtain partitioner instead of using global
partitioner as it was before.
Signed-off-by: Piotr Jastrzebski <piotr@scylladb.com>
Take const schema& as a parameter of shard_of and
use it to obtain partitioner instead of calling
global_partitioner().
Signed-off-by: Piotr Jastrzebski <piotr@scylladb.com>
i_partitioner.hh is widely included while sharders are used
only in 6 places so there's no need to include them in
the whole codebase.
Signed-off-by: Piotr Jastrzebski <piotr@scylladb.com>
clustering_interval_set is a rarely used class, but one that requires
boost/icl, which is quite heavyweight. To speed up compilation, move
it to its own header and sprinkle #includes where needed.
Tests: unit (dev)
Message-Id: <20200214190507.1137532-1-avi@scylladb.com>
Merged patch series from Avi Kivity:
token_metadata is a heavyweight class with heavyweight includes
(boost/icl) it is a good candidate for the pimpl pattern, which
this series implements.
Tests: unit (dev)
https://github.com/avikivity/scylla token_metadata-pimplification/v1
Avi Kivity (6):
locator: token_metadata: use non-deduced return type for ring_range()
locator: token_metadata: pimplify
locator: token_metadata: make token_metadata_impl::tokens_iterator a
non-nested class
locator: token_metadata: pimplify tokens_iterator
locator: token_metadata: move implementation classes to .cc
locator: token_metadata: remove unused include "query-request.hh"
locator/token_metadata.hh | 783 +---------------
locator/token_metadata.cc | 1338 ++++++++++++++++++++++++++-
test/boost/sstable_datafile_test.cc | 1 +
3 files changed, 1332 insertions(+), 790 deletions(-)
Message-Id: <20200214184954.1130194-1-avi@scylladb.com>
Change the way `service::pager::paging_state` is passed around
from `shared_ptr` to `lw_shared_ptr`. It's safe since
`paging_state` is final.
Tests: unit(dev, debug)
Signed-off-by: Pavel Solodovnikov <pa.solodovnikov@scylladb.com>
Running cdc_test binary fails with a segmentation fault
when run with --smp 1, because test_cdc_across_shards
assumes shard count to be >=2. This patch skips the test case
when run with a single shard and produces a log warning.
Message-Id: <9b00537db9419d8b7c545ce0c3b05b8285351e7d.1581600854.git.sarna@scylladb.com>
Cells in CDC logs used to be created while completely neglecting TTLs
(the TTLs from cdc = {...'ttl':600}). This patch adds TTLs to all cells;
there are no row markers, so wee need not set TTL there.
Fixes#5688
* jul-stas/5688-set-ttl-in-cdc-log-table:
tests/cdc: added test for TTL on log table cells
cdc: set TTLs on CDC log cells
Empty values (zero-sized string in serialized form) were not
handled properly in serialize routines for floating types and
uuids, which led to runtime exceptions and failing tests as
described in https://github.com/scylladb/scylla/issues/5782.
Also fix validation visitor to handle empty values properly.
There already was the code in place that took into
consideration zero-sized values. But it was trying to read
some bytes regardless of that (e.g. for timeuuid values),
even if there is none to read.
Tests: unit(dev, debug)
Fixes: #5782
Signed-off-by: Pavel Solodovnikov <pa.solodovnikov@scylladb.com>
Message-Id: <20200213130021.31598-1-pa.solodovnikov@scylladb.com>
The update generation path must track and apply all tombstones,
both from the existing base row (if read-before-write was needed)
and for the new row. One such path contained an error, because
it assumed that if the existing row is empty, then the update
can be simply generated from the new row. However, lack of the
existing row can also be the result of a partition/range tombstone.
If that's the case, it needs to be applied, because it's entirely
possible that this partition row also hides the new row.
Without taking the partition tombstone into account, creating
a future tombstone and inserting an out-of-order write before it
in the base table can result in ghost rows in the view table.
This patch comes with a test which was proven to fail before the
changes.
Branches 3.1,3.2,3.3
Fixes#5793
Tests: unit(dev)
Message-Id: <8d3b2abad31572668693ab585f37f4af5bb7577a.1581525398.git.sarna@scylladb.com>
All internal execution always uses query text as a key in the
cache of internal prepared statements. There is no need
to publish API for executing an internal prepared statement object.
The folded execute_internal() calls an internal prepare() and then
internal execute().
execute_internal(cache=true) does exactly that.
Rename an overloaded function process() to execute_direct().
Execute direct is a common term for executing a statement
that was not previously prepared. See, for example
SQLExecuteDirect in ODBC/SQL CLI specification,
mysql_stmt_execute_direct() in MySQL C API or EXECUTE DIRECT
in Postgres XC.
"
Lots of code needs storage_service just to get token_metadata from.
This creates unwanted dependency loops and increases the use of
global storage_service instance.
This set keeps the sharded<locator::token_metadata> on main's stack
and carries the references where needed. This removes the dependency
on storage_service from:
- storage_proxy
- gossiper
- redis
- batchlog manager
and makes the database only need it for sstables_format (will fix
in one of the next sets).
Also, this set is the prerequisite for controlling the copying of
token_metadata instances (spotted two occurrences in bootstrap
code).
Tests: unit(dev), manual start-stop
"
* 'br-token-metadata-standalone-2' of https://github.com/xemul/scylla:
api: Keep and use reference on token_metadata
redis: Use proxy token_metadata
gossiper: Keep needed for failure_detection values on board
database: Use own token_metadata
batchlog: Use token_metadata from proxy
proxy: Use own token_metadata
gossiper: Use own token_metadata
tokens: Switch into standalone sharded instance
batchlog: Use in-config ring-delay
database: Have it in size_estimate_virtual_reader
storage_proxy: Pass token_metadata in some static helpers
storage_service: Move get_local_tokens wrapper
size_estimates_virtual_reader: Make get_local_ranges static
migration_manager: Refactor validation of new/updating ksm
storage_service: Tiny cleanup of excessive self-reference
Instead of the restrictive `cleanup` boolean flag, which allows for choosing
between only two compaction types, use `compaction_options`, which in
addition to allowing any number of compaction types to be selected,
also allows seamlessly passing specific options to them.
Merged pull request https://github.com/scylladb/scylla/pull/5755 from
Avi Kivity:
This series removes some #include dependencies around cql3. It results in
30k line (6.6%) reduction in the preprocessed size of database.i, mainly
due to elimination of boost::regex (which was brought in in turn by
like_matcher). This should result in fewer and faster recompiles.
commits:
tracing: remove #include of modification_statement.hh from table_helper
cql3: selection: remove now-unneeded include of statement_restrictions.hh
cql3: deinline result_set_builder::restrictions_filter constructor
view_info: remove include of select_statement.hh
cql3: selection: remove unnecessary include of selector_factories
cql3: query_processor: reduce #includes
Setting TTL = -1 in cdc_options prevents any writes to CDC log.
But enabling CDC and having unwritable log table makes no sense.
Notably, normal writes USING TTL -1 are forbidden. This patch does
the same to TTLs in CDC options.
Fixes#5747
* jul-stas/5747-cdc-disallow-negative-ttl:
tests/cdc: added test for exception when TTL < 0
cdc: disallow negative TTL values in CDC
Way too many places in code needs storage_service just for token_metadata.
These references increase the amount of get(_local)?_storage_service()
calls and create loops in components dependencies. Keep the token_metadata
separately from storage_service and pass instances' references where
needed (for now -- only into the storage_service itself).
Signed-off-by: Pavel Emelyanov <xemul@scylladb.com>
There's the call of the same name in storage_service, so
make this one explicitly static for better readability.
Signed-off-by: Pavel Emelyanov <xemul@scylladb.com>
