"
There's a cql_type_parser::parse() method that needs to get user
types for a keyspace by its name. For this it uses the global
storage proxy instance as a place to get database from. This set
introduces an abstract user_types_storage helper object that's
responsible in providing the user types for the caller.
This helper, in turn, is provided to the parse() method by the
database itself or by the schema_ctxt object that needs parse()
to unfreeze schemas and doesn't have database at those times.
This removes one more get_storage_proxy() call.
"
* 'br-user-types-storage' of https://github.com/xemul/scylla:
cql_type_parser: Require user_types_storage& in parse()
schame_tables: Add db/ctxt args here and there
user_types: Carry storage on database and schema_ctxt
data_dictionary: Introduce user types storage
When pull_github_pr.sh uses git cherry-pick to merge a single-patch
pull request, this cherry-pick can fail. A typical example is trying
to merge a patch that has actually already been merged in the past,
so cherry-pick reports that the patch, after conflict resolution,
is empty.
When cherry-pick fails, it leaves the working directory in an annoying
mid-cherry-pick state, and today the user needs to manually call
"git cherry-pick --abort" to return to the normal state. The script
should it automatically - so this is what we do in this patch.
Signed-off-by: Nadav Har'El <nyh@scylladb.com>
When repair_meta stops it does so in the background and reports back
a shared future into whose shared promise peer it resolves that
background activity. There's a shorter way to forward a future result
into another, even shared, promise. And this method doesn't need to
discard a future.
tests: https://jenkins.scylladb.com/job/releng/job/Scylla-CI/253
Signed-off-by: Pavel Emelyanov <xemul@scylladb.com>
The lcs at those places are explicitly start()ed beforehand. The
is_start() check is necessary when using the latency_counter with a
histogram that may or may not start the counter (this is the case
in several class table methods).
tests: unit(dev)
Signed-off-by: Pavel Emelyanov <xemul@scylladb.com>
propagate_replacement() is an internal function that shouldn't be in
the public interface. No one besides an unit test for incremental
compaction needs it. In the future, I want to revisit incremental
compaction unit test to stop using it and only rely on public
interfaces
Signed-off-by: Raphael S. Carvalho <raphaelsc@scylladb.com>
Message-Id: <20220506171647.81063-1-raphaelsc@scylladb.com>
"
There's a enpoint->state map member of the gossiper class. First
ugly thing about it is that the member is public.
Next, there's a whole bunch of helpers around that map that export
various bits of information from it. All of those helpers reshard
to shard-0 to read from the state mape ignoring the fact that the
map is replicated on all shards internally. Also, some of those
helpers effectively duplicate each other for no real gain. Finally,
most of them are specific to api/ code, and open-coding them often
makes api/ handlers shorter and simpler.
This set removes the unused, api-only or trivial state map accessors
and marks the state map itself private (underscore prefix included).
tests: https://jenkins.scylladb.com/job/releng/job/Scylla-CI/233/
"
* 'br-gossiper-sanitize-api-2' of https://github.com/xemul/scylla:
gossiper: Add underscores to new private members
code: Indentation fix after previous patch
gossiper, code: Relax get_up/down/all_counters() helpers
api: Fix indentation after previous patch
gossiper, api: Remove get_arrival_samples()
gossiper, api: Remove get/set phi convict threshold helpers
gossiper, api: Move get_simple_states() into API code
gossiper: In-line std::optional<> get_endpoint_state_for_endpoint() overload
gossiper, api: Remove get_endpoint_state() helpers
gossiper: Make state and locks maps private
gossiper: Remove dead code
Attempting to call advance_to() on the index, after it is positioned at EOF, can result in an assert failure, because the operation results in an attempt to move backwards in the index-file (to read the last index page, which was already read). This only happens if the index cache entry belonging to the last index page is evicted, otherwise the advance operation just looks-up said entry and returns it. To prevent this, we add an early return conditioned on eof() to all the partition-level advance-to methods.
A regression unit test reproducing the above described crash is also added.
Fixes: #10403Closes#10491
* github.com:scylladb/scylla:
sstables/index_reader: short-circuit fast-forward-to when at EOF
test/lib/random_schema: add a simpler overload for fixed partition count
One user observed this assertion fail, but it's an extremely rare event.
The root cause - interlacing of processing STARTUP and OPTIONS messages -
is still there, but now it's harmless enough to leave it as is.
Fixes#10487Closes#10503
This series fixes a few issue on the table truncate path:
- "memtable_list: safely futurize clear_and_add"
- reinstates an async version of table::clear_and_add, just safe against #10421
- a unit test reproducing #10421 was added to make sure the new version is indeed safe.
- "table: clear: serialize with ongoing flush" fixes#10423
- a unit test reproducing #10423 was added
Fixes#10281Fixes#10423
Test: unit(dev), database_test. test_truncate_without_snapshot_during_{writes,flushes} (debug)
Closes#10424
* github.com:scylladb/scylla:
test: database_test: add test_truncate_without_snapshot_during_writes
memtable_list: safely futurize clear_and_add
table: clear: serialize with ongoing flush
primitive_consumer::read_bytes() destroys and creates a vector for every value it reads.
This happens for every cell.
We can save a bit of work by reusing the vector.
Closes#10512
* github.com:scylladb/scylla:
sstables: consumer: reuse the fragmented_temporary_buffer in read_bytes()
utils: fragmented_temporary_buffer: add release()
Dtest triggers the problem by:
1) creating table with LCS
2) disabling regular compaction
3) writing a few sstables
4) running maintenance compaction, e.g. cleanup
Once the maintenance compaction completes, disengaged optional _last_compacted_keys
triggers an exception in notify_completion().
_last_compacted_keys is used by regular for its round-robin file picking
policy. It stores the last compacted key for each level. Meaning it's
irrelevant for any other compaction type.
Regular compaction is responsible for initializing it when it runs for
the first time to pick files. But with it disabled, notify_completion()
will find it uninitialized, therefore resulting in bad_optional_access.
To fix this, the procedure is skipped if _last_compacted_keys is
disengaged. Regular compaction, once re-enabled, will be able to
fill _last_compacted_keys by looking at metadata of the files.
compaction_test.py::TestCompaction::test_disable_autocompaction_doesnt_
block_user_initiated_compactions[CLEANUP-LeveledCompactionStrategy]
now passes.
Fixes#10378.
Signed-off-by: Raphael S. Carvalho <raphaelsc@scylladb.com>
Closes#10508
SSTable was moved into descriptor, so on failure, it couldn't be used
without resulting in a segfault. Fix it by not moving sst, and changing
signature to make it explicit we don't want to move the content.
Fixes#10505.
Signed-off-by: Raphael S. Carvalho <raphaelsc@scylladb.com>
Closes#10506
These helpers count elements in the endpoint state map. It makes sense
to keep them in gossiper API, but it's worth removing the wrappers that
do invoke_on(0). This makes code shorter.
Signed-off-by: Pavel Emelyanov <xemul@scylladb.com>
The API method in question just tries to scan the state map. There's no
need in doing invoke_on(0) and in a separate helper method in gossiper,
the creation of the json return value can happen in the API handler.
Signed-off-by: Pavel Emelyanov <xemul@scylladb.com>
The method helps updating enpoint state in handle_major_state_change by
returning a copy of an endpoint state that's kept while the map's entry
is being replaced with the new state. It can be replaced with a shorter
code.
Signed-off-by: Pavel Emelyanov <xemul@scylladb.com>
There are two of them -- one to do invoke_on(0) the other one to get the
needed data. The former one is not needed -- the scanned endpoint state
map is replicated accross shards and is the same everywhere. The latter
is not needed, because there's only one user of it -- the API -- which
can work with the existing gossiper API.
Signed-off-by: Pavel Emelyanov <xemul@scylladb.com>
Locks are not needed outside gossiper, state map is sometimes read from,
but there a const getter for such cases. Both methods now desrve the
underbar prefix, but it doesn't come with this short patch.
Signed-off-by: Pavel Emelyanov <xemul@scylladb.com>
This series futurizes two synchronous functions used for data reconciliation:
`data_read_resolver::resolve` and `to_data_query_result` and does so
by introducing lower-level asynchronous infrastructure:
`mutation_partition_view::accept_gently`,
`frozen_mutation::unfreeze_gently` and `frozen_mutation::consume_gently`,
and `mutation::consume_gently`.
This trades some cycles on this cold path to prevent known reactor stalls.
Fixes#2361Fixes#10038Closes#10482
* github.com:scylladb/scylla:
mutation: add consume_gently
frozen_mutation: add consume_gently
query: coroutinize to_data_query_result
frozen_mutation: add unfreeze_gently
mutation_partition_view: add accept_gently methods
storage_proxy: futurize data_read_resolver::resolve
Attempting to call advance_to() on the index, after it is positioned at
EOF, can result in an assert failure, because the operation results in
an attempt to move backwards in the index-file (to read the last index
page, which was already read). This only happens if the index cache
entry belonging to the last index page is evicted, otherwise the advance
operation just looks-up said entry and returns it.
To prevent this, we add an early return conditioned on eof() to all the
partition-level advance-to methods.
A regression unit test reproducing the above described crash is also
added.
Currently, adding a cluster feature requires editing several files and
repeating the new feature name several times. This series reduces
the boilerplate to a single line (for non-experimental features), and
perhaps three for experimental features.
Closes#10488
* github.com:scylladb/scylla:
gms: feature_service: remove variable/helper function duplication
gms: feature: make `operator bool` implicit
gms: feature_service: remove feature variable duplication in enable()
gms: feature_service: remove feature variable declaration/definition duplication
gms: features: de-quadruplicate active feature names
gms: features: de-quadruplicate deprecated feature names
gms: feature_service: avoid duplicating feature names when listing known features
Reduce stalls by maybe yielding in-between partitions,
and by awaiting unfreeze_gently where possible.
Refs #10038
Signed-off-by: Benny Halevy <bhalevy@scylladb.com>
Allow yielding when consuming mutation_partition_view.
To be used in later patches by a new unfreeze_gently function
and frozen_mutation::consume.
Signed-off-by: Benny Halevy <bhalevy@scylladb.com>
Allow yielding in data_read_resolver::resolve to
prevent reactor stalls.
TODO: unfreeze_gently, to prevent stalls due
to large partitions.
Refs #2361
Signed-off-by: Benny Halevy <bhalevy@scylladb.com>
Right now to get user types the method in question gets global proxy
instance to get database from it and then peek a keyspace, its metadata
and, finally, the user types. There's also a safety check for proxy not
being initialized, which happens in tests.
Instead of messing with the proxy, the parse() method now accepts the
user_types_storage reference from which it gets the types. All the
callers already have the needed storage at hand -- in most of the cases
it's one shared between the database and schema_ctxt. In case of tests
is's a dummy storage, in case of schema-loader it's its local one.
The get_column_mapping() is special -- it doesn't expect any user-types
to be parsed and passes "" keyspace into it, neither it has db/ctxt to
get types storage from, so it can safely use the dummy one.
Signed-off-by: Pavel Emelyanov <xemul@scylladb.com>
This is to have them in places that call cql_type_parser::parse.
Pure churn reduction for the next patch.
Signed-off-by: Pavel Emelyanov <xemul@scylladb.com>
The user types storage is needed in cql_type_parser::parse which is in
turn called with either replica::database or scema_ctxt at hand.
To facilitate the former case replica::database has its own user types
storage created in database constructor.
The latter case is a bit trickier. In many cases the ctxt is created as
a temporary object and the database is available at those places. Also
the ctxt object lives on the schema_registry instance which doesn't have
database nearby. However, that ctxt lifetime is the same as the registry
instance one and when it's created there's a database at hand (it's the
database constructor that calls schema_registry.init() passing "this"
into it). Thus, the solution is to make database's user types storage be
a shared pointer that's shared between database itself and all the ctxts
out there including the one that lives on schema_registry instance.
When database goes away it .deactivate()s its user types storage so that
any ctxts that may share it stay on the safe side and don't use database
after free. This part will go away when the schema_registry will be
deglobalized.
Signed-off-by: Pavel Emelyanov <xemul@scylladb.com>
The interface in question will be used by cql type parser to get user
types. There are already three possible implementations of it:
- dummy, when no user types are in use (e.g. tests)
- schema-loader one, which gets user types from keyspaces that are
collected on its implementation of the database
- replica::database one, which does the same, but uses the real
database instance and that will be shared between scema_ctxts
Signed-off-by: Pavel Emelyanov <xemul@scylladb.com>
Each feature has a private variable and a public accessor. Since the
accessor effectively makes the variable public, avoid the intermediary
and make the variable public directly.
To ease mechanical translation, the variable name is chosen as
the function name (without the cluster_supports_ prefix).
References throughout the codebase are adjusted.
Features are usually used as booleans, so forcing allowing them
to implicitly decay to bool is not a mistake. In fact a bunch
of helper functions exist to cast feature variables to bool.
Prepare to reduce this boilerplate by allowing automatic conversion
to bool.
Active feature names are present four or five times in the code:
a delaration in feature.hh, a definition and initialization (two copies)
in feature_service.cc, a use in feature_service.cc, and a possible
reference in feature_service.cc if the feature is conditionally enabled.
Switch to just one copy or two, using the "foo"sv operator (and "foo"s)
to generate a string_view (string) as before.
Note that a few features had different external and C++ names; we
preserve the external name.
This patch does cause literal strings to be present in two places,
making them vulnerable to misspellings. But since feature names
are immutable, there is little risk that one will change without
the other.
Deprecated features are unused, but are present four times in the code:
a delaration in feature.hh, a definition and initialization (two copies)
in feature_service.cc, and a use in feature_service.cc. Switch to just
one copy, using the "foo"sv operator to generate a string_view as before.
Note that a few features had different external and C++ names; we
preserve the external name.
If we are redefining the log table, we need to ensure any dropped
columns are registered in "dropped_columns" table, otherwise clients will not
be able to read data older than now.
Includes unit test.
Should probably be backported to all CDC enabled versions.
Fixes#10473Closes#10474
