[
https://issues.apache.org/jira/browse/CASSANDRA-10130?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16050702#comment-16050702
]
Andrés de la Peña commented on CASSANDRA-10130:
-----------------------------------------------
Here is another version of the patch:
||[trunk|https://github.com/apache/cassandra/compare/trunk...adelapena:10130-trunk]|[utests|http://cassci.datastax.com/view/Dev/view/adelapena/job/adelapena-10130-trunk-testall/]|[dtests|http://cassci.datastax.com/view/Dev/view/adelapena/job/adelapena-10130-trunk-dtest/]|
bq. I mistakenly removed {{queryableIndexes]] in my refactor (as noted);
[this|https://github.com/adelapena/cassandra/commit/c5604ffd5ae36cc34d13af6c4c4eadba98458fa3]
commit added it back, but it's unfortunately not enough, as if the
initialization task fails, the index will never be made queryable: we need to
fix this, and we need a test for such failure case.
[Here|https://github.com/adelapena/cassandra/commit/2994aece1e936b81ad507f2e9baf0092e79edce0]
a successful full rebuild adds the index to the set of queryable indexes. I
have also added [some
tests|https://github.com/adelapena/cassandra/blob/2994aece1e936b81ad507f2e9baf0092e79edce0/test/unit/org/apache/cassandra/index/SecondaryIndexManagerTest.java#L386-L456]
to check the behaviour. Note that a partial build doesn't set the index as
queryable.
bq. Nit/OCD: I would change the
[rebuildFromSSTablesBlocking|https://github.com/adelapena/cassandra/commit/a945358a36f97632e94e9103650d72c04735354d#diff-3f2c8994c4ff8748c3faf7e70958520dR306]
signature to have the {{sstables}} first, consistently with
{{buildIndexesBlocking}}.
Initially done
[here|https://github.com/adelapena/cassandra/commit/7dfc959703231c9f90f53a3b474cf7f66b240213],
but see three quotes below.
bq. In {{buildIndexesBlocking}}, any exceptions thrown by
{{flushIndexesBlocking}} in the {{finally}} block will hide previous
exceptions: we should accumulate them.
Done
[here|https://github.com/adelapena/cassandra/commit/fbce3657132a8b87d2ff2446504634f5efdf2b59].
bq. Nit/OCD: In buildIndexesBlocking, we should put an additional comment
before invoking flushIndexesBlocking.
Done
[here|https://github.com/adelapena/cassandra/commit/b7bf81acd66c3bdac66ba86117e07cac251ca312].
bq. If {{rebuildFromSSTablesBlocking}} is not used elsewhere, I would strongly
recommend to make it private/protected and move its comment to
{{rebuildIndexesBlocking}}: the [general
rule|https://image.slidesharecdn.com/effectivejava-2ndedition-chapter4-151203200429-lva1-app6891/95/effective-java-chapter-4-classes-and-interfaces-4-638.jpg?cb=1449173201]
is methods (as well as attributes) should have the most restricted visibility
allowed by working code.
Actually {{rebuildFromSSTablesBlocking}} is only called by full rebuilds, so we
can eve get rid of this method and move its content and doc to
{{rebuildIndexesBlocking}}, as it is done
[here|https://github.com/adelapena/cassandra/commit/cdd32e54f86bd831faa4463489bad153520e2754].
Tests still have access to partial builds through
[{{handleNotification}}|https://github.com/adelapena/cassandra/blob/fbce3657132a8b87d2ff2446504634f5efdf2b59/test/unit/org/apache/cassandra/index/SecondaryIndexManagerTest.java#L115].
bq. Regarding the {{test_standalone_scrub failure}}, I believe it's due to the
fact we only manage counters in our marking methods when
{{DatabaseDescriptor.isDaemonInitialized()}}, which is obviously not the case
with the scrubber; if so, it's probably an easy fix.
Right, it's fixed
[here|https://github.com/adelapena/cassandra/commit/a2744560b2d3262b1872c5f989d1baee645b7c5c].
Apart form this,
[here|https://github.com/adelapena/cassandra/commit/70b477bc4c83613474756d39ac36c1301c4a54b2]
I have restored {{CassandraIndexTest#indexCorrectlyMarkedAsBuildAndRemoved}}
to its initial shape because all the added cases are (better) covered by
{{SecondaryIndexManagerTest}}. The reason to preserve the test is that it uses
a real index implementation instead of a mocked one, which can be useful
although the difficulties to simulate failures, blocking, etc.
I have also removed some unused imports.
> Node failure during 2i update after streaming can have incomplete 2i when
> restarted
> -----------------------------------------------------------------------------------
>
> Key: CASSANDRA-10130
> URL: https://issues.apache.org/jira/browse/CASSANDRA-10130
> Project: Cassandra
> Issue Type: Bug
> Components: Coordination
> Reporter: Yuki Morishita
> Assignee: Andrés de la Peña
> Priority: Minor
>
> Since MV/2i update happens after SSTables are received, node failure during
> MV/2i update can leave received SSTables live when restarted while MV/2i are
> partially up to date.
> We can add some kind of tracking mechanism to automatically rebuild at the
> startup, or at least warn user when the node restarts.
--
This message was sent by Atlassian JIRA
(v6.4.14#64029)
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
