[jira] [Commented] (CASSANDRA-11529) Checking if an unlogged batch is local is inefficient
[
https://issues.apache.org/jira/browse/CASSANDRA-11529?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=15236453#comment-15236453
]
Stefania commented on CASSANDRA-11529:
--
Thank you [~iamaleksey]!
> Checking if an unlogged batch is local is inefficient
> -
>
> Key: CASSANDRA-11529
> URL: https://issues.apache.org/jira/browse/CASSANDRA-11529
> Project: Cassandra
> Issue Type: Bug
> Components: Coordination
>Reporter: Paulo Motta
>Assignee: Stefania
>Priority: Critical
> Labels: docs-impacting
> Fix For: 2.1.14, 2.2.6, 3.6, 3.0.6
>
>
> Based on CASSANDRA-11363 report I noticed that on CASSANDRA-9303 we
> introduced the following check to avoid printing a {{WARN}} in case an
> unlogged batch statement is local:
> {noformat}
> for (IMutation im : mutations)
> {
> keySet.add(im.key());
> for (ColumnFamily cf : im.getColumnFamilies())
> ksCfPairs.add(String.format("%s.%s",
> cf.metadata().ksName, cf.metadata().cfName));
> +
> +if (localMutationsOnly)
> +localMutationsOnly &= isMutationLocal(localTokensByKs,
> im);
> }
>
> +// CASSANDRA-9303: If we only have local mutations we do not warn
> +if (localMutationsOnly)
> +return;
> +
> NoSpamLogger.log(logger, NoSpamLogger.Level.WARN, 1,
> TimeUnit.MINUTES, unloggedBatchWarning,
> keySet.size(), keySet.size() == 1 ? "" : "s",
> ksCfPairs.size() == 1 ? "" : "s", ksCfPairs);
> {noformat}
> The {{isMutationLocal}} check uses
> {{StorageService.instance.getLocalRanges(mutation.getKeyspaceName())}}, which
> underneaths uses {{AbstractReplication.getAddressRanges}} to calculate local
> ranges.
> Recalculating this at every unlogged batch can be pretty inefficient, so we
> should at the very least cache it every time the ring changes.
--
This message was sent by Atlassian JIRA
(v6.3.4#6332)
[jira] [Commented] (CASSANDRA-11529) Checking if an unlogged batch is local is inefficient
[
https://issues.apache.org/jira/browse/CASSANDRA-11529?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=15235660#comment-15235660
]
Aleksey Yeschenko commented on CASSANDRA-11529:
---
Committed as
[c1b1d3bccf30a7ee1deb633d2bc2dfbd7b9c542f|https://github.com/apache/cassandra/commit/c1b1d3bccf30a7ee1deb633d2bc2dfbd7b9c542f]
to 2.1 and merged upwards into 2.2, 3.0, and trunk, thanks.
dtest PR merged.
> Checking if an unlogged batch is local is inefficient
> -
>
> Key: CASSANDRA-11529
> URL: https://issues.apache.org/jira/browse/CASSANDRA-11529
> Project: Cassandra
> Issue Type: Bug
> Components: Coordination
>Reporter: Paulo Motta
>Assignee: Stefania
>Priority: Critical
> Fix For: 2.1.x, 2.2.x, 3.0.x, 3.x
>
>
> Based on CASSANDRA-11363 report I noticed that on CASSANDRA-9303 we
> introduced the following check to avoid printing a {{WARN}} in case an
> unlogged batch statement is local:
> {noformat}
> for (IMutation im : mutations)
> {
> keySet.add(im.key());
> for (ColumnFamily cf : im.getColumnFamilies())
> ksCfPairs.add(String.format("%s.%s",
> cf.metadata().ksName, cf.metadata().cfName));
> +
> +if (localMutationsOnly)
> +localMutationsOnly &= isMutationLocal(localTokensByKs,
> im);
> }
>
> +// CASSANDRA-9303: If we only have local mutations we do not warn
> +if (localMutationsOnly)
> +return;
> +
> NoSpamLogger.log(logger, NoSpamLogger.Level.WARN, 1,
> TimeUnit.MINUTES, unloggedBatchWarning,
> keySet.size(), keySet.size() == 1 ? "" : "s",
> ksCfPairs.size() == 1 ? "" : "s", ksCfPairs);
> {noformat}
> The {{isMutationLocal}} check uses
> {{StorageService.instance.getLocalRanges(mutation.getKeyspaceName())}}, which
> underneaths uses {{AbstractReplication.getAddressRanges}} to calculate local
> ranges.
> Recalculating this at every unlogged batch can be pretty inefficient, so we
> should at the very least cache it every time the ring changes.
--
This message was sent by Atlassian JIRA
(v6.3.4#6332)
[jira] [Commented] (CASSANDRA-11529) Checking if an unlogged batch is local is inefficient
[
https://issues.apache.org/jira/browse/CASSANDRA-11529?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=15234470#comment-15234470
]
Stefania commented on CASSANDRA-11529:
--
CI should be fine. The failing dtests on trunk do not seem related and all pass
locally.
Commit information:
||2.1||2.2||3.0||trunk||
|[patch|https://github.com/stef1927/cassandra/commits/11529-2.1]|[patch|https://github.com/stef1927/cassandra/commits/11529-2.2]|[patch|https://github.com/stef1927/cassandra/commits/11529-3.0]|[patch|https://github.com/stef1927/cassandra/commits/11529]|
|[testall|http://cassci.datastax.com/view/Dev/view/stef1927/job/stef1927-11529-2.1-testall/]|[testall|http://cassci.datastax.com/view/Dev/view/stef1927/job/stef1927-11529-2.2-testall/]|[testall|http://cassci.datastax.com/view/Dev/view/stef1927/job/stef1927-11529-3.0-testall/]|[testall|http://cassci.datastax.com/view/Dev/view/stef1927/job/stef1927-11529-testall/]|
|[dtest|http://cassci.datastax.com/view/Dev/view/stef1927/job/stef1927-11529-2.1-dtest/]|[dtest|http://cassci.datastax.com/view/Dev/view/stef1927/job/stef1927-11529-2.2-dtest/]|[dtest|http://cassci.datastax.com/view/Dev/view/stef1927/job/stef1927-11529-3.0-dtest/]|[dtest|http://cassci.datastax.com/view/Dev/view/stef1927/job/stef1927-11529-dtest/]|
No single patch up-merges cleanly, there are conflicts for all branches and the
trunk patch is slightly different in cassandra.yaml and has 2 authors.
There is also a dtest [pull
request|https://github.com/riptano/cassandra-dtest/pull/919].
> Checking if an unlogged batch is local is inefficient
> -
>
> Key: CASSANDRA-11529
> URL: https://issues.apache.org/jira/browse/CASSANDRA-11529
> Project: Cassandra
> Issue Type: Bug
> Components: Coordination
>Reporter: Paulo Motta
>Assignee: Stefania
>Priority: Critical
> Fix For: 2.1.x, 2.2.x, 3.0.x, 3.x
>
>
> Based on CASSANDRA-11363 report I noticed that on CASSANDRA-9303 we
> introduced the following check to avoid printing a {{WARN}} in case an
> unlogged batch statement is local:
> {noformat}
> for (IMutation im : mutations)
> {
> keySet.add(im.key());
> for (ColumnFamily cf : im.getColumnFamilies())
> ksCfPairs.add(String.format("%s.%s",
> cf.metadata().ksName, cf.metadata().cfName));
> +
> +if (localMutationsOnly)
> +localMutationsOnly &= isMutationLocal(localTokensByKs,
> im);
> }
>
> +// CASSANDRA-9303: If we only have local mutations we do not warn
> +if (localMutationsOnly)
> +return;
> +
> NoSpamLogger.log(logger, NoSpamLogger.Level.WARN, 1,
> TimeUnit.MINUTES, unloggedBatchWarning,
> keySet.size(), keySet.size() == 1 ? "" : "s",
> ksCfPairs.size() == 1 ? "" : "s", ksCfPairs);
> {noformat}
> The {{isMutationLocal}} check uses
> {{StorageService.instance.getLocalRanges(mutation.getKeyspaceName())}}, which
> underneaths uses {{AbstractReplication.getAddressRanges}} to calculate local
> ranges.
> Recalculating this at every unlogged batch can be pretty inefficient, so we
> should at the very least cache it every time the ring changes.
--
This message was sent by Atlassian JIRA
(v6.3.4#6332)
[jira] [Commented] (CASSANDRA-11529) Checking if an unlogged batch is local is inefficient
[
https://issues.apache.org/jira/browse/CASSANDRA-11529?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=15234378#comment-15234378
]
Stefania commented on CASSANDRA-11529:
--
Thanks for the suggestion, it LGTM.
I've incorporated your commit into my trunk patch, rebased and squashed with
correct authors.
I've restarted CI on trunk because there was a problem with the cqlsh version.
If CI on trunk is OK, I will mark as ready to commit.
> Checking if an unlogged batch is local is inefficient
> -
>
> Key: CASSANDRA-11529
> URL: https://issues.apache.org/jira/browse/CASSANDRA-11529
> Project: Cassandra
> Issue Type: Bug
> Components: Coordination
>Reporter: Paulo Motta
>Assignee: Stefania
>Priority: Critical
> Fix For: 2.1.x, 2.2.x, 3.0.x, 3.x
>
>
> Based on CASSANDRA-11363 report I noticed that on CASSANDRA-9303 we
> introduced the following check to avoid printing a {{WARN}} in case an
> unlogged batch statement is local:
> {noformat}
> for (IMutation im : mutations)
> {
> keySet.add(im.key());
> for (ColumnFamily cf : im.getColumnFamilies())
> ksCfPairs.add(String.format("%s.%s",
> cf.metadata().ksName, cf.metadata().cfName));
> +
> +if (localMutationsOnly)
> +localMutationsOnly &= isMutationLocal(localTokensByKs,
> im);
> }
>
> +// CASSANDRA-9303: If we only have local mutations we do not warn
> +if (localMutationsOnly)
> +return;
> +
> NoSpamLogger.log(logger, NoSpamLogger.Level.WARN, 1,
> TimeUnit.MINUTES, unloggedBatchWarning,
> keySet.size(), keySet.size() == 1 ? "" : "s",
> ksCfPairs.size() == 1 ? "" : "s", ksCfPairs);
> {noformat}
> The {{isMutationLocal}} check uses
> {{StorageService.instance.getLocalRanges(mutation.getKeyspaceName())}}, which
> underneaths uses {{AbstractReplication.getAddressRanges}} to calculate local
> ranges.
> Recalculating this at every unlogged batch can be pretty inefficient, so we
> should at the very least cache it every time the ring changes.
--
This message was sent by Atlassian JIRA
(v6.3.4#6332)
[jira] [Commented] (CASSANDRA-11529) Checking if an unlogged batch is local is inefficient
[
https://issues.apache.org/jira/browse/CASSANDRA-11529?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=15232392#comment-15232392
]
Paulo Motta commented on CASSANDRA-11529:
-
LGTM, given that alternative was already proposed and discussed on
CASSANDRA-9282. Thanks!
Since we now have quite a few warning thresholds, I think we should group them
together on cassandra.yaml to make finding them easier, so I updated the trunk
patch moving the warning thresholds to its own section (the previous versions
remain unchanged):
||trunk||
|[branch|https://github.com/apache/cassandra/compare/trunk...pauloricardomg:trunk-11529]|
|[testall|http://cassci.datastax.com/view/Dev/view/paulomotta/job/pauloricardomg-trunk-11529-testall/lastCompletedBuild/testReport/]|
|[dtest|http://cassci.datastax.com/view/Dev/view/paulomotta/job/pauloricardomg-trunk-11529-dtest/lastCompletedBuild/testReport/]|
(please mark as ready to commit if tests look good and you agree with above
change)
> Checking if an unlogged batch is local is inefficient
> -
>
> Key: CASSANDRA-11529
> URL: https://issues.apache.org/jira/browse/CASSANDRA-11529
> Project: Cassandra
> Issue Type: Bug
> Components: Coordination
>Reporter: Paulo Motta
>Assignee: Stefania
>Priority: Critical
> Fix For: 2.1.x, 2.2.x, 3.0.x, 3.x
>
>
> Based on CASSANDRA-11363 report I noticed that on CASSANDRA-9303 we
> introduced the following check to avoid printing a {{WARN}} in case an
> unlogged batch statement is local:
> {noformat}
> for (IMutation im : mutations)
> {
> keySet.add(im.key());
> for (ColumnFamily cf : im.getColumnFamilies())
> ksCfPairs.add(String.format("%s.%s",
> cf.metadata().ksName, cf.metadata().cfName));
> +
> +if (localMutationsOnly)
> +localMutationsOnly &= isMutationLocal(localTokensByKs,
> im);
> }
>
> +// CASSANDRA-9303: If we only have local mutations we do not warn
> +if (localMutationsOnly)
> +return;
> +
> NoSpamLogger.log(logger, NoSpamLogger.Level.WARN, 1,
> TimeUnit.MINUTES, unloggedBatchWarning,
> keySet.size(), keySet.size() == 1 ? "" : "s",
> ksCfPairs.size() == 1 ? "" : "s", ksCfPairs);
> {noformat}
> The {{isMutationLocal}} check uses
> {{StorageService.instance.getLocalRanges(mutation.getKeyspaceName())}}, which
> underneaths uses {{AbstractReplication.getAddressRanges}} to calculate local
> ranges.
> Recalculating this at every unlogged batch can be pretty inefficient, so we
> should at the very least cache it every time the ring changes.
--
This message was sent by Atlassian JIRA
(v6.3.4#6332)
[jira] [Commented] (CASSANDRA-11529) Checking if an unlogged batch is local is inefficient
[
https://issues.apache.org/jira/browse/CASSANDRA-11529?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=15231641#comment-15231641
]
Stefania commented on CASSANDRA-11529:
--
I've removed the check for local mutations and added a threshold that can be
configured via a new yaml parameter,
{{unlogged_batch_across_partitions_warn_threshold}}.
Here are the patches and CI results:
||2.1||2.2||3.0||trunk||
|[patch|https://github.com/stef1927/cassandra/commits/11529-2.1]|[patch|https://github.com/stef1927/cassandra/commits/11529-2.2]|[patch|https://github.com/stef1927/cassandra/commits/11529-3.0]|[patch|https://github.com/stef1927/cassandra/commits/11529]|
|[testall|http://cassci.datastax.com/view/Dev/view/stef1927/job/stef1927-11529-2.1-testall/]|[testall|http://cassci.datastax.com/view/Dev/view/stef1927/job/stef1927-11529-2.2-testall/]|[testall|http://cassci.datastax.com/view/Dev/view/stef1927/job/stef1927-11529-3.0-testall/]|[testall|http://cassci.datastax.com/view/Dev/view/stef1927/job/stef1927-11529-testall/]|
|[dtest|http://cassci.datastax.com/view/Dev/view/stef1927/job/stef1927-11529-2.1-dtest/]|[dtest|http://cassci.datastax.com/view/Dev/view/stef1927/job/stef1927-11529-2.2-dtest/]|[dtest|http://cassci.datastax.com/view/Dev/view/stef1927/job/stef1927-11529-3.0-dtest/]|[dtest|http://cassci.datastax.com/view/Dev/view/stef1927/job/stef1927-11529-dtest/]|
No single patch up-merges cleanly, there are conflicts for all branches.
We also have a dtest patch
[here|https://github.com/stef1927/cassandra-dtest/commits/11529].
> Checking if an unlogged batch is local is inefficient
> -
>
> Key: CASSANDRA-11529
> URL: https://issues.apache.org/jira/browse/CASSANDRA-11529
> Project: Cassandra
> Issue Type: Bug
> Components: Coordination
>Reporter: Paulo Motta
>Assignee: Stefania
>Priority: Critical
> Fix For: 2.1.x, 2.2.x, 3.0.x, 3.x
>
>
> Based on CASSANDRA-11363 report I noticed that on CASSANDRA-9303 we
> introduced the following check to avoid printing a {{WARN}} in case an
> unlogged batch statement is local:
> {noformat}
> for (IMutation im : mutations)
> {
> keySet.add(im.key());
> for (ColumnFamily cf : im.getColumnFamilies())
> ksCfPairs.add(String.format("%s.%s",
> cf.metadata().ksName, cf.metadata().cfName));
> +
> +if (localMutationsOnly)
> +localMutationsOnly &= isMutationLocal(localTokensByKs,
> im);
> }
>
> +// CASSANDRA-9303: If we only have local mutations we do not warn
> +if (localMutationsOnly)
> +return;
> +
> NoSpamLogger.log(logger, NoSpamLogger.Level.WARN, 1,
> TimeUnit.MINUTES, unloggedBatchWarning,
> keySet.size(), keySet.size() == 1 ? "" : "s",
> ksCfPairs.size() == 1 ? "" : "s", ksCfPairs);
> {noformat}
> The {{isMutationLocal}} check uses
> {{StorageService.instance.getLocalRanges(mutation.getKeyspaceName())}}, which
> underneaths uses {{AbstractReplication.getAddressRanges}} to calculate local
> ranges.
> Recalculating this at every unlogged batch can be pretty inefficient, so we
> should at the very least cache it every time the ring changes.
--
This message was sent by Atlassian JIRA
(v6.3.4#6332)
[jira] [Commented] (CASSANDRA-11529) Checking if an unlogged batch is local is inefficient
[
https://issues.apache.org/jira/browse/CASSANDRA-11529?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=15231202#comment-15231202
]
Russell Bradberry commented on CASSANDRA-11529:
---
This is a critical issue for us as our cluster is in a mixed-version state
where we have coordinator-only nodes running an older version to compensate for
this issue. The impact on a 50 node (8 cores, 256 vnodes) cluster with a few
thousand batch inserts per second sends the average load to above 120.
> Checking if an unlogged batch is local is inefficient
> -
>
> Key: CASSANDRA-11529
> URL: https://issues.apache.org/jira/browse/CASSANDRA-11529
> Project: Cassandra
> Issue Type: Bug
> Components: Coordination
>Reporter: Paulo Motta
>
> Based on CASSANDRA-11363 report I noticed that on CASSANDRA-9303 we
> introduced the following check to avoid printing a {{WARN}} in case an
> unlogged batch statement is local:
> {noformat}
> for (IMutation im : mutations)
> {
> keySet.add(im.key());
> for (ColumnFamily cf : im.getColumnFamilies())
> ksCfPairs.add(String.format("%s.%s",
> cf.metadata().ksName, cf.metadata().cfName));
> +
> +if (localMutationsOnly)
> +localMutationsOnly &= isMutationLocal(localTokensByKs,
> im);
> }
>
> +// CASSANDRA-9303: If we only have local mutations we do not warn
> +if (localMutationsOnly)
> +return;
> +
> NoSpamLogger.log(logger, NoSpamLogger.Level.WARN, 1,
> TimeUnit.MINUTES, unloggedBatchWarning,
> keySet.size(), keySet.size() == 1 ? "" : "s",
> ksCfPairs.size() == 1 ? "" : "s", ksCfPairs);
> {noformat}
> The {{isMutationLocal}} check uses
> {{StorageService.instance.getLocalRanges(mutation.getKeyspaceName())}}, which
> underneaths uses {{AbstractReplication.getAddressRanges}} to calculate local
> ranges.
> Recalculating this at every unlogged batch can be pretty inefficient, so we
> should at the very least cache it every time the ring changes.
--
This message was sent by Atlassian JIRA
(v6.3.4#6332)
