sstable parsing: assert we do not lose clustering rows

The sstable parsing code calls mp_row_consumer::flush() after every
clustering row has been read, and this puts the now complete row in a single
field "_ready". The assumption is that at this point parsing will stop, the
consumer will move out this _ready (mp_row_consumer::get_mutation_fragment())
and when flush() is later called again, _ready will be empty again.

This assumption is correct in our code, but is based on an intricate
combination of estoreric parts of the code, such as:

 1. In data_consume_row_context we stop parsing after reading the parition's
    header, before reading any clustering rows, giving the caller the chance
    to call sstable_streamed_mutation::read_next() to be prepared for the
    incoming mutations.

 2. In mp_row_consumer::flush_if_needed(), we stop the parser after each
    individual clustering row.

It is easy to break this assumption, and I did this in one of my code changes,
and the result was silent loss of clustering rows, as "_ready" got silently
overwritten before the reader had a chance to move it out.

What this patch does is to add an assertion: If a clustering row is silently
lost before being transferred to the mutation fragment reader, we croak.

Signed-off-by: Nadav Har'El <nyh@scylladb.com>
Message-Id: <1468389955-24600-1-git-send-email-nyh@scylladb.com>
This commit is contained in:
Nadav Har'El
2016-07-13 09:05:55 +03:00
committed by Paweł Dziepak
parent 4eca7632ec
commit aec90a22da

View File

@@ -320,6 +320,10 @@ public:
void flush() {
flush_pending_collection(*_schema);
// If _ready is already set we have a bug: get_mutation_fragment()
// was not called, and below we will lose one clustering row!
assert(!_ready);
assert(!_range_tombstone_end_ready);
if (!_skip_clustering_row) {
_ready = move_and_disengage(_in_progress);
_range_tombstone_end_ready = move_and_disengage(_range_tombstone_end_in_progress);