The existing TTL tests use only tiny tables, so don't exercise the
expiration-time scanner's use of paging. So in this patch we add
another test with a much larger table (with 40,000 items).
To verify that this test indeed checks paging, I stopped the scanner's
iteration after one page, and saw that this test starts failing (but
the smaller tests all pass).
Signed-off-by: Nadav Har'El <nyh@scylladb.com>
Most tests in test_ttl.py now pass, so remove their "xfail" tag.
The only remaining failing test is test_ttl_expiration_streams -
which cannot yet pass because the expiration event is not yet marked.
Note that the fact that almost all tests for Alternator's TTL feature
now pass does not mean the feature is complete. The current implementation
is very partial and inefficient, and only works reasonably in tests on
a single node. The current tests cannot expose these problems, so
we will need to develop additional tests for them. The tests will of
course remain useful to see that as the implementation continues to
improve, none of the tests that already work will break.
The Alternator TTL continues to remain "experimental", cannot be used
without explicitly enabling this experimental feature, and must not be
used for any important deployment.
Signed-off-by: Nadav Har'El <nyh@scylladb.com>
The tests for the TTL feature in test/alternator/test_ttl.py takes huge
amount of time on DynamoDB - 10 to 30 minutes (!) - because it delays
expiration of items a long time after their intended expiration times.
We intend Scylla's implementation to have a configurable delay for the
expiration scanner, which we will be able to configure to very short
delays for tests. So These tests can be made much faster on Scylla.
So in this patch we change all of the tests to finish much more quickly
on Scylla.
Many of the tests still fail, because the TTL feature is not implemented
yet.
Although after this change all the tests in test_ttl.py complete in
a reasonable amount of time (around 3 seconds each), we still mark
them as "veryslow" and the "--runveryslow" flag is needed to run them.
We should consider changing this in the future, so that these tests will
run as part of our default test suite.
Signed-off-by: Nadav Har'El <nyh@scylladb.com>
Although it isn't terribly useful, an Alternator user can enable TTL
with an expiration-time attribute set to a *key* attribute. Because
expiration times should be numeric - not other types like strings -
DynamoDB could warn the user when a chosen key attribute hs a non-
numeric type (since key attributes do have fixed types!). But DynamoDB
doesn't warn about this - it simply expires nothing. This test
verifies this that it indeed does this.
Signed-off-by: Nadav Har'El <nyh@scylladb.com>
The basic test for TTL expiration, test_ttl.py::test_ttl_expiration,
uses a table with only a partition key. Most of the item expiration
logic is exactly the same for tables that also have a sort key, but
the step of *deleting* the item is different, so let's add a test
that verifies that also in this case, the expired item is properly
deleted.
Signed-off-by: Nadav Har'El <nyh@scylladb.com>
This patch improves test_ttl.py::test_ttl_expiration in two ways:
First, it checks yet another case - that items that have the wrong type
for the expiration-time column (e.g., a string) never get expired - even
if that string happens to contain a number that looks like an expiration
time.
Second, instead of the huge 15-minute duration for this test, the
test now has a configurable duration; We still need to use a very long
duration on AWS, but in Scylla we expect to be able to configure the
TTL scan frequency, and can finish this test in just a few seconds!
We already have experimental code which makes this test pass in just
3 seconds.
Signed-off-by: Nadav Har'El <nyh@scylladb.com>
Extract a boolean function is_aws() out of the "scylla_only" fixture, so
it can be used in tests for other purposes.
For example, in the next patch the TTL tests will use them to pick
different timeouts on AWS (where TTL expiration have huge many-minute
delays) and on Scylla (which can be configured to have very short delays).
Signed-off-by: Nadav Har'El <nyh@scylladb.com>
Query time must be fetched after populate. If compaction is executed
during populate it may be executed with timestamp later than query_time.
This would cause the test expected compaction and compaction during
populate to be executed at different time points producing different
results. The result would be sporadic test failures depending on relative
timing of those operations. If no other mutations happen after populate,
and query_time is later than the compaction time during population, we're
guaranteed to have the same results.
Message-Id: <20211123134808.105068-1-mikolaj.sieluzycki@scylladb.com>
"
There's a nest of globals in streaming/ code. The stream_manager
itself and a whole lot of its dependencies (database, sys_dist_ks,
view_update_generator and messaging). Also streaming code gets
gossiper instance via global call.
The fix is, as usual, in keeping the sharded<stream_manager> in
the main() code and pushing its reference everywhere. Somwehere
in the middle the global pointers go away being replaced with
respective references pushed to the stream_manager ctor.
This reveals an implicit dependency:
storage_service -> stream_manager
tests: unit(dev),
dtest.cdc_tests.cluster_reduction_with_cdc(dev)
v1: dtest.bootstrap_test.add_node(dev)
v1: dtest.bootstrap_test.simple_bootstrap(dev)
"
* 'br-unglobal-stream-manager-3-rebase' of https://github.com/xemul/scylla: (26 commits)
streaming, main: Remove global stream_manager
stream_transfer_task: Get manager from session (result-future)
stream_transfer_task: Keep Updater fn onboard
stream_transfer_task: Remove unused database reference
stream_session: Use manager reference from result-future
stream_session: Capture container() in message handler
stream_session: Keep stream_manager reference
stream_session: Remove unused default contructor
stream_result_future: Use local manager reference
stream_result_future: Keep stream_manager reference
stream_plan: Keep stream_manager onboard
dht: Keep stream_manager on board
streaming, api: Use captured manager in handlers
streaming, api: Standardize the API start/stop
storage_service: Sanitize streaming shutdown
storage_service: Keep streaming_manager reference
stream_manager: Use container() in notification code
streaming: Move get_session into stream_manager
streaming: Use container.invoke_on in rpc handlers
streaming: Fix interaction with gossiper
...
The manager is drained() on drain/decommission/isolate. Since now
it's storage_service who orchestrates all of the above, it needs
and explicit reference on the target.
Signed-off-by: Pavel Emelyanov <xemul@scylladb.com>
This is a translation of Cassandra's CQL unit test source file
validation/entities/UserTypesTest.java into our our cql-pytest
framework.
This test file includes 26 tests for various features and corners of
the user-defined type feature. Two additional tests which were more
involved to translate were dropped with a comment explaining why.
All 26 tests pass on Cassandra, and all but one pass on Scylla:
The test testUDTWithUnsetValues fails on Scylla so marked xfail.
It reproduces a previously-unknown Scylla bug:
Refs #9671: In some cases, trying to assign an UNSET value into part
of a UDT is not detected
Signed-off-by: Nadav Har'El <nyh@scylladb.com>
Message-Id: <20211124074001.708183-1-nyh@scylladb.com>
This series gets rid of the global batchlog_manager instance.
It does so by first, allowing to set a global pointer
and instatiating stack-local instances in main and
cql_test_env.
Expose the cql_test_env batchlog_manager to tests
so they won't need the global `get_batchlog_manager()` as
used in batchlog_manager_test.test_execute_batch.
Then we pass a reference to the `sharded<db::batchlog_manager>` to
storage_service so it can be used instead of the global one.
Derive batchlog_manager from peering_sharded_service so it
get its `container()` rather than relying on the global `get_batchlog_manager()`.
And finally, handle a circular dependency between the batchlog_manager,
that relies on the query_processor that, in turn, relies on the storage_proxy,
and the the storage_proxy itself that depends on the batchlog_manager for
`mutate_atomically`.
Moved `endpoint_filter` to gossiper so `storage_proxy::mutate_atomically`
can call it via the `_gossiper` member it already has.
The function requires a gossiper object rather than a batchlog_manager
object.
Also moved `get_batch_log_mutation_for` to storage_proxy so it can be
called from `sync_write_to_batchlog` (also from the mutate_atomically path)
Test: unit(dev)
DTest: batch_test.py:TestBatch.test_batchlog_manager_issue(dev)
* git@github.com:bhalevy/scylla.git deglobalize-batchlog_manager-v2
get rid of the global batchlog_manager
batchlog_manager: get_batch_log_mutation_for: move to storage_proxy
batchlog_manager: endpoint_filter: move to gossiper
batchlog_manager: do_batch_log_replay: use lambda coroutine
batchlog_manager: derive from peering_sharded_service
storage_service: keep a reference to the batchlog_manager
test: cql_test_env: expose batchlog_manager
main: allow setting the global batchlog_manager
These test cases may crash if running with more shards.
This is not required for test.py runs, but rather when
running the test manually using the command line.
Signed-off-by: Benny Halevy <bhalevy@scylladb.com>
Message-Id: <20211122204340.1020932-1-bhalevy@scylladb.com>
Add a restriction in create_view_statement to disallow creation of MV for CDC Log table.
Also add a CQL test that checks the new restriction works.
Test: unit(dev)
Fixes#9233Closes#9663
* 'fix9233' of https://github.com/haaawk/scylla:
tests: Add cql test to verify it's impossible to create MV for CDC Log
cql3: Make it impossible to create MV on CDC log
In the DynamoDB API, UpdateItem's AttributeUpdates parameter (the older
syntax, which was superseded by UpdateExpression) has a DELETE operation
that can do two different things: It can delete an attribute, or it can
delete elements from a set. Before this patch we only implemented the
first feature, and this patch implements the second.
Note that unlike the ordinary delete, the second feature - set subtraction -
is a read-modify-write operation. This is not only because of Alternator's
serialization (as JSON strings, not CRDTs) - but also fundementally because
of the API's guarantees - e.g., the operation is supposed to fail if the
attribute's existing value is *not* a set of the correct type, so it
needs to read the old value.
The test for this feature begins to pass, so its "xfail" mark is
removed. After this, all tests in test/alternator/test_item.py pass :-)
Fixes#5864.
Signed-off-by: Nadav Har'El <nyh@scylladb.com>
Message-Id: <20211103151206.157184-1-nyh@scylladb.com>
And rename to get_batchlog_mutation_for while at it,
as it's about the batchlog, not batch_log.
This resolves a circular dependency between the
batchlog_manager and the storage_proxy that required
it in the case.
Signed-off-by: Benny Halevy <bhalevy@scylladb.com>
As a prerequisite to globalizing the batchlog_manager,
allow setting a global pointer to it and instantiate
the sharded<db::batchlog_manager> on the main/cql_test_env
stack.
Signed-off-by: Benny Halevy <bhalevy@scylladb.com>
Indexed queries are using paging over the materialized view
table. Results of the view read are then used to issue reads of the
base table. If base table reads are short reads, the page is returned
to the user and paging state is adjusted accordingly so that when
paging is resumed it will query the view starting from the row
corresponding to the next row in the base which was not yet
returned. However, paging state's "remaining" count was not reset, so
if the view read was exhausted the reading will stop even though the
base table read was short.
Fix by restoring the "remaining" count when adjusting the paging state
on short read.
Tests:
- index_with_paging_test
- secondary_index_test
Fixes#9198
Message-Id: <20210818131840.1160267-1-tgrabiec@scylladb.com>
First, it doesn't test the gossiper so
it's unclear why have it at all.
And it doesn't test anything more than what we test
using the cql_test_env either.
For testing gossip there is test/manual/gossip.
Signed-off-by: Benny Halevy <bhalevy@scylladb.com>
Message-Id: <20211122081305.789375-2-bhalevy@scylladb.com>
"
After this series, compaction will finally stop including database.hh.
tests: unit(debug).
"
* 'stop_including_database_hh_for_compaction' of github.com:raphaelsc/scylla:
compaction: stop including database.hh
compaction: switch to table_state in get_fully_expired_sstables()
compaction: switch to table_state
compaction: table_state: Add missing methods required by compaction
Make compaction procedure switch to table_state. Only function in
compaction.cc still directly using table is
get_fully_expired_sstables(T,...), but subsequently we'll make it
switch to table_state and then we can finally stop including database.hh
in the compaction code.
Signed-off-by: Raphael S. Carvalho <raphaelsc@scylladb.com>
These are the only methods left for compaction to switch to
table_state, so compaction can finally stop including database.hh
Signed-off-by: Raphael S. Carvalho <raphaelsc@scylladb.com>
"
Add a sharded locator::effective_replication_map_factory that holds
shared effective_replication_maps.
To search for e_r_m in the factory, we use a compound `factory_key`:
<replication_strategy type, replication_strategy options, token_metadata ring version>.
Start the sharded factory in main (plus cql_test_env and tools/schema_loader)
and pass a reference to it to storage_proxy and storage_server.
For each keyspace, use the registry to create the effective_replication_map.
When registered, effective_replication_map objects erase themselves
from the factory when destroyed. effective_replication_map then schedules
a background task to clear_gently its contents, protected by the e_r_m_f::stop()
function.
Note that for non-shard 0 instances, if the map
is not found in the registry, we construct it
by cloning the precalculated replication_map
from shard 0 to save the cpu cycles of re-calculating
it time and again on every shard.
Test: unit(dev), schema_loader_test(debug)
DTest: bootstrap_test.py:TestBootstrap.decommissioned_wiped_node_can_join_test update_cluster_layout_tests.py:TestUpdateClusterLayout.simple_add_new_node_while_schema_changes_with_repair_test (dev)
"
* tag 'effective_replication_map_factory-v7' of https://github.com/bhalevy/scylla:
effective_replication_map: clear_gently when destroyed
database: shutdown keyspaces
test: cql_test_env: stop view_update_generator before database shuts down
effective_replication_map_factory: try cloning replication map from shard 0
tools: schema_loader: start a sharded erm_factory
storage_service: use erm_factory to create effective_replication_map
keyspace: use erm_factory to create effective_replication_map
effective_replication_map: erase from factory when destroyed
effective_replication_map_factory: add create_effective_replication_map
effective_replication_map: enable_lw_shared_from_this
effective_replication_map: define factory_key
keyspace: get a reference to the erm_factory
main: pass erm_factory to storage_service
main: pass erm_factory to storage_proxy
locator: add effective_replication_map_factory
We can't have view updates happening after the database shuts down.
In particular, mutateMV depends on the keyspace effective_replaication_map
and it is going to be released when all keyspaces shut down, in the next patch.
Signed-off-by: Benny Halevy <bhalevy@scylladb.com>
To be used for creating effective_replication_map
when token_metadata changes, and update all
keyspaces with it.
Signed-off-by: Benny Halevy <bhalevy@scylladb.com>
It will be used further to create shared copies
of effective_replication_map based on replication_strategy
type and config options.
Signed-off-by: Benny Halevy <bhalevy@scylladb.com>
We're using a coarse resolution when rounding clock time for sstables to
be evenly distributed across time buckets. Let's use a better resolution,
to make sure sstables won't fall into the edges.
Signed-off-by: Raphael S. Carvalho <raphaelsc@scylladb.com>
Message-Id: <20211118172126.34545-1-raphaelsc@scylladb.com>
The test checks every 100 * smp::count milliseconds that a shard
had been able to make at least once step. Shards, in turn, take up
to 100 ms sleeping breaks between steps. It seems like on heavily
loaded nodes the checking period is too small and the test
stuck-detector shoots false-positives.
Signed-off-by: Pavel Emelyanov <xemul@scylladb.com>
Message-Id: <20211118154932.25859-1-xemul@scylladb.com>
The previous implementation based on `delivery_queue` had a serious
defect: if receiving a message (`rpc::receive`) blocked, other messages
in the queue had to wait. This would cause, for example, `vote_request`
messages to stop being handled by a server if the server was in the middle
of applying a snapshot.
Now `rpc::receive` returns `void`, not `future<>`. Thus we no longer
need `delivery_queue`: the network message delivery function can simply
call `rpc::receive` directly. Messages which require asynchronous work
to be performed (such as snapshot application) are handled in
`rpc::receive` by spawning a background task. The number of such
background tasks is limited separately for each message type; now if
we exceed that limit, we drop other messages of this type (previously
they would queue up indefinitely and block not only other messages
of this type but different types as well).
Message-Id: <20211116163316.129970-1-kbraun@scylladb.com>
fmt 8 checks format strings at compile time, and requires that
non-compile-time format strings be wrapped with fmt::runtime().
Do that, and to allow coexistence with fmt 7, supply our own
do-nothing version of fmt::runtime() if needed. Strictly speaking
we shouldn't be introducing names into the fmt namespace, but this
is transitional only.
Closes#9640
A member variable is a reference, not a pure value, so std::same_as<>
needs to be given a reference (and clanf 13 insists). However, clang
12 doesn't accept the correct constraint, so use std::convertible_to<>
as a compromise.
Closes#9642
If we get errors/exceptions in delete_segments we can (and probably will) loose track of disk footprint counters. This can in turn, if using hard limits, cause us to block indefinitely on segment allocation since we might think we have larger footprint than we actually do.
Of course, if we actually fail deleting a segment, it is 100% true that we still technically hold this disk footprint (now unreachable), but for cases where for example outside forces (or wacky tests) delete a file behind our backs, this might not be true. One could also argue that our footprint is the segments and file names we keep track of, and the rest is exterior sludge.
In any case, if we have any exceptions in delete_segments, we should recalculate disk footprint based on current state, and restart all new_segment paths etc.
Fixes#9348
(Note: this is based on previous PR #9344 - so shows these commits as well. Actual changes are only the latter two).
Closes#9349
* github.com:scylladb/scylla:
commitlog: Recalculate footprint on delete_segment exceptions
commitlog_test: Add test for exception in alloc w. deleted underlying file
commitlog: Ensure failed-to-create-segment is re-deleted
commitlog::allocate_segment_ex: Don't re-throw out of function
Add schema parameter so that:
* Caller has better control over schema -- especially relevant for
reverse reads where it is not possible to follow the convention of
passing the query schema which is reversed compared to that of the
mutations.
* Now that we don't depend on the mutations for the schema, we can lift
the restriction on mutations not being empty: this leads to safer
code. When the mutations parameter is empty, an empty reader is
created.
Add "make_" prefix to follow convention of similar reader factory
functions.
Tests: unit(dev)
Signed-off-by: Botond Dénes <bdenes@scylladb.com>
Message-Id: <20211115155614.363663-1-bdenes@scylladb.com>
In the DynamoDB API, a number is encoded in JSON requests as something
like: {"N": "123"} - the type is "N" and the value "123". Note that the
value of the number is encoded as a string, because the floating-point
range and accuracy of DynamoDB differs from what various JSON libraries
may support.
We have a function unwrap_number() which supported the value of the
number being encoded as an actual number, not a string. But we should
NOT support this case - DynamoDB doesn't. In this patch we add a test
that confirms that DynamoDB doesn't, and remove the unnecessary case
from unwrap_number(). The unnecessary case also had a FIXME, so it's
a good opportunity to get rid of a FIXME.
When writing the test, I noticed that the error which DynamoDB returns
in this case is SerializionException instead of the more usual
ValidationException. I don't know why, but let's also change the error
type in this patch.
Signed-off-by: Nadav Har'El <nyh@scylladb.com>
Message-Id: <20211115125738.197099-1-nyh@scylladb.com>
Add "rows" field to system.large_partitions. Add partitions to the
table when they are too large or have too many rows.
Fixes#9506
Signed-off-by: Michael Livshin <michael.livshin@scylladb.com>
Closes#9577
The db::config reference is available on the database, which
can be get from the virtual_table itself. The problem is that
it's a const refernece, while system.config will be updateable
and will need non-const reference.
Adding non-const get_config() on the database looks wrong. The
database shouldn't be used as config provider, even the const
one.
Signed-off-by: Pavel Emelyanov <xemul@scylladb.com>
The series fixes issues:
server may use the wrong configuration after applying a remote snapshot, causing a split-brain situation
assertion ins raft::server_impl::notify_waiters()
snapshot transfer to a server removed from the configuration should be aborted
cluster may become stuck when a follower takes a snapshot after an accepted entry that the leader didn't learn about
* scylla-dev/random-test-fixes-v2:
raft: rename rpc_configuration to configuration in fsm output
raft: test: test case for the issue #9552
raft: fix matching of a snapshotted log on a follower
raft: abort snapshot transfer to a server that was removed from the configuration
raft: fix race between snapshot application and committing of new entries
raft: test: add test for correct last configuration index calculation during snapshot application
raft: do not maintain _last_conf_idx and _prev_conf_idx past snapshot index
raft: correctly truncate the log in a persistence module during snapshot application
"
table_state is being introduced for compaction subsystem, to remove table dependency
from compaction interface, fix layer violations, and also make unit testing
easier as table_state is an abstraction that can be implemented even with no
actual table backing it.
In this series, compaction strategy interfaces are switching to table_state,
and eventually, we'll make compact_sstables() switch to it too. The idea is
that no compaction code will directly reference a table object, but only work
with the abstraction instead. So compaction subdirectory can stop
including database.hh altogether, which is a great step forward.
"
* 'table_state_v5' of https://github.com/raphaelsc/scylla:
sstable_compaction_test: switch to table_state
compaction: stop including database.hh for compaction_strategy
compaction: switch to table_state in estimated_pending_compactions()
compaction: switch to table_state in compaction_strategy::get_major_compaction_job()
compaction: switch to table_state in compaction_strategy::get_sstables_for_compaction()
DTCS: reduce table dependency for task estimation
LCS: reduce table dependency for task estimation
table: Implement table_state
compaction: make table param of get_fully_expired_sstables() const
compaction_manager: make table param of has_table_ongoing_compaction() const
Introduce table_state
