[ 
https://issues.apache.org/jira/browse/CASSANDRA-10130?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16008617#comment-16008617
 ] 

Andrés de la Peña edited comment on CASSANDRA-10130 at 5/12/17 9:04 PM:
------------------------------------------------------------------------

I have updated the patch to use counters, as suggested by [~pauloricardomg]:

||[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/]|

To complement 
[{{SecondaryIndexManager#markIndexBuilt}}|https://github.com/adelapena/cassandra/blob/55c1fbad6116b8a8b9ccc86ce9fa50396d6bc522/src/java/org/apache/cassandra/index/SecondaryIndexManager.java#L421]
 and 
[{{SecondaryIndexManager#markIndexRemoved}}|https://github.com/adelapena/cassandra/blob/55c1fbad6116b8a8b9ccc86ce9fa50396d6bc522/src/java/org/apache/cassandra/index/SecondaryIndexManager.java#L432]
 I have added a new method 
[{{SecondaryIndexManager#markIndexBuilding}}|https://github.com/adelapena/cassandra/blob/55c1fbad6116b8a8b9ccc86ce9fa50396d6bc522/src/java/org/apache/cassandra/index/SecondaryIndexManager.java#L410]
 to be called before any index (re)building. When this method is called it 
increases a 
[{{SecondaryIndexManager.PendingIndexBuildsCounter}}|https://github.com/adelapena/cassandra/blob/55c1fbad6116b8a8b9ccc86ce9fa50396d6bc522/src/java/org/apache/cassandra/index/SecondaryIndexManager.java#L1216]
 associated to the index, and if the counter value is zero it removes the index 
from the {{IndexInfo}} table. Later, after the index (re)building has 
successfully ended, a call to the modified 
[{{SecondaryIndexManager#markIndexBuilt}}|https://github.com/adelapena/cassandra/blob/55c1fbad6116b8a8b9ccc86ce9fa50396d6bc522/src/java/org/apache/cassandra/index/SecondaryIndexManager.java#L421]
 will decrease the counter and, if it is zero, it will add the index to the 
{{IndexInfo}} table.

I'm not sure about if the assert in 
[{{SecondaryIndexManager.PendingIndexBuildsCounter#decrease()}}|https://github.com/adelapena/cassandra/blob/55c1fbad6116b8a8b9ccc86ce9fa50396d6bc522/src/java/org/apache/cassandra/index/SecondaryIndexManager.java#L1236]
 is too aggressive. The idea is to easily detect calls to 
[{{SecondaryIndexManager#markIndexBuilt}}|https://github.com/adelapena/cassandra/blob/55c1fbad6116b8a8b9ccc86ce9fa50396d6bc522/src/java/org/apache/cassandra/index/SecondaryIndexManager.java#L421]
 that are not preceded by a call to 
[{{SecondaryIndexManager#markIndexBuilding}}|https://github.com/adelapena/cassandra/blob/55c1fbad6116b8a8b9ccc86ce9fa50396d6bc522/src/java/org/apache/cassandra/index/SecondaryIndexManager.java#L410],
 but maybe it could be replaced by a warning, or even write the {{IndexInfo}} 
table anyway. What do you think?

Also, it's worth noting that, with this approach, if any (re)building fails for 
a index, it will get inevitably marked for rebuilding at restart, even if an 
index rebuilding is manually run. Should we avoid this? If so, how could we 
avoid race conditions between these manual (re)buildings and those produced by 
streaming?


was (Author: adelapena):
I have updated the patch to use counters, as suggested by [~pauloricardomg]:

||[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/]|

To complement 
[{{SecondaryIndexManager#markIndexBuilt}}|https://github.com/adelapena/cassandra/blob/55c1fbad6116b8a8b9ccc86ce9fa50396d6bc522/src/java/org/apache/cassandra/index/SecondaryIndexManager.java#L421]
 and 
[{{SecondaryIndexManager#markIndexRemoved}}|https://github.com/adelapena/cassandra/blob/55c1fbad6116b8a8b9ccc86ce9fa50396d6bc522/src/java/org/apache/cassandra/index/SecondaryIndexManager.java#L432]
 I have added a new method 
[{{SecondaryIndexManager#markIndexBuilding}}|https://github.com/adelapena/cassandra/blob/55c1fbad6116b8a8b9ccc86ce9fa50396d6bc522/src/java/org/apache/cassandra/index/SecondaryIndexManager.java#L410]
 to be called before any index (re)building. When this method is called from it 
increases a 
[{{SecondaryIndexManager#PendingIndexBuildsCounter}}|https://github.com/adelapena/cassandra/blob/55c1fbad6116b8a8b9ccc86ce9fa50396d6bc522/src/java/org/apache/cassandra/index/SecondaryIndexManager.java#L1216]
 associated to the index, and if the counter value is zero it removes the index 
from the {{IndexInfo}} table. Later, after the index (re)building has 
successfully ended, a call to the modified 
[{{SecondaryIndexManager#markIndexBuilt}}|https://github.com/adelapena/cassandra/blob/55c1fbad6116b8a8b9ccc86ce9fa50396d6bc522/src/java/org/apache/cassandra/index/SecondaryIndexManager.java#L421]
 will decrease the counter and, if it is zero, it will add the index to the 
{{IndexInfo}} table.

I'm not sure about if the assert in 
[{{SecondaryIndexManager.PendingIndexBuildsCounter#decrease()}}|https://github.com/adelapena/cassandra/blob/55c1fbad6116b8a8b9ccc86ce9fa50396d6bc522/src/java/org/apache/cassandra/index/SecondaryIndexManager.java#L1236]
 is too aggressive. The idea is to easily detect calls to 
[{{SecondaryIndexManager#markIndexBuilt}}|https://github.com/adelapena/cassandra/blob/55c1fbad6116b8a8b9ccc86ce9fa50396d6bc522/src/java/org/apache/cassandra/index/SecondaryIndexManager.java#L421]
 that are not preceded by a call to 
[{{SecondaryIndexManager#markIndexBuilding}}|https://github.com/adelapena/cassandra/blob/55c1fbad6116b8a8b9ccc86ce9fa50396d6bc522/src/java/org/apache/cassandra/index/SecondaryIndexManager.java#L410],
 but maybe it could be replaced by a warning, or even write the {{IndexInfo}} 
table anyway. What do you think?

Also, it's worth noting that, with this approach, if any (re)building fails for 
a index, it will get inevitably marked for rebuilding at restart, even if an 
index rebuilding is manually run. Should we avoid this? If so, how could we 
avoid race conditions between these manual (re)buildings and those produced by 
streaming?

> 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: commits-unsubscr...@cassandra.apache.org
For additional commands, e-mail: commits-h...@cassandra.apache.org

Reply via email to