column_specification_or_tuple is now used internally, wrapping and
a column_specification or a vector and immediately unwrapping in
the callee. The only exceptions are bind_variable and tuple_constructor,
which handles both cases.
Use the underlying types directly instead, and add dispatching
to prepare_term_multi_column() for the two cases it handles.
column_specification_or_tuple was introduced since some terms
were prepared using a single receiver e.g. (receiver = <term>) and
some using multiple receivers (e.g. (r1, r2) = <term>. Some
term types supported both.
To hide this complexity, the term->expr conversion used a single
interface for both variations (column_expression_or_tuple), but now
that we got rid of the term class and there are no virtual functions
any more, we can just use two separate functions for the two variants.
Internally we still use column_expression_or_tuple, it can be
removed later.
The grammar now talks to expression API:s solely, so it can be converted
internally to expressions too. Calls to as_term_raw() and as_expression()
are removed, and productions return expressions instead of term::raw:s.
Change term::raw in multi_column_relation to expressions. Because a single
raw class is used to represent multiple shapes (IN ? and IN (x, y, z)),
some of the expressions are optional, corresponding to nullables before the
conversion.
to_term() is not converted, since it's part of the larger relation
hierarchy.
Change term::raw in single_column_relation to expressions. Because a single
raw class is used to represent multiple shapes (IN ? and IN (x, y, z)),
some of the expressions are optional, corresponding to nullables before the
conversion.
to_term() is not converted, since it's part of the larger relation
hierarchy.
Change term::raw in column_condition::raw to expressions. Because a single
raw class is used to represent multiple shapes (IN ? and IN (x, y, z)),
some of the expressions are optional, corresponding to nullables before the
conversion.
to_term() is not converted, since it's part of the larger relation
hierarchy.
Now that we have the prepare machinery exposed as expression API:s
(not just term::raw) we can avoid conversions from expressions to
term::raw when preparing subexpressions.
Convert the three variables in attrbutes::raw to expressions. Since
those attributes are optional, use std::optional to indicate it
(since we can't rely on shared_ptr<term::raw> being null).
The test_assignment class has a test_all() helper to test
a vector of assignment_testable. But expressions are not
derived from assignment_testable, so introduce a new helper
that does the same for expressions.
So far prepare (in the term domain) was called via term::raw. To be
able to prepare in the expression domain, expose functions prepare_term()
and test_assignment() that accept expressions as arguments.
prepare_term() was chosen rather that prepare() to differentiate wrt.
the other domain that can be prepared (selectables).
While we have a bridge between expressions and term::raw, which is
derived from assignment_testable, we will soon get rid of term::raw
and so won't be able to interface with API:s that require an
assignment_testable. So add a bridge for that. The user is
function::get(), which uses assignment_testable to infer the
function overload from the argument types.
Convert the user_types::literal raw to a new expression type
usertype_constructor. I used "usertype" to convey that is is a
((user type) constructor), not a (user (type constructor)).
We have this dependency now:
column_identifier -> selectable -> expression
and want to introduce this:
expression -> user types -> column_identifier
This leads to a loop, since expression is not (yet) forward
declarable.
Fix by moving any mention of expression from selectable.hh to a new
header selection-expr.hh.
database.cc lost access to timeout_config, so adjust its includes
to regain it.
Add set and map styles to collection_constructor. Maps are implemented as
collection_constructor{tuple_constructor{key, value}...}. This saves
having a new expression type, and reduces the effort to implement
recursive descent evaluation for this omitted expression type.
Move them closer to prepare related functions for modification. Since
sets and maps share some implementation details in the grammar, they
are moved and converted as a unit.
Introduce a collection_constructor (similar to C++'s std::initializer_list)
to hold subexpressions being gathered into a list. Since sets, maps, and
lists construction share some attributes (all elements must be of the
same type) collection_constructor will be used for all of them, so it
also holds an enum. I used "style" for the enum since it's a weak
attribute - an empty set is also an empty map. I chose collection_constructor
rather than plain 'collection' to highlight that it's not the only way
to get a collection (selecting a collection column is another, as an
example) and to hint at what it does - construct a collection from
more primitive elements.
Introduce tuple_constructor (not a literal, since (?, ?) and (column_value,
column_value) are not literals) to represent a tuple constructed from
subexpressions. In the future we can replace column_value_tuple
with tuple_constructor(column_value, column_value, ...), but this is
not done now.
I chose the name 'tuple_constructor' since other expressions can represent
tuples (e.g. my_tuple_column, :bind_variable_of_tuple_type,
func_returning_tuple()). It also explains what the expression does.
Introduce a new expression untyped_constant that corresponds to
constants::literal, which is removed. untyped_constant is rather
ugly in that it won't exist post-prepare. We should probably instead
replace it with typed constants that use the widest possible type
(decimal and varint), and select a narrower type during the prepare
phase when we perform type inference. The conversion itseld is
straightforward.
Convert the four forms of abstract_marker to expr::bind_variable (the
name was chosen since variable is the role of the thing, while "marker"
refers more to the grammar). Having four variants is unnecessary, but
this patch doesn't do anything about that.
We can only convert expressions to term::raw, not the subclass
abstract_marker::in_raw, so relax the types. They will all be converted
to expressions. Relaxing types isn't good, but the structure is enforced
now by the grammar (and dynamically using variant casts), and in the future
by a typecheck pass (which will allow us to remove the many variations
of markers).
null_literal (which is in the term::raw domain) will be converted to an
expression, so unnest the nested class null_value (which is in the term
domain and is not converted now).
We reuse the expr::cast type that was previously used for selectables.
When preparing, subexpressions are converted to term::raw; this will
be removed later.
These methods will be converted to the expression variant, and
it's impossible to do this while inlined due to #include cycles. In
any case, deinlining is better.
Since there is no type_cast.cc, and since they'll become part of
expr_term call chain soon, they're moved there, even though it seems
odd for this patch. It's a waste to create type_cast.cc just for those
three functions.
The cast expression has two operands: the subexpression to cast and the
type to cast to. Since prepared and unprepared expressions are the
same type, we don't have to do anything, but prepared and unprepared
types are different. So add a variant to be able to support both.
The reason the selectable->expression transformation did not need to
do this is that casts in a selector cannot accept a user defined type.
Note those casts also have different syntax and different execution,
so we'll have to choose whether to unify the two semantics, or whether
to keep them separate. This patch does not force anything (but does hint
at unification by not including any discriminant beyond the type's
rawness). The string representation matches the part of the grammar
it was derived from (or conversion back to CQL will yield wrong
results).
Remove cql3::functions::function_call::raw and replace it with
cql3::expr::function_call, which already existed from the selector
migration to expressions. The virtual functions implementing term::raw
are made free functions and remain in place, to ease migration and
review.
Note that preparing becomes a more complicated as it needs to
account for anonymous functions, which were not representable
in the previous structure (and still cannot be created by the
parser for the term::raw path).
The parser now wraps all its arguments with the term::raw->expr
bridge, since that's what expr::function_call expects, and in
turn wraps the function call with an expr->term::raw bridge, since
that's what the rest of the parser expects. These will disappear
when the migration completes.
Since expressions can nest, and since we won't covert everything at once,
add a way to store a term::raw as an expression. We can now have a
term::raw that is internally an expression, and an expression that is
implemented as term::raw.
A term_raw_expression is a term::raw that holds an expression. It will
be used to incrementally convert the source base to expressions, while
still exposing the result to the common interface of shared_ptr<term::raw>.
Now that the signatures of term::raw::prepare and multi_column_raw::prepare
are identical, we can eliminate multi_column_raw, replacing it with
term::raw where needed. In some cases we delete it from the inheritance chain
since we reach term::raw via a different base class.
Note that a dynamic_cast<> is eliminated, so we compenate for the addition
of runtime checks in the previous patch by the deletion of runtime checks
in this patch.
In order to replace the term::raw hierarchy with expressions,
we need to unify the signatures of term::raw::prepare() and
term::multi_column_raw::prepare(). This is because we'll only have
one expression type to represent both single values and tuples
(although, different subexpression types will may used).
The difference in the two prepare() signatures is the
`receiver` parameter - which is a (type, name) pair used
to perfom type inference on the expression being prepared,
with the name used to report errors. In a perfect world, this
would just be an expression - a tuple or a singular expression
as the case requires. But we don't have the needed expression
infrastructure yet - general tuples or name-annotated expressions.
Resolve the problem by introducing a variant for the single-value
and tuple. This is more or less creating a mini-expression type
used just for this. Once our expression type grows the needed
capabilities, it can replace this type.
Note that for some cases, this replaces compile-time checks by
runtime checks (which should never trigger). In other cases
the classes really needed both interfaces, so the new variant
is a better fit.
"
This series moves the timeout parameter, that is passed to most
f_m_r methods, into the reader_permit. This eliminates
the need to pass the timeout around, as it's taken
from the permit when needed.
The permit timeout is updated in certain cases
when the permit/reader is paused and retrieved
later on for reuse.
Following are perf_simple_query results showing ~1%
reduction in insns/op and corresponding increase in tps.
$ build/release/test/perf/perf_simple_query -c 1 --operations-per-shard 1000000 --task-quota-ms 10
Before:
102500.38 tps ( 75.1 allocs/op, 12.1 tasks/op, 45620 insns/op)
After:
103957.53 tps ( 75.1 allocs/op, 12.1 tasks/op, 45372 insns/op)
Test: unit(dev)
DTest:
repair_additional_test.py:RepairAdditionalTest.repair_abort_test (release)
materialized_views_test.py:TestMaterializedViews.remove_node_during_mv_insert_3_nodes_test (release)
materialized_views_test.py:InterruptBuildProcess.interrupt_build_process_with_resharding_half_to_max_test (release)
migration_test.py:TTLWithMigrate.big_table_with_ttls_test (release)
"
* tag 'reader_permit-timeout-v6' of github.com:bhalevy/scylla:
flat_mutation_reader: get rid of timeout parameter
reader_concurrency_semaphore: use permit timeout for admission
reader_concurrency_semaphore: adjust reactivated reader timeout
multishard_mutation_query: create_reader: validate saved reader permit
repair: row_level: read_mutation_fragment: set reader timeout
flat_mutation_reader: maybe_timed_out: use permit timeout
test: sstable_datafile_test: add sstable_reader_with_timeout
reader_permit: add timeout member
With data segregation on repair, thousands of sstables are potentially
added to maintenance set which causes high latency due to stalls.
That's because N*M sstables are created by a repair,
where N = # of ranges
and M = # of segregations
For TWCS, M = # of windows.
Assuming N = 768 and M = 20, ~15k sstables end up in sstable set
To fix this problem, let's avoid performing data segregation in repair,
as offstrategy will already perform the segregation anyway.
So from now on, only N non-overlapping sstables will be added to set.
Read amplification isn't affected because a query will only touch one
sstable in maintenance set.
When offstrategy starts, it will pick all sstables from set and
compact them in a single step while performing data segregation,
so data is properly laid out before integrated into the main set.
tests:
- sstable_compaction_test.twcs_reshape_with_disjoint_set_test
- mode(dev)
- manual test using repair-based bootstrap
Fixes#9199.
Signed-off-by: Raphael S. Carvalho <raphaelsc@scylladb.com>
Message-Id: <20210824185043.76475-1-raphaelsc@scylladb.com>
Current code std::move()-s the range tombstone into consumer thus
moving the tombstone's linkage to the containing list as well. As
the result the orignal range tombstone itself leaks as it leaves
the tree and cannot be reached on .clear(). Another danger is that
the iterator pointing to the tombstone becomes invalid while it's
then ++-ed to advance to the next entry.
The immediate fix is to keep the tombstone linked to the list while
moving.
fixes: #9207
Signed-off-by: Pavel Emelyanov <xemul@scylladb.com>
Message-Id: <20210825100834.3216-1-xemul@scylladb.com>
