Commit Graph

13849 Commits

Author SHA1 Message Date
Avi Kivity
060e5d3354 Merge "Improve time-series performance by not actually compacting fully expired tables" from Raphael
"In time-series, it's common for tables in a given time window to be eventually
fully expired. The deletion of such tables is done by compaction, but there's
*no* need to *actually* compact such fully expired sstables *iff* their full
deletion will not cause older data to be ressurected. In other words, a fully
expired table can be actually skipped (but deleted in the end) by compaction
*iff* it doesn't contain newer data than its overlapping counterparts. So there
may be false negatives, but never false positives.
All that said, the goal behind this patchset is to save read bandwidth of disk
in such scenarios. Given that fully expired sstables will not be read by
compaction process anymore, read amplification will be greatly reduced too.

Fixes #2620."

* 'time_series_performance_improvement_v2_2' of github.com:raphaelsc/scylla:
  tests: check sstable auto correct bad max deletion time
  tests: add test for compaction with fully expired table
  sstables/compaction: do not actually compact fully expired sstables
  sstables: make sstable auto correct max_local_deletion_time
  sstables: switch to const ref wherever possible
  sstables: use gc_clock::time_point for gc_before
  gc_clock: introduce operator<<(ostream&, gc_clock::time_point)
  sstables: introduce sstable::get_max_local_deletion_time
  sstables: remove unnecessary copy in time series strategies
  sstables: change return value type of get_fully_expired_sstables
  dtcs: make code to extract non expired tables faster
  sstables: add has_correct_max_deletion_time to sstable
2017-12-07 10:29:31 +02:00
Avi Kivity
908daa67bd Merge "Generalize data_resource" from Jesse
"Soon we will have resources beyond just keyspaces and table names. There
will be resources for roles, for user-defined functions (UDFs), and
possible resources for REST end-points. This change generalizes the
implementation of a `data_resource` to many different kinds of
resources, though there is still only one kind (`data`).

