You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@cassandra.apache.org by sl...@apache.org on 2020/08/17 09:43:37 UTC
[cassandra] branch cassandra-3.0 updated: Remove broken
"defragment-on-read" optimization
This is an automated email from the ASF dual-hosted git repository.
slebresne pushed a commit to branch cassandra-3.0
in repository https://gitbox.apache.org/repos/asf/cassandra.git
The following commit(s) were added to refs/heads/cassandra-3.0 by this push:
new e2ecdf2 Remove broken "defragment-on-read" optimization
e2ecdf2 is described below
commit e2ecdf268a82fa3ac0f4c9fe77ab35bca33cc72a
Author: Sylvain Lebresne <le...@gmail.com>
AuthorDate: Wed Aug 12 16:29:43 2020 +0200
Remove broken "defragment-on-read" optimization
The read path for names queries has had a "defragment-on-read"
optimization for a while whereby if too many sstables are hit by the
read, the result is written back into memtable, in the hope that later
reads will only read that newly written data in a single sstable (or at
least fewer).
The principle of that optimisation does not work however as data is
written back with the same timestamp as it originally has and that means
future reads cannot know to skip older sstables (at least with the
metadata we currently store).
As such, this optimisation never saved anything and in fact added load.
The patch removes that broken code.
Patch by Sylvain Lebresne, reviewed by Aleksey Yeschenko for
CASSANDRA-15432
---
CHANGES.txt | 1 +
.../cassandra/db/SinglePartitionReadCommand.java | 28 ----------------------
.../db/compaction/AbstractCompactionStrategy.java | 5 ----
.../db/compaction/CompactionStrategyManager.java | 6 -----
.../compaction/SizeTieredCompactionStrategy.java | 6 -----
5 files changed, 1 insertion(+), 45 deletions(-)
diff --git a/CHANGES.txt b/CHANGES.txt
index 7d4b7a9..9b4f8c3 100644
--- a/CHANGES.txt
+++ b/CHANGES.txt
@@ -1,4 +1,5 @@
3.0.22:
+ * Remove broken 'defrag-on-read' optimization (CASSANDRA-15432)
* Check for endpoint collision with hibernating nodes (CASSANDRA-14599)
* Operational improvements and hardening for replica filtering protection (CASSANDRA-15907)
* stop_paranoid disk failure policy is ignored on CorruptSSTableException after node is up (CASSANDRA-15191)
diff --git a/src/java/org/apache/cassandra/db/SinglePartitionReadCommand.java b/src/java/org/apache/cassandra/db/SinglePartitionReadCommand.java
index 841c3b9..ca4e8e3 100644
--- a/src/java/org/apache/cassandra/db/SinglePartitionReadCommand.java
+++ b/src/java/org/apache/cassandra/db/SinglePartitionReadCommand.java
@@ -883,7 +883,6 @@ public class SinglePartitionReadCommand extends ReadCommand
/* add the SSTables on disk */
Collections.sort(view.sstables, SSTableReader.maxTimestampComparator);
- boolean onlyUnrepaired = true;
// read sorted sstables
SSTableReadMetricsCollector metricsCollector = new SSTableReadMetricsCollector();
for (SSTableReader sstable : view.sstables)
@@ -952,9 +951,6 @@ public class SinglePartitionReadCommand extends ReadCommand
if (iter.isEmpty())
continue;
- if (sstable.isRepaired())
- onlyUnrepaired = false;
-
result = add(
RTBoundValidator.validate(isForThrift() ? ThriftResultsMerger.maybeWrap(iter, nowInSec()) : iter, RTBoundValidator.Stage.SSTABLE, false),
result,
@@ -972,30 +968,6 @@ public class SinglePartitionReadCommand extends ReadCommand
DecoratedKey key = result.partitionKey();
cfs.metric.samplers.get(TableMetrics.Sampler.READS).addSample(key.getKey(), key.hashCode(), 1);
- // "hoist up" the requested data into a more recent sstable
- if (metricsCollector.getMergedSSTables() > cfs.getMinimumCompactionThreshold()
- && onlyUnrepaired
- && !cfs.isAutoCompactionDisabled()
- && cfs.getCompactionStrategyManager().shouldDefragment())
- {
- // !!WARNING!! if we stop copying our data to a heap-managed object,
- // we will need to track the lifetime of this mutation as well
- Tracing.trace("Defragmenting requested data");
-
- try (UnfilteredRowIterator iter = result.unfilteredIterator(columnFilter(), Slices.ALL, false))
- {
- final Mutation mutation = new Mutation(PartitionUpdate.fromIterator(iter));
- StageManager.getStage(Stage.MUTATION).execute(new Runnable()
- {
- public void run()
- {
- // skipping commitlog and index updates is fine since we're just de-fragmenting existing data
- Keyspace.open(mutation.getKeyspaceName()).apply(mutation, false, false);
- }
- });
- }
- }
-
return result.unfilteredIterator(columnFilter(), Slices.ALL, clusteringIndexFilter().isReversed());
}
diff --git a/src/java/org/apache/cassandra/db/compaction/AbstractCompactionStrategy.java b/src/java/org/apache/cassandra/db/compaction/AbstractCompactionStrategy.java
index 7219504..3ceda93 100644
--- a/src/java/org/apache/cassandra/db/compaction/AbstractCompactionStrategy.java
+++ b/src/java/org/apache/cassandra/db/compaction/AbstractCompactionStrategy.java
@@ -299,11 +299,6 @@ public abstract class AbstractCompactionStrategy
return new ScannerList(scanners);
}
- public boolean shouldDefragment()
- {
- return false;
- }
-
public String getName()
{
return getClass().getSimpleName();
diff --git a/src/java/org/apache/cassandra/db/compaction/CompactionStrategyManager.java b/src/java/org/apache/cassandra/db/compaction/CompactionStrategyManager.java
index 1d3d18c..5d4a254 100644
--- a/src/java/org/apache/cassandra/db/compaction/CompactionStrategyManager.java
+++ b/src/java/org/apache/cassandra/db/compaction/CompactionStrategyManager.java
@@ -235,12 +235,6 @@ public class CompactionStrategyManager implements INotificationConsumer
return res;
}
- public boolean shouldDefragment()
- {
- assert repaired.getClass().equals(unrepaired.getClass());
- return repaired.shouldDefragment();
- }
-
public Directories getDirectories()
{
assert repaired.getClass().equals(unrepaired.getClass());
diff --git a/src/java/org/apache/cassandra/db/compaction/SizeTieredCompactionStrategy.java b/src/java/org/apache/cassandra/db/compaction/SizeTieredCompactionStrategy.java
index 80f5e8c..dae3a78 100644
--- a/src/java/org/apache/cassandra/db/compaction/SizeTieredCompactionStrategy.java
+++ b/src/java/org/apache/cassandra/db/compaction/SizeTieredCompactionStrategy.java
@@ -310,12 +310,6 @@ public class SizeTieredCompactionStrategy extends AbstractCompactionStrategy
}
@Override
- public boolean shouldDefragment()
- {
- return true;
- }
-
- @Override
public synchronized void addSSTable(SSTableReader added)
{
sstables.add(added);
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@cassandra.apache.org
For additional commands, e-mail: commits-help@cassandra.apache.org