"
Issue https://github.com/scylladb/scylla/issues/7019 describes a problem of an ever-growing map of temporary values stored in query_options. In order to mitigate this kind of problems, the storage for temporary values is moved from an external data structure to the value views itself. This way, the temporary lives only as long as it's accessible and is automatically destroyed once a request finishes. The downside is that each temporary is now allocated separately, while previously they were bundled in a single byte stream.
Tests: unit(dev)
Fixes https://github.com/scylladb/scylla/issues/7019
"
* psarna-move_temporaries_to_value_view:
cql3: remove query_options::linearize and _temporaries
cql3: remove make_temporary helper function
cql3: store temporaries in-place instead of in query_options
cql3: add temporary_value to value view
cql3: allow moving data out of raw_value
cql3: split values.hh into a .cc file
With relatively big summaries, reactor can be stalled for a couple
of milliseconds.
This patch:
a. allocates positions upfront to avoid excessive reallocation.
b. returns a future from seal_summary() and uses `seastar::do_for_each`
to iterate over the summary entries so the loop can yield if necessary.
Fixes#7108.
Based on 2470aad5a389dfd32621737d2c17c7e319437692 by Raphael S. Carvalho <raphaelsc@scylladb.com>
Test: unit(dev)
Signed-off-by: Benny Halevy <bhalevy@scylladb.com>
Message-Id: <20200826091337.28530-1-bhalevy@scylladb.com>
Per sheduling-group data was moved from the task queues to a separate
data member in the reactor itself. Update `scylla memory` to use the new
location to get the per sheduling group data for the storage proxy
stats.
Signed-off-by: Botond Dénes <bdenes@scylladb.com>
Message-Id: <20200825140256.309299-1-bdenes@scylladb.com>
We copy a list, which was reported to generate a 15ms stall.
This is easily fixed by moving it instead, which is safe since this is
the last use of the variable.
Fixes#7115.
If file_writer::close() fails to close the output stream
closing will be retried in file_writer::~file_writer,
leading to:
```
include/seastar/core/future.hh:1892: seastar::future<T ...> seastar::promise<T>::get_future() [with T = {}]: Assertion `!this->_future && this->_state && !this->_task' failed.
```
as seen in https://github.com/scylladb/scylla/issues/7085Fixes#7085
Test: unit(dev), database_test with injected error in posix_file_impl::close()
Signed-off-by: Benny Halevy <bhalevy@scylladb.com>
Message-Id: <20200826062456.661708-1-bhalevy@scylladb.com>
This patch adds tests for the "NewImage" attribute in Alternator Streams
in NEW_IMAGE and NEW_AND_OLD_IMAGES mode.
It reproduces issue #7107, that items' key attributes are missing in the
NewImage. It also verifies the risky corner cases where the new item is
"empty" and NewImage should include just the key, vs. the case where the
item is deleted, so NewImage should be missing.
This test currently passes on AWS DynamoDB, and xfails on Alternator.
Refs #7107.
Signed-off-by: Nadav Har'El <nyh@scylladb.com>
Message-Id: <20200825113857.106489-1-nyh@scylladb.com>
Never trust Occam's Razor - it turns out that the use-after-free bug in the
"exists" command was caused by two separate bugs. We fixed one in commit
9636a33993, but there is a second one fixed in
this patch.
The problem fixed here was that a "service_permit" object, which is designed to
be copied around from place to place (it contains a shared pointer, so is cheap
to copy), was saved by reference, and the reference was to a function argument
and was destroyed prematurely.
This time I tested *many times* that that test_strings.py passes on both dev and
debug builds.
Note that test/run/redis still fails in a debug build, but due to a different
problem.
Fixes#6469
Signed-off-by: Nadav Har'El <nyh@scylladb.com>
Reviewed-by: Benny Halevy <bhalevy@scylladb.com>
Message-Id: <20200825183313.120331-1-nyh@scylladb.com>
test/redis/README.md suggests that when running "pytest" the default is to connect
to a local redis on localhost:6379. This default was recently lost when options
were added to use a different host and port. It's still good to have the default
suggested in README.md.
It also makes it easier to run the tests against the standard redis, which by
default runs on localhost:6379 - by just running "pytest".
Signed-off-by: Nadav Har'El <nyh@scylladb.com>
Message-Id: <20200825195143.124429-1-nyh@scylladb.com>
query_options::linearize was the only user of _temporaries helper
attribute, and it turns out that this function is never used -
- and is therefore removed.
When a value_view needs to store a temporarily instantiated object,
it can use the new variant field. The temporary value will live
only as long as the view itself.
We saw errors in killed_wiped_node_cannot_join_test dtest:
Aug 2020 10:30:43 [node4] Missing: ['A node with address 127.0.76.4 already exists, cancelling join']:
The test does:
n1, n2, n3, n4
wipe data on n4
start n4 again with the same ip address
Without this patch, n4 will bootstrap into the cluster new tokens. We
should prevent n4 to bootstrap because there is an existing
node in the cluster.
In shadow round, the local node should apply the application state of
the node with the same ip address. This is useful to detect a node
trying to bootstrap with the same IP address of an existing node.
Tests: bootstrap_test.py
Fixes: #7073
Fixes#6900
Clustered range deletes did not clear out the "row_states" data associated
with affected rows (might be many).
Adds a sweep through and erases relevant data. Since we do pre- and
postimage in "order", this should only affect postimage.
Fix the default number of test repeats to 1, which it was before
(spotted by Nadav). Also, prefix the options so that they become
"--test-repeat" and "--test-timeout" (spotted by Avi).
Message-Id: <20200825081456.197210-1-penberg@scylladb.com>
It is useful to distinguish if the repair is a regular repair or used
for node operations.
In addition, log the keyspace and tables are repaired.
Fixes#7086
A check, to validate that counter column cannot be added into non-counter table,
is missing for alter table statement. Validation is performed when building new
schema, but it's limited to checking that a schema will not contain both counter
and non-counter columns.
Due to lack of validation, the added counter column could be incorrectly
persisted to the schema, but this results in a crash when setting the new
schema to its table. On restart, it can be confirmed that the schema change
was indeed persisted when describing the table.
This problem is fixed by doing proper validation for the alter table statement,
which consists of making sure a new counter column cannot be added to a
non-counter table.
The test cdc_disallow_cdc_for_counters_test is adjusted because one of its tests
was built on the assumption that counter column can be added into a non-counter
table.
Fixes#7065.
Signed-off-by: Raphael S. Carvalho <raphaelsc@scylladb.com>
Message-Id: <20200824155709.34743-1-raphaelsc@scylladb.com>
To verify offline installed image with do_verify_package() in scylla_setup, we introduce is_offline() function works just like is_nonroot() but the install not with --nonroot.
* syuu1228-do_verify_package_for_offline:
scylla_setup: verify package correctly on offline install
scylla_util.py: implement is_offline() to detect offline installed image
do_verify_package written only for .rpm/.deb, does not working correctly
for offline install(including nonroot).
We should check file existance for the environment, not by package existance
using rpm/dpkg.
Fixes#7075
A missing "&" caused the key stored in a long-living command to be copied
and the copy quickly freed - and then used after freed.
This caused the test test_strings.py::test_exists_multiple_existent_key for
this feature to frequently crash.
Fixes#6469
Signed-off-by: Nadav Har'El <nyh@scylladb.com>
Message-Id: <20200823190141.88816-1-nyh@scylladb.com>
Consider:
1) Start n1,n2,n3
2) Stop n3
3) Start n4 to replace n3 but list n4 as seed node
4) Node n4 finishes replacing operation
5) Restart n2
6) Run SELECT * from system.peers on node or node 1.
cqlsh> SELECT * from system.peers ;
peer| data_center | host_id| preferred_ip | rack | release_version | rpc_address | schema_version| supported_features| tokens
127.0.0.3 |null |null | null | null | null |null |null |null | {'-90410082611643223', '5874059110445936121'}
The replaced old node 127.0.0.3 shows in system.peers.
(Note, since commit 399d79fc6f (init: do not allow
replace-address for seeds), step 3 will be rejected. Assume we use a version without it)
The problem is that n2 sees n3 is in gossip status of SHUTDOWN after
restart. The storage_service::handle_state_normal callback is called for
127.0.0.3. Since n4 is using different token as n3 (seed node does not bootstrap so
it uses new tokens instead of tokens of n3 which is being replaced), so
owned_tokens will be set. We see logs like:
[shard 0] storage_service - handle_state_normal: New node 127.0.0.3 at token 5874059110445936121
[shard 0] storage_service - Host ID collision for cbec60e5-4060-428e-8d40-9db154572df7 between 127.0.0.4
and 127.0.0.3; ignored 127.0.0.3
As a result, db::system_keyspace::update_tokens
will be called to write to system.peers for 127.0.0.3 wrongly.
if (!owned_tokens.empty()) {
db::system_keyspace::update_tokens(endpoint, owned_tokens)
}
To fix, we should skip calling db::system_keyspace::update_tokens if the
nodes is present in endpoints_to_remove.
Refs: #4652
Refs: #6397
Just like test/alternator, make redis-test runnable from test.py.
For this we move the redis tests into a subdirectory of tests/,
and create a script to run them: tests/redis/run.
These tests currently fail, so we did not yet modify test.py to actually
run them automatically.
Fixes#6331
"
There's last call for global storage service left in compaction code, it
comes from cleanup_compaction to get local token ranges for filtering.
The call in question is a pure wrapper over database, so this set just
makes use of the database where it's already available (perform_cleanup)
and adds it where it's needed (perform_sstable_upgrade).
tests: unit(dev), nodetool upgradesstables
"
* 'br-remove-ss-from-compaction-3' of https://github.com/xemul/scylla:
storage_service: Remove get_local_ranges helper
compaction: Use database from options to get local ranges
compaction: Keep database reference on upgrade options
compaction: Keep database reference on cleanup options
db: Factor out get_local_ranges helper
This patch causes orphaned hints (hints that were written towards a node
that is no longer their replica) to be sent with a default write
timeout. This is what is currently done for non-orphaned hints.
Previously, the timeout was hardcoded to one hour. This could cause a
long delay while shutting down, as hints manager waits until all ongoing
hint sending operation finish before stopping itself.
Fixes: #7051
This reader was probably created in ancient times, when readers didn't
yet have a _schema member of their own. But now that they do, it is not
necessary to store the schema in the reader implementation, there is one
available in the parent class.
While at it also move the schema into the class when calling the
constructor.
Signed-off-by: Botond Dénes <bdenes@scylladb.com>
Reviewed-by: Benny Halevy <bhalevy@scylladb.com>
Message-Id: <20200821070358.33937-1-bdenes@scylladb.com>
"
We keep refrences to locator::token_metadata in many places.
Most of them are for read-only access and only a few want
to modify the token_metadata.
Recently, in 94995acedb,
we added yielding loops that access token_metadata in order
to avoid cpu stalls. To make that possible we need to make
sure they token_metadata object they are traversing won't change
mid-loop.
This series is a first step in ensuring the serialization of
updates to shared token metadata to reading it.
Test: unit(dev)
Dtest: bootstrap_test:TestBootstrap.start_stop_test{,_node}, update_cluster_layout_tests.py -a next-gating(dev)
"
* tag 'constify-token-metadata-access-v2' of github.com:bhalevy/scylla:
api/http_context: keep a const sharded<locator::token_metadata>&
gossiper: keep a const token_metadata&
storage_service: separate get_mutable_token_metadata
range_streamer: keep a const token_metadata&
storage_proxy: delete unused get_restricted_ranges declaration
storage_proxy: keep a const token_metadata&
storage_proxy: get rid of mutable get_token_metadata getter
database: keep const token_metadata&
database: keyspace_metadata: pass const locator::token_metadata& around
everywhere_replication_strategy: move methods out of line
replication_strategy: keep a const token_metadata&
abstract_replication_strategy: get_ranges: accept const token_metadata&
token_metadata: rename calculate_pending_ranges to update_pending_ranges
token_metadata: mark const methods
token_ranges: pending_endpoints_for: return empty vector if keyspace not found
token_ranges: get_pending_ranges: return empty vector if keyspace not found
token_ranges: get rid of unused get_pending_ranges variant
replication_strategy: calculate_natural_endpoints: make token_metadata& param const
token_metadata: add get_datacenter_racks() const variant
The cleanup compaction wants to keep local tokens on-board and gets
them from storage_service.get_local_ranges().
This method is the wrapper around database.get_keyspace_local_ranges()
created in previous patch, the live database reference is already
available on the descriptor's options, so we can short-cut the call.
This allows removing the last explicit call for global storage_service
instance from compaction code.
Signed-off-by: Pavel Emelyanov <xemul@scylladb.com>
The only place that creates them is the API upgrade_sstables call.
The created options object doesn't over-survive the returned
future, so it's safe to keep this reference there.
Signed-off-by: Pavel Emelyanov <xemul@scylladb.com>
The database is available at both places that create the options --
tests and API perform_cleanup call.
Options object doesn't over-survive the returned future, so it's
safe to keep the reference on it.
Signed-off-by: Pavel Emelyanov <xemul@scylladb.com>
Storage service and repair code have identical helpers to get local
ranges for keyspace. Move this helper's code onto database, later it
will be reused by one more place.
Signed-off-by: Pavel Emelyanov <xemul@scylladb.com>
Use a different getter for a token_metadata& that
may be changed so we can better synchronize readers
and writers of token_metadata and eventually allow
them to yield in asynchronous loops.
Signed-off-by: Benny Halevy <bhalevy@scylladb.com>
We'd like to strictly control who can modify token metadata
and nobody currently needs a mutable reference to storage_proxy::_token_metadata.
Signed-off-by: Benny Halevy <bhalevy@scylladb.com>
