[CASSANDRA-11529] Checking if an unlogged batch is local is inefficient

• Type: Bug
• Status: Resolved
• Priority: Urgent
• Resolution: Fixed
• Fix Version/s: 2.1.14, 2.2.6, 3.0.6, 3.6
• Component/s: Legacy/Coordination
• Severity: Critical
• Impacts: Docs
• Assignee: Stefania Alborghetti
• Reporter: Paulo Motta

Authors

Stefania Alborghetti

Reviewers

Paulo Motta

• Created: 07/Apr/16 21:33
• Updated: 06/Nov/19 22:04
• Resolved: 11/Apr/16 18:21

Description

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:

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

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.

Issue Links

• is broken by: CASSANDRA-9303 Match cassandra-loader options in COPY FROM
• is duplicated by: CASSANDRA-10792 Make warning level for count of partitions in unlogged BatchStatement configurable.
• is related to: CASSANDRA-9282 Warn on unlogged batches