The most important patch is 2/5 ("auth/resource: Generalize to different
kinds"), which re-writes `auth::data_resource`. The patch message should
sufficiently explain the design decisions involved.

The other patches rename files and identifiers based on the expanded
role of this class, except for 5/5 ("auth/resource.hh: Rename
`resource_ids`"): this patch gives a more appropriate name to a type
alias.

Fixes #3027."

* 'jhk/generalize_resource/v3' of https://github.com/hakuch/scylla:
  auth/resource.hh: Rename `resource_ids`
  auth: Rename `data_resource` files
  cql3/authorization_statement: Fix typo
  auth/resource: Generalize to different kinds
  auth: Rename `data_resource` to `resource`
2017-12-07 10:25:58 +02:00
Botond Dénes
9fce51f8a0 Add streamed mutation fast-forwarding unit test for the flat combined-reader
Test for the bug fixed by 9661769.

Signed-off-by: Botond Dénes <bdenes@scylladb.com>
Message-Id: <fc917bae8e9c99f026bf7b366e6e9d39faf466af.1512630741.git.bdenes@scylladb.com>
2017-12-07 09:45:12 +02:00
Raphael S. Carvalho
5eef7371b3 tests: check sstable auto correct bad max deletion time
Signed-off-by: Raphael S. Carvalho <raphaelsc@scylladb.com>
2017-12-06 19:52:33 -02:00
Raphael S. Carvalho
a86ee38638 tests: add test for compaction with fully expired table
Signed-off-by: Raphael S. Carvalho <raphaelsc@scylladb.com>
2017-12-06 19:52:33 -02:00
Raphael S. Carvalho
809b30c4a2 sstables/compaction: do not actually compact fully expired sstables
There's no need to actually compact a sstable which is fully expired
and which deletion of all its data will not ressurect older data.
For that, a sstable will only be considered fully expired if it
doesn't contain data newer than its overlapping counterparts.
That way, there could be a false negative, but never a false positive.
Currently, a fully expired sstable would unnecessarily waste read
bandwidth of disk. This will help a lot time series workloads in
which data for a given time window is all deleted at once using TTL.

Fixes #2620.

Signed-off-by: Raphael S. Carvalho <raphaelsc@scylladb.com>
2017-12-06 19:52:33 -02:00
Raphael S. Carvalho
810e2ec3d9 sstables: make sstable auto correct max_local_deletion_time
sstables created prior to cc6c383 can contain bad max deletion time stat,
which would make get_fully_expired_sstables return sstables that aren't
actually fully expired. Let's make sstable invalidate the stat if it
is potentially incorrect.

Signed-off-by: Raphael S. Carvalho <raphaelsc@scylladb.com>
2017-12-06 19:52:33 -02:00
Raphael S. Carvalho
d2ab154f12 sstables: switch to const ref wherever possible
Signed-off-by: Raphael S. Carvalho <raphaelsc@scylladb.com>
2017-12-06 19:52:33 -02:00
Raphael S. Carvalho
d916c8cdad sstables: use gc_clock::time_point for gc_before
Signed-off-by: Raphael S. Carvalho <raphaelsc@scylladb.com>
2017-12-06 19:52:33 -02:00
Raphael S. Carvalho
1d0e6496ec gc_clock: introduce operator<<(ostream&, gc_clock::time_point)
Signed-off-by: Raphael S. Carvalho <raphaelsc@scylladb.com>
2017-12-06 19:52:32 -02:00
Raphael S. Carvalho
fcdce38e7f sstables: introduce sstable::get_max_local_deletion_time
Signed-off-by: Raphael S. Carvalho <raphaelsc@scylladb.com>
2017-12-06 18:47:05 -02:00
Raphael S. Carvalho
18bdf496fe sstables: remove unnecessary copy in time series strategies
Signed-off-by: Raphael S. Carvalho <raphaelsc@scylladb.com>
2017-12-06 18:46:46 -02:00
Raphael S. Carvalho
45c11865fa sstables: change return value type of get_fully_expired_sstables
unordered_set will allow us to quickly extract fully expired tables
from a set of compacting sstables.

Signed-off-by: Raphael S. Carvalho <raphaelsc@scylladb.com>
2017-12-06 18:45:55 -02:00
Raphael S. Carvalho
4fe6fea758 dtcs: make code to extract non expired tables faster
since it's O(n) and not O(n log n).

change also needed for change in interface of function to retrieve
fully expired tables, or sort lambda would need to be parametrized.

Signed-off-by: Raphael S. Carvalho <raphaelsc@scylladb.com>
2017-12-06 18:40:16 -02:00
Raphael S. Carvalho
11176324bd sstables: add has_correct_max_deletion_time to sstable
Commit cc6c38324 fixes the stat. It was only updated for range
tombstone prior to fix, so a sstable that had a regular cell with
no expiration time could be considered fully expired which can
lead to bad decisions in compaction for time series workloads.

Signed-off-by: Raphael S. Carvalho <raphaelsc@scylladb.com>
2017-12-06 18:40:05 -02:00
Jesse Haber-Kucharsky
aea262cdc4 auth/resource.hh: Rename resource_ids 2017-12-06 14:39:40 -05:00
Jesse Haber-Kucharsky
3cad18631d auth: Rename data_resource files
Now that there can be many kinds of resources, the old name doesn't fit.
2017-12-06 14:39:40 -05:00
Jesse Haber-Kucharsky
3665261a90 cql3/authorization_statement: Fix typo 2017-12-06 14:39:40 -05:00
Jesse Haber-Kucharsky
1bb22bb190 auth/resource: Generalize to different kinds
This change generalizes the implementation of a `resource` to many
different kinds of resources, though there is still only one
kind (`data`). In the future, we also expect resource kinds for roles,
user-defined functions (UDFs), and possibly on particular REST
end-points.

I considered several approaches to generalizing to different kinds of
resources.

One approach is to have a base class that is inherited from by different
resource kinds. The common functionality would be accessed through
virtual member functions and kind-specific functions would exist in
sub-classes. I rejected this approach because dealing with different
kinds of resources uniformly requires storage and life-time management
through something like `std::unique_ptr<auth::resource>`, which means
that we lose value semantics (including comparison) and must deal with
complications around ownership.

Another option was to use `boost::variant` (or, in future,
`std::variant`). This is closer to what we want, since there a static
set of resource kinds that we support. I rejected this approach for two
reasons. The first is that all resource kinds share the same data (a
list of segments and a root identifier), which would be duplicated in
each type that composed the variant. The second is that the complexity
and source-code overhead of `boost::variant` didn't seem warranted.

The solution I ended up with is home-grown variant. All resources are
described in the same `final` class: `auth::resource`. This class has
value semantics, supports equality comparison, and has a strict
ordering. All resources have in common a tag ("kind") and a list of
parts. Most operations on resources don't care about the kind of
resource (like getting its name, parsing a name, querying for the
parent, etc). These are just member functions of the class.

When we care about a kind-specific interpretation of a resource, we can
produce a "view" of the resource. For example, `data_resource_view`
allows for accessing the (optional) keyspace and table names.

I anticipate in the future to add functions for creating role
resources (`auth::resource::role`) and also `role_resource_view`.

The functional behaviour of the system should be unchanged with this
patch.

I've added new unit tests in `auth_resource_test.cc` and removed the old
test from `auth_test.cc`.

Fixes #3027.
2017-12-06 14:37:56 -05:00
Jesse Haber-Kucharsky
8fe53ecf78 auth: Rename data_resource to resource
The implementation and interface of `auth::resource` will change soon to
support different kinds of resources beyond just data (keyspaces and
tables).
2017-12-06 10:18:05 -05:00
Gleb Natapov
ddf117535a storage_proxy: add counters for speculative reads
Fixes #3030

Message-Id: <20171206143611.8756-1-gleb@scylladb.com>
2017-12-06 16:38:16 +02:00
Avi Kivity
ccc315bcfe Merge "storage_proxy: allow fail request earlier if CL cannot be reached due to errors" from Gleb
"This is CASSANDRA-7886 and CASSANDRA-8592. The patch series detects
that CL of a request can no longer be reached due to errors and fails
the request earlier. New type of errors are reported: read/write failure
which were introduced in cql v4 protocol. For compatibility if older
protocol is used the error is translated to timeout error."

* 'gleb/request-failure_v2' of github.com:scylladb/seastar-dev:
  storage_proxy: fail read/write requests early if it cannot be completed due to errors
  storage_service: add WRITE_FAILURE_REPLY_FEATURE feature
  gossiper: add node_has_feature() function
  cql: add read/write failure exceptions
  storage_proxy: fix data presence reporting in read timeout error during
  storage_proxy: remove inheritance from enable_shared_from_this for abstract_write_response_handler
  storage_proxy: remove unneeded field in abstract_write_response_handler
  storage_proxy: fix pending endpoint accounting for EACH_QUORUM
  consistency_level: constify quorum_for() and local_quorum_for()
2017-12-06 16:17:19 +02:00
Botond Dénes
9661769313 combined_mutation_reader: fix fast-fowarding related row-skipping bug
When fast forwarding is enabled and all readers positioned inside the
current partition return EOS, return EOS from the combined-reader
too. Instead of skipping to the next partition if there are idle readers
(positioned at some later partition) available. This will cause rows to
be skipped in some cases.

The fix is to distinguish EOS'd readers that are only halted (waiting
for a fast-forward) from thoose really out of data. To achieve this we
track the last fragment-kind the reader emitted. If that was a
partition-end then the reader is out of data, otherwise it might emit
more fragments after a fast-forward. Without this additional information
it is impossible to determine why a reader reached EOS and the code
later may make the wrong decision about whether the combined-reader as
a whole is at EOS or not.
Also when fast-forwarding between partition-ranges or calling
next_partition() we set the last fragment-kind of forwarded readers
because they should emit a partition-start, otherwise they are out of
data.

Signed-off-by: Botond Dénes <bdenes@scylladb.com>
Message-Id: <6f0b21b1ec62e1197de6b46510d5508cdb4a6977.1512569218.git.bdenes@scylladb.com>
2017-12-06 16:09:05 +02:00
Takuya ASADA
aeb6ebce5a dist/debian: need apt-get update after installing GPG key for 3rdparty repo
We need apt-get update after install GPG key, otherwise we still get
unauthenticated package error on Debian package build.

Signed-off-by: Takuya ASADA <syuu@scylladb.com>
Message-Id: <1512556948-29398-1-git-send-email-syuu@scylladb.com>
2017-12-06 12:43:17 +02:00
Jesse Haber-Kucharsky
772b432345 auth: Copying role exceptions cannot throw
This is a small correctness change.

According to cppreference.com [1], derived classes of `std::exception`
are not permitted to throw exceptions when they are copied.

To satisfy this requirement for `auth::roles_argument_exception`, we
store exception members as `std::shared_ptr` which has a `noexcept` copy
ctor. Since exceptions can cross shards, we cannot use a
`seastar::shared_ptr`.

This change is motivated by #3021.

[1] http://en.cppreference.com/w/cpp/error/exception/exception

Signed-off-by: Jesse Haber-Kucharsky <jhaberku@scylladb.com>
Message-Id: <7706df0c701b90e7cb309c84a86d9f813461e801.1512501024.git.jhaberku@scylladb.com>
2017-12-06 09:42:45 +01:00
Vladimir Krivopalov
1fc0c60fdc Support "CREATE TABLE WITH id" command.
Fixes #2059

Signed-off-by: Vladimir Krivopalov <vladimir@scylladb.com>
Message-Id: <92874a2bf1b4e79ef9f05875b3fa42804d17833c.1512508924.git.vladimir@scylladb.com>
2017-12-06 09:39:56 +01:00
Takuya ASADA
8f02967a3b dist/debian: install CA certificates before install repo GPG key
Since pbuilder chroot environment does not install CA certificates by default,
accessing https://download.opensuse.org will cause certificate verification
error.
So we need to install it before installing 3rdparty repo GPG key.

Also, checking existance of gpgkeys_curl is not needed, since it's always
not installed since we are running the script in clean chroot environment.

Signed-off-by: Takuya ASADA <syuu@scylladb.com>
Message-Id: <1512517001-27524-1-git-send-email-syuu@scylladb.com>
2017-12-06 08:57:01 +01:00
Avi Kivity
3501c147b7 Merge "Use new recommended classes from JsonCpp instead of deprecated ones" from Vladimir
"This fix for the issue #2989 first adds unit tests for caching_options which
is the only class that uses the helpers from json.hh. This is done to
have regression tests in place for the main change.
The second commit adds conditional use of new recommended JsonCpp API
where available. For older versions of the library, it uses the old
code."

* 'issues/2989/v1' of https://github.com/argenet/scylla:
  Use CharReaderBuilder/CharReader and StreamWriterBuilder from JsonCpp.
  tests: Add unit tests for caching_options.
2017-12-06 09:11:40 +02:00
Avi Kivity
601a03dda7 Merge "Make sstable tests use flat_mutation_reader" from Paweł
"This series makes sstable tests use flat stream interface. The main
motivation is to allow eventual removal of mutation_reader and
streamed_mutation and ensuring that the conversion between the
interfaces doesn't hide any bugs that would be otherwise found."

* tag 'flat_mutation_reader-sstable-tests/v1' of https://github.com/pdziepak/scylla:
  sstables: drop read_range_rows()
  tests/mutation_reader: stop using read_range_rows()
  incremental_reader_selector: do not use read_range_rows()
  tests/sstable: stop using read_range_rows()
  sstables: drop read_row()
  tests/sstables: use read_row_flat() instead of read_row()
  database: use read_row_flat() instead of read_row()
  tests/sstable_mutation_test: get flat_mutation_readers from mutation sources
  tests/sstables: make sstable_reader return flat_mutation_reader
  sstable: drop read_row() overload accepting sstable::key
  tests/sstable: stop using read_row() with sstable::key
  tests/flat_mutation_reader_assertions: add has_monotonic_positions()
  tests/flat_mutation_reader_assertions: add produces(Range)
  tests/flat_mutation_reader_assertions: add produces(mutation)
  tests/flat_mutation_reader_assertions: add produces(dht::decorated_key)
  tests/flat_mutation_reader_assertions: add produces(mutation_fragment::kind)
  tests/flat_mutation_reader_assertions: fix fast forwarding
2017-12-05 18:10:43 +02:00
Vladimir Krivopalov
b35c2fe177 Attach backtrace to marshal_exception-s thrown from generic functions.
Signed-off-by: Vladimir Krivopalov <vladimir@scylladb.com>
Message-Id: <06ad18c3563855771dd3ea8d0ec99533642e1919.1511931828.git.vladimir@scylladb.com>
2017-12-05 16:14:55 +01:00
Paweł Dziepak
0d8f964a79 sstables: drop read_range_rows()
It has been deprecated by read_range_rows_flat().
2017-12-05 14:53:14 +00:00
Paweł Dziepak
0c50f113c8 tests/mutation_reader: stop using read_range_rows() 2017-12-05 14:53:14 +00:00
Paweł Dziepak
ce9a890940 incremental_reader_selector: do not use read_range_rows() 2017-12-05 14:53:14 +00:00
Paweł Dziepak
15ad148604 tests/sstable: stop using read_range_rows()
read_range_rows() is deprecated by read_range_rows_flat().
2017-12-05 14:53:14 +00:00
Paweł Dziepak
e739ad98e5 sstables: drop read_row() 2017-12-05 14:53:14 +00:00
Paweł Dziepak
de8ebd6752 tests/sstables: use read_row_flat() instead of read_row() 2017-12-05 14:53:14 +00:00
Paweł Dziepak
bccca90207 database: use read_row_flat() instead of read_row() 2017-12-05 14:52:57 +00:00
Paweł Dziepak
582bacbd81 tests/sstable_mutation_test: get flat_mutation_readers from mutation sources 2017-12-05 14:52:32 +00:00
Paweł Dziepak
74e1c38f80 tests/sstables: make sstable_reader return flat_mutation_reader 2017-12-05 14:52:32 +00:00
Paweł Dziepak
7fce7a9e3a sstable: drop read_row() overload accepting sstable::key
sstable::key needs to be converted to a dht::decorated_key which needs
to be kept alive until the returned reader dies.
2017-12-05 14:49:25 +00:00
Paweł Dziepak
77a4231147 tests/sstable: stop using read_row() with sstable::key 2017-12-05 14:47:46 +00:00
Paweł Dziepak
52c1e9fcf4 tests/flat_mutation_reader_assertions: add has_monotonic_positions()
has_monotonic_positions() verifies that the stream is monotonic.
Based on streamed_mutation_assertions::has_monotonic_positions().
2017-12-05 14:47:46 +00:00
Paweł Dziepak
5b6f680b45 tests/flat_mutation_reader_assertions: add produces(Range)
The assertions already have produces(mutation) and
produces(dht::decorated_key) overloads. Additional overload that accepts
a range of elements will allow to check if a range of mutations of
decorated keys is produced.
The same interface is exposed by mutation_reader_assertions.
2017-12-05 14:47:46 +00:00
Paweł Dziepak
ef4fa1a8c1 tests/flat_mutation_reader_assertions: add produces(mutation) 2017-12-05 14:47:31 +00:00
Gleb Natapov
16964de1f3 storage_proxy: fail read/write requests early if it cannot be completed due to errors
If errors make reaching CL impossible a request can be aborted earlier
without waiting for timeout.
2017-12-05 16:46:25 +02:00
Gleb Natapov
0be3bd383b storage_service: add WRITE_FAILURE_REPLY_FEATURE feature
Presence of the flag indicates that the node is ready to process
negative mutation write replies.
2017-12-05 16:46:25 +02:00
Paweł Dziepak
d2dfca458f tests/flat_mutation_reader_assertions: add produces(dht::decorated_key)
There is an equivalent member function in mutation_reader assertions.
2017-12-05 13:11:55 +00:00
Paweł Dziepak
28caa76c8c tests/flat_mutation_reader_assertions: add produces(mutation_fragment::kind)
produces(mutation_fragment::kind) is provided by
streamed_mutation_assertions and is going to be needed in order to
fully convert tests to the flat mutation readers.
2017-12-05 13:04:16 +00:00
Paweł Dziepak
21886b7a3f tests/flat_mutation_reader_assertions: fix fast forwarding
Both fast_forward_to() overloads return a future which should be waited
for. Additionally, fast_forward_to(const dht::partition_range&) expects
the range to remain valid at least until the next call to
fast_forward_to(). The original mutation_reader_assertions guaranteed
that and so should flat_mutation_reader_assertions.
2017-12-05 13:04:16 +00:00
Gleb Natapov
fb8a626813 gossiper: add node_has_feature() function
The function allows to check if an endpoint supports certain feature.
2017-12-05 15:02:17 +02:00