[
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)
