The condition is incorrect after 9b52f5bf2b.
We no longer express the full range as (min, min], and missing upper
bound as bound as (x, min] so we don't need to exclude those cases in
the check.
Get values from cf->schema instead of using hardcoded threshold
values. In addition, move DEFAULT_MIN_COMPACTION_THRESHOLD and
DEFAULT_MAX_COMPACTION_THRESHOLD to schema.hh so as not to have
knowledge duplicated.
Signed-off-by: Raphael S. Carvalho <raphaelsc@cloudius-systems.com>
Since parsing involves a unique_ptr<metadata> holding a pointer to a
subclass of metadata, it must define a virtual destructor, or it can
cause memory leaks when deleted, or, with C++14 sized deallocators, it
can cause the wrong memory pool to be used for deleting the object.
Seen on EC2.
Define a virtual destructor to tell the compiler how to destroy
and free the object.
It is now necessary to close() a file before destroying it, otherwise a big
ugly warning message is printed by the reactor. Our sstable read path was
especially careless about closing the countless files it opens, and the
sstable test generated as many as 400 (!) of these warning messages, despite
running correctly. This patch adds the missing close() calls.
After this patch, the sstable test still shows 3 warning messages.
Those are unavoidable: They happen while broken sstables are being
tested, and an exception is thrown in the middle of the sstable processing,
causing us to destroy a file object without calling close() on it first.
This, in my opinion, proves that requiring close() in the read path is not
a good thing, it is un-RAII-like and not exception-safe. But it is benign
except the warning message, so whatever. 3 scary warning messages from the
test are better than 400...
If these 3 remaining messages really bother us, I guess we can fix it by
catching the exceptions in the sstable code, closing the file and rethrowing
the exception, but it will be quite ugly...
Signed-off-by: Nadav Har'El <nyh@cloudius-systems.com>
There is some missing information in the last log printout, because
it's currently hard to generate such information.
Anyway, this patch is a good start towards providing the same log
messages as origin.
Addresses issue #12
Signed-off-by: Raphael S. Carvalho <raphaelsc@cloudius-systems.com>
Instead of trying to second-guess the seastar build system, always rebuild
libseastar.a. Specify restat = 1 so that binaries are only relinked if
something truly changed.
Return a std::chrono::steady_clock::duration and switch the caller in
migration manager to also use proper C++ durations.
Reviewed-by: Nadav Har'El <nyh@cloudius-systems.com>
Signed-off-by: Pekka Enberg <penberg@cloudius-systems.com>
The idea is to reuse the same testing code on any mutation_source, for
example on memtable.
The range query test cases are now part of a generic mutation_source
test suite.
data_consume_rows(0, 0) was returning all partitions instead of no
partitions, because -1 was passed as count in such case, which was
then casted to uint64_t.
Special-casing it that way is problematic for code which calculates
the bounds, and when the key is not found we simple end up with 0 as
upper bound. Instead of convoluting the range lookup code to special
case for 0, let's simplify the interface so that (0, 0) returns no
rows, same as (1, 1). There is a new overload of data_consume_rows()
without bounds, which returns all data.
The functionality is similar to RuntimeMBean.getUptime() that's needed
in schema pulling logic.
Signed-off-by: Pekka Enberg <penberg@cloudius-systems.com>
Make storage proxy a singleton and add helpers to look up a reference.
This makes it easier to convert code from Origin in areas such as
storage service.
Signed-off-by: Pekka Enberg <penberg@cloudius-systems.com>
recently, "file" started to use a shared_ptr internally, and is already
copy-able and reference counted, and there is no reason to use
lw_shared_ptr<file>. This patch cleans up a few remaining places where
lw_shared_ptr<file> was used.
Signed-off-by: Nadav Har'El <nyh@cloudius-systems.com>
I will need those abstractions later to handle
inclusiveness/exclusiveness of both staring and ending bounds.
They're also familiar abstractions, so the code is hopefully easier to
comprehend now.
The entry contains not only the key, but other stuff like
position. Why would casting to bytes_view give the view on just the
key and not the whole entry. Better to be explicit.
If we had a range (x; ...] then x is excluded, but token iterator was
initialized with x. The splitting loop would exit prematurely because
it would detect that the token is outside the range.
The fix is to teach ring_range() to recognize this and always give
tokens which are not smaller than the range's lower bound.
