We defined less for some types and compare for others. There is no
type for which compare is substantially more expensive, so define it
for all types and implement less with compare.
Signed-off-by: Rafael Ávila de Espíndola <espindola@scylladb.com>
The type walking is similar to what the find function does, but
refactoring it doesn't seem worth it if these are the only two uses.
Signed-off-by: Rafael Ávila de Espíndola <espindola@scylladb.com>
With this patch the logic for walking all nested types is moved to a
helper function. It also fixes reversed_type_impl not being handled in
references_duration.
Signed-off-by: Rafael Ávila de Espíndola <espindola@scylladb.com>
The api is inspired by on std::variant.
This bridges the runtime type of a abstract_type object to a compile
time overload resolution. For example, it is possible to have a single
lambda to visit a string_type_impl, but it corresponds to two leaf
types (ascii and utf8).
Signed-off-by: Rafael Ávila de Espíndola <espindola@scylladb.com>
The type hierarchy is closed, so we can give each leaf an enum value.
This will be used to implement a visitor pattern and reduce code
duplication.
Signed-off-by: Rafael Ávila de Espíndola <espindola@scylladb.com>
The corresponding code is correct, but I noticed no tests would fail
if it was broken while refactoring it.
Signed-off-by: Rafael Ávila de Espíndola <espindola@scylladb.com>
These only use public member functions from data_value, so there is no
reason for not making them public too.
Signed-off-by: Rafael Ávila de Espíndola <espindola@scylladb.com>
Since we do accept pull requests (in a long-running experiment), the
pull request template suggesting not to use them is inaccurate, and
many requesters forget to remove the boilerplace.
Remove the outdate template.
"Commit e3f7fe4 added file owner validation to prevent Scylla from
crashing when it tries to touch a file it doesn't own. However, under
docker, we cannot expect to pass this check since user IDs are from
different namespaces: the process runs in a container namespace, but the
data files usually come from a mounted volume, and so their uids are
from the host namespace.
So we need to relax the check. We do this by reverting b1226fb, which
causes Scylla to run as euid 0 in docker, and by special-casing euid 0
in the ownership verification step.
Fixes #4823."
* 'docker-euid-0' of git://github.com/avikivity/scylla:
main: relax file ownership checks if running under euid 0
Revert "dist/docker/redhat: change user of scylla services to 'scylla'"
During startup, we check that the data files are owned by our euid.
But in a container environment, this is impossible to enforce because
uid/username mappings are different between the host and the container,
and the data files are likely to be mounted from the host.
To allow for such environments, relax the checks if euid=0. This
both matches what happens in a container (programs run as root) and
the kernel access checks (euid 0 can do anything).
We can reconsider this when container uid mapping is better developed.
Fixes#4823.
Fixes#4536.
This reverts commit b1226fb15a. When the
data volume is mounted from the host (as is usual in container
deployments), we can't expect that the files will be owned by the
in-container scylla user. So that commit didn't really fix#4536.
A follow-up patch will relax the check so it passes in a container
environment.
This adds a '--configure-flags FLAGS' command line option, which
overrides the flags passed to scylla.git 'configure.py' script. We need
this for flexibility of custom builds in Jenkins pipelines, for example.
Message-Id: <20190813095428.13590-1-penberg@scylladb.com>
Make the reader recreation logic more robust, by moving away from
deciding which fragments have to be dropped based on a bunch of
special cases, instead replacing this with a general logic which just
drops all already seen fragments (based on their position). Special
handling is added for the case when the last position is a range
tombstone with a non full prefix starting position. Reproducer unit
tests are added for both cases.
Refs #4695Fixes#4733
We should wait for a future returned from add_local_application_state() to
resolve before issuing new calculation, otherwise two
add_local_application_state() may run simultaneously for the same state.
Fixes#4838.
Message-Id: <20190812082158.GE17984@scylladb.com>
Add `single_fragment_buffer` test variable. When set, the shard readers
are created with a max buffer size of 1, effectively causing them to
read a single fragment at a time. This, when combined with
`evict_readers=true` will stress the recreate reader logic to the max.
Be more tolerant with different but equivalent representation of range
deletions. When expecting a range tombstone, keep reading range
tombstones while these can be merged with the cumulative range
tombstone, resulting from the merging of the previous range tombstones.
This results in tests tolerating range tombstones that are split into
several, potentially overlapping range tombstones, representing the
same underlying deletion.
This is tailored to the multishard_combining_reader, to make sure it
doesn't loos rows following a range tombstone with a prefix starting
position (whose prefix their keys fall into).
Currently the remote reader uses the last seen fragment's position to
calculate the position the reader should continue from when the reader
is recreated after having been evicted. Recently it was discovered that
this logic breaks down badly when this last position is a non-full
clustering prefix (a range tombstone start bound). In this case, if only
the last position is available, there is no good way of computing the
starting position. Starting after this position will potentially miss
any rows that fall into the prefix (the current behaviour). Starting
from before it will cause all range tombstones with said prefix to be
re-emitted, causing other problems. A better solution is to exploit the
fact that sometimes we also know what the next fragment is.
These "some" times are the exact times that are problematic with the
current approach -- when the last fragment is a range tombstone.
Exploiting this extra knowledge allows for a much better way for
calculating the starting position: instead of maintaining the last
position, we maintain the next position, which is always safe to start
from. This is not always possible, but in many cases we can know for
sure what the next position is, for example if the last position was a
static row we can be sure the next position is the first clustering
position (or partition end). In the few cases where we cannot calculate
the next position we fall back to the previous logic and start from
*after* the last positions. The good news is that in these remaining
cases (the last fragment is a clustering row) it is safe to do so.
This patch also does some refactoring of the remote-reader internals,
all fill-buffer related logic is grouped together in a single
`fill_buffer()` method.
