You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@cassandra.apache.org by "Paulo Motta (JIRA)" <ji...@apache.org> on 2016/12/22 00:19:58 UTC

[jira] [Comment Edited] (CASSANDRA-13069) Local batchlog for MV may not be correctly written on node movements

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

Paulo Motta edited comment on CASSANDRA-13069 at 12/22/16 12:19 AM:
--------------------------------------------------------------------

Good catch! In the patch below I fixed this by writing a single batchlog for all non-local mutations (remote as well as non-paired). If the batchlog contains non-paired mutations, it will not receive acks and thus it will not be cleared, so it will be hopefully replayed after the ring has stabilized.

I also removed the {{pendingEndpoints.isEmpty()}} condition to skip the batchlog for local mutations, since this was a pre-CASSANDRA-10674 leftover when [ViewUtils.getViewNaturalEndpoint|https://github.com/apache/cassandra/blob/cassandra-3.0.0-rc1/src/java/org/apache/cassandra/db/view/ViewUtils.java#L100] returned the local address to force non-paired replicas to be written to the batchlog.

While fixing this I noticed that we don't decrement the {{BatchlogCleanup}} counter on local mutations, so if there are both local and remote paired mutations the batchlog is never cleared. This was reproduced on [this dtest|https://github.com/riptano/cassandra-dtest/compare/master...pauloricardomg:13069]. This might explain why [~brstgt] experienced multi-GB batchlogs when bootstrapping nodes with multiple MVs on CASSANDRA-12905.

Patch and tests available below:
||3.0||3.11||trunk||dtest||
|[branch|https://github.com/apache/cassandra/compare/cassandra-3.0...pauloricardomg:3.0-13069]|[branch|https://github.com/apache/cassandra/compare/cassandra-3.11...pauloricardomg:3.11-13069]|[branch|https://github.com/apache/cassandra/compare/trunk...pauloricardomg:trunk-13069]|[branch|https://github.com/riptano/cassandra-dtest/compare/master...pauloricardomg:13069]|
|[testall|http://cassci.datastax.com/view/Dev/view/paulomotta/job/pauloricardomg-3.0-13069-testall/lastCompletedBuild/testReport/]|[testall|http://cassci.datastax.com/view/Dev/view/paulomotta/job/pauloricardomg-3.11-13069-testall/lastCompletedBuild/testReport/]|[testall|http://cassci.datastax.com/view/Dev/view/paulomotta/job/pauloricardomg-trunk-13069-testall/lastCompletedBuild/testReport/]|
|[dtest|http://cassci.datastax.com/view/Dev/view/paulomotta/job/pauloricardomg-3.0-13069-dtest/lastCompletedBuild/testReport/]|[dtest|http://cassci.datastax.com/view/Dev/view/paulomotta/job/pauloricardomg-3.11-13069-dtest/lastCompletedBuild/testReport/]|[dtest|http://cassci.datastax.com/view/Dev/view/paulomotta/job/pauloricardomg-trunk-13069-dtest/lastCompletedBuild/testReport/]|



was (Author: pauloricardomg):
Good catch! In the patch below I fixed this by writing a single batchlog for all non-local mutations (remote as well as non-paired). If the batchlog contains non-paired mutations, it will not receive acks and thus it will not be cleared.

I also removed the {{pendingEndpoints.isEmpty()}} condition to skip the batchlog for local mutations, since this was a pre-CASSANDRA-10674 leftover when [ViewUtils.getViewNaturalEndpoint|https://github.com/apache/cassandra/blob/cassandra-3.0.0-rc1/src/java/org/apache/cassandra/db/view/ViewUtils.java#L100] returned the local address to force non-paired replicas to be written to the batchlog.

While fixing this I noticed that we don't decrement the {{BatchlogCleanup}} counter on local mutations, so if there are both local and remote paired mutations the batchlog is never cleared. This was reproduced on [this dtest|https://github.com/riptano/cassandra-dtest/compare/master...pauloricardomg:13069]. This might explain why [~brstgt] experienced multi-GB batchlogs when bootstrapping nodes with multiple MVs on CASSANDRA-12905.

Patch and tests available below:
||3.0||3.11||trunk||dtest||
|[branch|https://github.com/apache/cassandra/compare/cassandra-3.0...pauloricardomg:3.0-13069]|[branch|https://github.com/apache/cassandra/compare/cassandra-3.11...pauloricardomg:3.11-13069]|[branch|https://github.com/apache/cassandra/compare/trunk...pauloricardomg:trunk-13069]|[branch|https://github.com/riptano/cassandra-dtest/compare/master...pauloricardomg:13069]|
|[testall|http://cassci.datastax.com/view/Dev/view/paulomotta/job/pauloricardomg-3.0-13069-testall/lastCompletedBuild/testReport/]|[testall|http://cassci.datastax.com/view/Dev/view/paulomotta/job/pauloricardomg-3.11-13069-testall/lastCompletedBuild/testReport/]|[testall|http://cassci.datastax.com/view/Dev/view/paulomotta/job/pauloricardomg-trunk-13069-testall/lastCompletedBuild/testReport/]|
|[dtest|http://cassci.datastax.com/view/Dev/view/paulomotta/job/pauloricardomg-3.0-13069-dtest/lastCompletedBuild/testReport/]|[dtest|http://cassci.datastax.com/view/Dev/view/paulomotta/job/pauloricardomg-3.11-13069-dtest/lastCompletedBuild/testReport/]|[dtest|http://cassci.datastax.com/view/Dev/view/paulomotta/job/pauloricardomg-trunk-13069-dtest/lastCompletedBuild/testReport/]|


> Local batchlog for MV may not be correctly written on node movements
> --------------------------------------------------------------------
>
>                 Key: CASSANDRA-13069
>                 URL: https://issues.apache.org/jira/browse/CASSANDRA-13069
>             Project: Cassandra
>          Issue Type: Bug
>            Reporter: Sylvain Lebresne
>            Assignee: Paulo Motta
>
> Unless I'm really reading this wrong, I think the code [here|https://github.com/apache/cassandra/blob/trunk/src/java/org/apache/cassandra/service/StorageProxy.java#L829-L843], which comes from CASSANDRA-10674, isn't working properly.
> More precisely, I believe we can have both paired and unpaired mutations, so that both {{if}} can be taken, but if that's the case, the 2nd write to the batchlog will basically overwrite (remove) the batchlog write of the 1st {{if}} and I don't think that's the intention. In practice, this means "paired" mutation won't be in the batchlog, which mean they won't be replayed at all if they fail.



--
This message was sent by Atlassian JIRA
(v6.3.4#6332)