[
https://issues.apache.org/jira/browse/CASSANDRA-10130?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16019655#comment-16019655
]
Andrés de la Peña commented on CASSANDRA-10130:
-----------------------------------------------
Here is the updated 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. Why executing the
[preBuildTask|https://github.com/apache/cassandra/compare/trunk...adelapena:10130-trunk#diff-3f2c8994c4ff8748c3faf7e70958520dR389]
in another thread via {{executeBlocking}}, rather than just calling
{{Runnable#run}}?
Totally agree,
[changed|https://github.com/adelapena/cassandra/blob/627266a894d7a34c2c0c7d3226cd91c07fad5a6d/src/java/org/apache/cassandra/index/SecondaryIndexManager.java#L390].
bq. Currently, {{buildIndexesBlocking}} will end up rebuilding all indexes even
if just a single one failed, because {{markIndexBuilt}} is called in bulk at
the very end; I know this is in line with the previous behaviour, but wouldn't
it make sense to improve it in this issue?
I've just changed it to use a [per-builder
callback|https://github.com/adelapena/cassandra/blob/627266a894d7a34c2c0c7d3226cd91c07fad5a6d/src/java/org/apache/cassandra/index/SecondaryIndexManager.java#L416]
that flushes the indexes, marks them as built and logs the index building
completion message.
{quote}
Do we have tests checking:
* Index status pre and post (re)building actions (create index, rebuild index).
* Index status upon index removal.
* Index status and automatic rebuild in case of failures.
{quote}
Indeed, some more tests were needed:
* For unit tests, I have extended the already existent
[{{CassandraIndexTest#indexCorrectlyMarkedAsBuildAndRemoved}}|https://github.com/adelapena/cassandra/blob/627266a894d7a34c2c0c7d3226cd91c07fad5a6d/test/unit/org/apache/cassandra/index/internal/CassandraIndexTest.java#L510]
to cover the new cases.
* For dtests,
[{{sstable_generation_loading_test.py#sstableloader_with_failing_2i_test}}|https://github.com/adelapena/cassandra-dtest/blob/8da749e26a4712f4581cabf56c60df92aa65f066/sstable_generation_loading_test.py#L297]
covers index status with {{StreamReceiveTask}}. I have also added
[{{test_failing_manual_rebuild_index}}|https://github.com/adelapena/cassandra-dtest/blob/8da749e26a4712f4581cabf56c60df92aa65f066/secondary_indexes_test.py#L338-L396],
[{{test_drop_index_while_building}}|https://github.com/adelapena/cassandra-dtest/blob/8da749e26a4712f4581cabf56c60df92aa65f066/secondary_indexes_test.py#L398-L434]
and
[{{test_index_is_not_always_rebuilt_at_start}}|https://github.com/adelapena/cassandra-dtest/blob/8da749e26a4712f4581cabf56c60df92aa65f066/secondary_indexes_test.py#L436-L471]
to
[{{secondary_indexes_test.py#TestSecondaryIndexes}}|https://github.com/adelapena/cassandra-dtest/blob/8da749e26a4712f4581cabf56c60df92aa65f066/secondary_indexes_test.py].
{quote}
While inspecting usages of {{buildAllIndexesBlocking}} with the
{{preBuildTask}} parameter, I noticed that rebuilding indexes is a natural
consequence of adding new SSTables to the tracker - I don't see a situation
where we want to add SSTables to the tracker and NOT rebuild the indexes, so
instead of requiring users of {{Tracker.addSSTables}} to figure out they need
to rebuild indexes and create a dependency with the {{SecondaryIndexManager}}
(such as {{OnCompletionRunnable}} or {{ColumnFamilyStore.loadNewSSTables}}), or
even creating a dependency between {{Tracker.addSSTables}} and the secondary
index manager, we could leverage the tracker notification support and make the
secondary index automatically rebuild indexes when receiving an
{{SSTableAddedNotification}} from the tracker.
However this notification is only triggered *after* the SSTables are added to
the tracker, but there is a possibility that there is a failure after some
SSTables were already added and we would need to rebuild indexes in that case,
so we could maybe add a new {{SSTableBeforeAddedNotification}} (or better name)
that is triggered at the start of {{Tracker.addSSTables}}, mark the index as
building when receiving that notification and actually trigger
{{buildAllIndexesBlocking}} when receiving the {{SSTableAddedNotification}}.
{quote}
This would be similar to how SASI indexes work. However, I'm not sure about if
there are cases where a {{SSTableAddedNotification}} shouldn't trigger the
indexing of the notified SSTables. For example,
{{Tracker#replaceFlushed(Memtable, Iterable<SSTableReader>)}} sends a
{{SSTableAddedNotification}}. If I understand it right, this happens when a
memtable is flushed to disk. I guess that something similar happens when
SSTables are merged by compaction. Currently, these
{{SSTableAddedNotification}} s are not associated to an reindexing because the
added SSTables contain data that has been already indexed. What do you think?
> 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.3.15#6346)
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
