jyothsnakonisa commented on code in PR #4476:
URL: https://github.com/apache/cassandra/pull/4476#discussion_r2629064367
##########
src/java/org/apache/cassandra/db/compaction/UnifiedCompactionStrategy.java:
##########
@@ -529,6 +529,25 @@ List<Level> getLevels()
return getLevels(getSSTables(),
UnifiedCompactionStrategy::isSuitableForCompaction);
}
+ /**
+ * @return a list of the levels in the compaction hierarchy, that also
includes SSTables that
+ * are currently undergoing compaction. This is used only for table stats
so we can have a consistent
+ * snapshot of the levels.
+ */
+ @VisibleForTesting
Review Comment:
This method is being used in other places along with tests, may be we can
remove `@VisibleForTesting` annotation
##########
src/java/org/apache/cassandra/tools/nodetool/stats/TableStatsHolder.java:
##########
@@ -275,6 +284,66 @@ private void initializeKeyspaces(NodeProbe probe, boolean
ignore, List<String> t
}
}
+ double[] ucsSSTableTokenSpace =
table.getPerLevelAvgTokenSpace();
+ if (ucsSSTableTokenSpace != null)
+ {
+ statsTable.isUCSSstable = true;
+ for (int level = 0; level < ucsSSTableTokenSpace.length;
level++)
+ {
+
statsTable.sstableAvgTokenSpaceInEachLevel.add(String.format("%.03f",
ucsSSTableTokenSpace[level]));
+ }
+ }
+
+ double[] ucsMaxDensityThreshold =
table.getPerLevelMaxDensityThreshold();
+ if (ucsMaxDensityThreshold != null)
+ {
+ statsTable.isUCSSstable = true;
+ for (int level = 0; level < ucsMaxDensityThreshold.length;
level++)
+ {
+
statsTable.sstableMaxDensityThresholdInEachLevel.add(String.format("%.03f",
ucsMaxDensityThreshold[level]));
+ }
+ }
+
+ double[] ucsSsTableAvgSize = table.getPerLevelAvgSize();
Review Comment:
```suggestion
double[] ucsSSTableAvgSize = table.getPerLevelAvgSize();
```
Please make the variable name consistent with other variable names
##########
test/distributed/org/apache/cassandra/distributed/test/ColumnFamilyStoreMBeansTest.java:
##########
@@ -0,0 +1,126 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.cassandra.distributed.test;
+
+import org.junit.*;
+
+import org.apache.cassandra.db.ColumnFamilyStore;
+import org.apache.cassandra.db.Keyspace;
+import org.apache.cassandra.distributed.Cluster;
+
+import java.io.IOException;
+
+import static org.apache.cassandra.distributed.api.Feature.GOSSIP;
+import static org.apache.cassandra.distributed.api.Feature.NETWORK;
+
+public class ColumnFamilyStoreMBeansTest extends TestBaseImpl
+{
+ private static Cluster CLUSTER;
+
+ @BeforeClass
+ public static void setup() throws IOException
+ {
+ CLUSTER = init(Cluster.build(1).withConfig(c ->
+ c.with(GOSSIP, NETWORK))
+ .start());
+
+ CLUSTER.schemaChange(withKeyspace("DROP KEYSPACE %s"));
+ CLUSTER.schemaChange(withKeyspace("CREATE KEYSPACE %s WITH replication
= {'class': 'SimpleStrategy', 'replication_factor': 1}"));
+ CLUSTER.schemaChange(withKeyspace("CREATE TABLE %s.cf (k text, c1
text, c2 text, PRIMARY KEY (k)) WITH compaction = {'class':
'UnifiedCompactionStrategy', 'scaling_parameters': 'L10'}"));
+
+ for (int i = 0; i < 10000; i++)
+ CLUSTER.get(1).executeInternal(withKeyspace("INSERT INTO %s.cf (k,
c1, c2) VALUES (?, 'value1', 'value2');"), Integer.toString(i));
+
+ CLUSTER.get(1).nodetool("flush");
+ }
+
+ @Test
+ public void testPerLevelAverageTokenSpace() throws Throwable
+ {
+ CLUSTER.get(1).runOnInstance(() -> {
+ ColumnFamilyStore cfs =
Keyspace.open(KEYSPACE).getColumnFamilyStore("cf");
+ double[] perLevelAvgTokenSpace = cfs.getPerLevelAvgTokenSpace();
+ assert(perLevelAvgTokenSpace.length > 0);
+ for (int i = 0; i < perLevelAvgTokenSpace.length; i++)
+ Assert.assertTrue(perLevelAvgTokenSpace[i] > 0);
+ });
+ }
+
+ @Test
+ public void testGetPerLevelMaxDensityThreshold() throws Throwable
+ {
+ CLUSTER.get(1).runOnInstance(() -> {
+ ColumnFamilyStore cfs =
Keyspace.open(KEYSPACE).getColumnFamilyStore("cf");
+ Assert.assertTrue(cfs.getPerLevelMaxDensityThreshold().length > 0);
+ });
+ }
+
+ @Test
+ public void testGetPerLevelAvgSize() throws Throwable
+ {
+ CLUSTER.get(1).runOnInstance(() -> {
+ ColumnFamilyStore cfs =
Keyspace.open(KEYSPACE).getColumnFamilyStore("cf");
+ double[] perLevelAvgSize = cfs.getPerLevelAvgSize();
+ assert(perLevelAvgSize.length > 0);
+ for (int i = 0; i < perLevelAvgSize.length; i++)
+ Assert.assertTrue(perLevelAvgSize[i] > 0);
+ });
+ }
+
+ @Test
+ public void testGetPerLevelAvgDensity() throws Throwable
+ {
+ CLUSTER.get(1).runOnInstance(() -> {
+ ColumnFamilyStore cfs =
Keyspace.open(KEYSPACE).getColumnFamilyStore("cf");
+ double[] perLevelAvgDensity = cfs.getPerLevelAvgDensity();
+ assert(perLevelAvgDensity.length > 0);
+ for (int i = 0; i < perLevelAvgDensity.length; i++)
+ Assert.assertTrue(perLevelAvgDensity[i] > 0);
+ });
+ }
+
+ @Test
+ public void testGetPerLevelAvgDensityMaxDensityThresholdRatio() throws
Throwable
+ {
+ CLUSTER.get(1).runOnInstance(() -> {
+ ColumnFamilyStore cfs =
Keyspace.open(KEYSPACE).getColumnFamilyStore("cf");
+ double[] perLevelAvgDensityMaxDensityThresholdRatio =
cfs.getPerLevelAvgDensityMaxDensityThresholdRatio();
+ assert(perLevelAvgDensityMaxDensityThresholdRatio.length > 0);
+ for (int i = 0; i <
perLevelAvgDensityMaxDensityThresholdRatio.length; i++)
+ {
+ Assert.assertTrue(0 <=
perLevelAvgDensityMaxDensityThresholdRatio[i]);
+
Assert.assertTrue(perLevelAvgDensityMaxDensityThresholdRatio[i] < 1);
+ }
+ });
+ }
+
+ @Test
+ public void testGetPerLevelMaxDensityMaxDensityThresholdRatio() throws
Throwable
+ {
+ CLUSTER.get(1).runOnInstance(() -> {
+ ColumnFamilyStore cfs =
Keyspace.open(KEYSPACE).getColumnFamilyStore("cf");
+ double[] perLevelMaxDensityMaxDensityThresholdRatio =
cfs.getPerLevelMaxDensityMaxDensityThresholdRatio();
+ assert(perLevelMaxDensityMaxDensityThresholdRatio.length > 0);
+ for (int i = 0; i <
perLevelMaxDensityMaxDensityThresholdRatio.length; i++) {
+ Assert.assertTrue(0 <=
perLevelMaxDensityMaxDensityThresholdRatio[i]);
+
Assert.assertTrue(perLevelMaxDensityMaxDensityThresholdRatio[i] < 1);
+ }
+ });
+ }
+}
Review Comment:
Please add a new line at the end
##########
src/java/org/apache/cassandra/tools/nodetool/stats/TableStatsPrinter.java:
##########
@@ -100,6 +100,22 @@ protected void printStatsTable(StatsTable table, String
tableDisplayName, String
table.sstableBytesInEachLevel) + "]");
}
+ if (table.isUCSSstable)
+ {
+ out.println(indent + "Average token space for SSTables in each
level: [" + String.join(", ",
+ table.sstableAvgTokenSpaceInEachLevel) + "]");
+ out.println(indent + "Maximum density for SSTables in each
level: [" + String.join(", ",
Review Comment:
```suggestion
out.println(indent + "Maximum density threshold for SSTables
in each level: [" + String.join(", ",
```
##########
src/java/org/apache/cassandra/db/compaction/CompactionStrategyManager.java:
##########
@@ -702,6 +793,85 @@ public boolean isLeveledCompaction()
}
}
+ double[] avgUCSHelper(BiFunction<UnifiedCompactionStrategy, SSTableReader,
Double> fn)
+ {
+ readLock.lock();
+ try
+ {
+ if (repaired.first() instanceof UnifiedCompactionStrategy)
+ {
+ int numberOfLevels = 0;
+
+ double[] sum = new
double[UnifiedCompactionStrategy.MAX_LEVELS];
+ int[] count = new int[UnifiedCompactionStrategy.MAX_LEVELS];
+
+ for (AbstractCompactionStrategy strategy : getAllStrategies())
+ {
+ UnifiedCompactionStrategy ucsStrategy =
(UnifiedCompactionStrategy) strategy;
+ List<Level> levels = ucsStrategy.getLevelsSnapshot();
+ numberOfLevels = Math.max(numberOfLevels, levels.size());
+ for (int i = 0; i < levels.size(); i++)
+ {
+ for (SSTableReader sstable :
levels.get(i).getSSTables())
+ {
+ sum[i] += fn.apply(ucsStrategy, sstable);
+ count[i] += 1;
+ }
+ }
+ }
+
+ double[] res = new double[numberOfLevels];
+ for (int i = 0; i < numberOfLevels; i++)
+ res[i] = count[i] == 0 ? 0 : sum[i] / count[i];
+
+ return res;
+ }
+ return null;
+ }
+ finally
+ {
+ readLock.unlock();
+ }
+ }
+
+ double[] perLevelUCSHelper(TriFunction<Level, SSTableReader, Double,
Double> fn)
+ {
+ readLock.lock();
+ try
+ {
+ if (repaired.first() instanceof UnifiedCompactionStrategy)
+ {
+ int numberOfLevels = 0;
+
+ double[] tmp = new
double[UnifiedCompactionStrategy.MAX_LEVELS];
+
+ for (AbstractCompactionStrategy strategy : getAllStrategies())
+ {
+ UnifiedCompactionStrategy ucsStrategy =
(UnifiedCompactionStrategy) strategy;
+ List<Level> levels = ucsStrategy.getLevelsSnapshot();
+ numberOfLevels = Math.max(numberOfLevels, levels.size());
+ for (int i = 0; i < levels.size(); i++)
+ {
+ for (SSTableReader sstable :
levels.get(i).getSSTables())
+ {
+ tmp[i] = fn.apply(levels.get(i), sstable, tmp[i]);
+ }
+ }
+ }
+
+ double[] res = new double[numberOfLevels];
+ System.arraycopy(tmp, 0, res, 0, numberOfLevels);
+
+ return res;
+ }
+ return null;
+ }
+ finally
+ {
+ readLock.unlock();
+ }
+ }
+
Review Comment:
I noticed `getPerLevelAvgDensityMaxDensityThresholdRatio(),
getPerLevelMaxDensityMaxDensityThresholdRatio(), avgUCSHelper(), and
perLevelUCSHelper() `all share common code patterns (lock acquisition,
iterating across levels and sstables, result array creation).
Suggestion: We could consolidate these into a single generic helper:
computeUCSMetric(Consumer<UCSLevelData> accumulator, IntFunction<Double>
finalizer)
Where:
- accumulator contains the per-sstable operation logic (what to do in the
loop)
- finalizer computes the final result for each level
This would eliminate the duplication while keeping each metric method
focused on its specific calculation logic.
Would this approach work for you? Happy to provide a more detailed example
if helpful!
##########
src/java/org/apache/cassandra/tools/nodetool/stats/TableStatsPrinter.java:
##########
@@ -100,6 +100,22 @@ protected void printStatsTable(StatsTable table, String
tableDisplayName, String
table.sstableBytesInEachLevel) + "]");
}
+ if (table.isUCSSstable)
+ {
+ out.println(indent + "Average token space for SSTables in each
level: [" + String.join(", ",
+ table.sstableAvgTokenSpaceInEachLevel) + "]");
+ out.println(indent + "Maximum density for SSTables in each
level: [" + String.join(", ",
+ table.sstableMaxDensityThresholdInEachLevel) + "]");
+ out.println(indent + "Average SSTable size in each level: [" +
String.join(", ",
+ table.sstableAvgSizeInEachLevel) + "]");
+ out.println(indent + "Average SSTable density in each level:
[" + String.join(", ",
+ table.sstableAvgDensityInEachLevel) + "]");
+ out.println(indent + "Ratio of average SSTable density and
maximum density threshold in each level: [" + String.join(", ",
+
table.sstableAvgDensityMaxDensityThresholdRatioInEachLevel) + "]");
+ out.println(indent + "Ratio of maximum SSTable density and
maximum density threshold in each level: [" + String.join(", ",
+
table.sstableMaxDensityMaxDensityThresholdRatioInEachLevel) + "]");
Review Comment:
```suggestion
out.println(indent + "Average density to max threshold ratio
in each leve: [" + String.join(", ",
table.sstableAvgDensityMaxDensityThresholdRatioInEachLevel) + "]");
out.println(indent + "Maximum density to max threshold ratio
in each level: [" + String.join(", ",
table.sstableMaxDensityMaxDensityThresholdRatioInEachLevel) + "]");
```
shorter names?
##########
src/java/org/apache/cassandra/db/compaction/UnifiedCompactionStrategy.java:
##########
@@ -546,6 +565,11 @@ public List<Level> getLevels(Collection<SSTableReader>
sstables,
return formLevels(suitable);
}
+ public double getDensity(SSTableReader sstable)
Review Comment:
Please add java docs
##########
test/distributed/org/apache/cassandra/distributed/test/ColumnFamilyStoreMBeansTest.java:
##########
@@ -0,0 +1,126 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.cassandra.distributed.test;
+
+import org.junit.*;
+
+import org.apache.cassandra.db.ColumnFamilyStore;
+import org.apache.cassandra.db.Keyspace;
+import org.apache.cassandra.distributed.Cluster;
+
+import java.io.IOException;
+
+import static org.apache.cassandra.distributed.api.Feature.GOSSIP;
+import static org.apache.cassandra.distributed.api.Feature.NETWORK;
+
+public class ColumnFamilyStoreMBeansTest extends TestBaseImpl
+{
+ private static Cluster CLUSTER;
+
+ @BeforeClass
+ public static void setup() throws IOException
+ {
+ CLUSTER = init(Cluster.build(1).withConfig(c ->
+ c.with(GOSSIP, NETWORK))
+ .start());
+
+ CLUSTER.schemaChange(withKeyspace("DROP KEYSPACE %s"));
+ CLUSTER.schemaChange(withKeyspace("CREATE KEYSPACE %s WITH replication
= {'class': 'SimpleStrategy', 'replication_factor': 1}"));
+ CLUSTER.schemaChange(withKeyspace("CREATE TABLE %s.cf (k text, c1
text, c2 text, PRIMARY KEY (k)) WITH compaction = {'class':
'UnifiedCompactionStrategy', 'scaling_parameters': 'L10'}"));
+
+ for (int i = 0; i < 10000; i++)
+ CLUSTER.get(1).executeInternal(withKeyspace("INSERT INTO %s.cf (k,
c1, c2) VALUES (?, 'value1', 'value2');"), Integer.toString(i));
+
+ CLUSTER.get(1).nodetool("flush");
+ }
+
+ @Test
+ public void testPerLevelAverageTokenSpace() throws Throwable
+ {
+ CLUSTER.get(1).runOnInstance(() -> {
+ ColumnFamilyStore cfs =
Keyspace.open(KEYSPACE).getColumnFamilyStore("cf");
+ double[] perLevelAvgTokenSpace = cfs.getPerLevelAvgTokenSpace();
+ assert(perLevelAvgTokenSpace.length > 0);
+ for (int i = 0; i < perLevelAvgTokenSpace.length; i++)
+ Assert.assertTrue(perLevelAvgTokenSpace[i] > 0);
+ });
+ }
+
+ @Test
+ public void testGetPerLevelMaxDensityThreshold() throws Throwable
+ {
+ CLUSTER.get(1).runOnInstance(() -> {
+ ColumnFamilyStore cfs =
Keyspace.open(KEYSPACE).getColumnFamilyStore("cf");
+ Assert.assertTrue(cfs.getPerLevelMaxDensityThreshold().length > 0);
+ });
+ }
+
+ @Test
+ public void testGetPerLevelAvgSize() throws Throwable
+ {
+ CLUSTER.get(1).runOnInstance(() -> {
+ ColumnFamilyStore cfs =
Keyspace.open(KEYSPACE).getColumnFamilyStore("cf");
+ double[] perLevelAvgSize = cfs.getPerLevelAvgSize();
+ assert(perLevelAvgSize.length > 0);
+ for (int i = 0; i < perLevelAvgSize.length; i++)
+ Assert.assertTrue(perLevelAvgSize[i] > 0);
Review Comment:
Please do static import instead everywhere for assertTrue
##########
test/distributed/org/apache/cassandra/distributed/test/ColumnFamilyStoreMBeansTest.java:
##########
@@ -0,0 +1,126 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.cassandra.distributed.test;
+
+import org.junit.*;
+
+import org.apache.cassandra.db.ColumnFamilyStore;
+import org.apache.cassandra.db.Keyspace;
+import org.apache.cassandra.distributed.Cluster;
+
+import java.io.IOException;
+
+import static org.apache.cassandra.distributed.api.Feature.GOSSIP;
+import static org.apache.cassandra.distributed.api.Feature.NETWORK;
+
+public class ColumnFamilyStoreMBeansTest extends TestBaseImpl
+{
+ private static Cluster CLUSTER;
+
+ @BeforeClass
+ public static void setup() throws IOException
+ {
+ CLUSTER = init(Cluster.build(1).withConfig(c ->
+ c.with(GOSSIP, NETWORK))
+ .start());
+
+ CLUSTER.schemaChange(withKeyspace("DROP KEYSPACE %s"));
+ CLUSTER.schemaChange(withKeyspace("CREATE KEYSPACE %s WITH replication
= {'class': 'SimpleStrategy', 'replication_factor': 1}"));
+ CLUSTER.schemaChange(withKeyspace("CREATE TABLE %s.cf (k text, c1
text, c2 text, PRIMARY KEY (k)) WITH compaction = {'class':
'UnifiedCompactionStrategy', 'scaling_parameters': 'L10'}"));
+
+ for (int i = 0; i < 10000; i++)
+ CLUSTER.get(1).executeInternal(withKeyspace("INSERT INTO %s.cf (k,
c1, c2) VALUES (?, 'value1', 'value2');"), Integer.toString(i));
+
+ CLUSTER.get(1).nodetool("flush");
+ }
+
+ @Test
+ public void testPerLevelAverageTokenSpace() throws Throwable
+ {
+ CLUSTER.get(1).runOnInstance(() -> {
+ ColumnFamilyStore cfs =
Keyspace.open(KEYSPACE).getColumnFamilyStore("cf");
+ double[] perLevelAvgTokenSpace = cfs.getPerLevelAvgTokenSpace();
+ assert(perLevelAvgTokenSpace.length > 0);
Review Comment:
can you please use `Assert.assertTrue` instead everywhere in the test
class. Also, you can do static imports
##########
src/java/org/apache/cassandra/tools/nodetool/stats/TableStatsHolder.java:
##########
@@ -275,6 +284,66 @@ private void initializeKeyspaces(NodeProbe probe, boolean
ignore, List<String> t
}
}
+ double[] ucsSSTableTokenSpace =
table.getPerLevelAvgTokenSpace();
+ if (ucsSSTableTokenSpace != null)
+ {
+ statsTable.isUCSSstable = true;
+ for (int level = 0; level < ucsSSTableTokenSpace.length;
level++)
+ {
+
statsTable.sstableAvgTokenSpaceInEachLevel.add(String.format("%.03f",
ucsSSTableTokenSpace[level]));
+ }
+ }
+
+ double[] ucsMaxDensityThreshold =
table.getPerLevelMaxDensityThreshold();
+ if (ucsMaxDensityThreshold != null)
+ {
+ statsTable.isUCSSstable = true;
+ for (int level = 0; level < ucsMaxDensityThreshold.length;
level++)
+ {
+
statsTable.sstableMaxDensityThresholdInEachLevel.add(String.format("%.03f",
ucsMaxDensityThreshold[level]));
+ }
+ }
+
+ double[] ucsSsTableAvgSize = table.getPerLevelAvgSize();
+ if (ucsSsTableAvgSize != null)
+ {
+ statsTable.isUCSSstable = true;
+ for (int level = 0; level < ucsSsTableAvgSize.length;
level++)
+ {
+
statsTable.sstableAvgSizeInEachLevel.add(String.format("%.03f",
ucsSsTableAvgSize[level]));
+ }
+ }
+
+ double[] ucsSStableAvgDensity = table.getPerLevelAvgDensity();
+ if (ucsSStableAvgDensity != null)
+ {
+ statsTable.isUCSSstable = true;
+ for (int level = 0; level < ucsSStableAvgDensity.length;
level++)
+ {
+
statsTable.sstableAvgDensityInEachLevel.add(String.format("%.03f",
ucsSStableAvgDensity[level]));
+ }
+ }
+
+ double[] ucsSStableAvgDensityMaxDensityThresholdRatio =
table.getPerLevelAvgDensityMaxDensityThresholdRatio();
+ if (ucsSStableAvgDensityMaxDensityThresholdRatio != null)
+ {
+ statsTable.isUCSSstable = true;
+ for (int level = 0; level <
ucsSStableAvgDensityMaxDensityThresholdRatio.length; level++)
+ {
+
statsTable.sstableAvgDensityMaxDensityThresholdRatioInEachLevel.add(String.format("%.03f",
ucsSStableAvgDensityMaxDensityThresholdRatio[level]));
+ }
+ }
+
+ double[]
ucsSStableMaxDensityMaxDensityThresholdRatioInEachLevel =
table.getPerLevelMaxDensityMaxDensityThresholdRatio();
+ if (ucsSStableMaxDensityMaxDensityThresholdRatioInEachLevel !=
null)
+ {
+ statsTable.isUCSSstable = true;
+ for (int level = 0; level <
ucsSStableMaxDensityMaxDensityThresholdRatioInEachLevel.length; level++)
+ {
+
statsTable.sstableMaxDensityMaxDensityThresholdRatioInEachLevel.add(String.format("%.03f",
ucsSStableMaxDensityMaxDensityThresholdRatioInEachLevel[level]));
+ }
+ }
+
Review Comment:
All the six blocks doing almost the same thing. Please extract the if
condition and for loop into a common method like `addUCSMetric ` and call it 6
times to avoid code duplication
EX
```
addUCSMetric(table.getPerLevelAvgTokenSpace(),
statsTable.sstableAvgTokenSpaceInEachLevel,
false,
humanReadable);
```
##########
test/distributed/org/apache/cassandra/distributed/test/ColumnFamilyStoreMBeansTest.java:
##########
@@ -0,0 +1,126 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.cassandra.distributed.test;
+
+import org.junit.*;
+
+import org.apache.cassandra.db.ColumnFamilyStore;
+import org.apache.cassandra.db.Keyspace;
+import org.apache.cassandra.distributed.Cluster;
+
+import java.io.IOException;
+
+import static org.apache.cassandra.distributed.api.Feature.GOSSIP;
+import static org.apache.cassandra.distributed.api.Feature.NETWORK;
+
+public class ColumnFamilyStoreMBeansTest extends TestBaseImpl
+{
+ private static Cluster CLUSTER;
Review Comment:
Can you also add cluster cleanup after the test
##########
src/java/org/apache/cassandra/tools/nodetool/stats/TableStatsPrinter.java:
##########
@@ -100,6 +100,22 @@ protected void printStatsTable(StatsTable table, String
tableDisplayName, String
table.sstableBytesInEachLevel) + "]");
}
+ if (table.isUCSSstable)
+ {
+ out.println(indent + "Average token space for SSTables in each
level: [" + String.join(", ",
+ table.sstableAvgTokenSpaceInEachLevel) + "]");
+ out.println(indent + "Maximum density for SSTables in each
level: [" + String.join(", ",
+ table.sstableMaxDensityThresholdInEachLevel) + "]");
+ out.println(indent + "Average SSTable size in each level: [" +
String.join(", ",
+ table.sstableAvgSizeInEachLevel) + "]");
+ out.println(indent + "Average SSTable density in each level:
[" + String.join(", ",
+ table.sstableAvgDensityInEachLevel) + "]");
+ out.println(indent + "Ratio of average SSTable density and
maximum density threshold in each level: [" + String.join(", ",
+
table.sstableAvgDensityMaxDensityThresholdRatioInEachLevel) + "]");
+ out.println(indent + "Ratio of maximum SSTable density and
maximum density threshold in each level: [" + String.join(", ",
+
table.sstableMaxDensityMaxDensityThresholdRatioInEachLevel) + "]");
Review Comment:
```suggestion
out.println(indent + "Ratio of average SSTable density to
maximum density threshold in each level: [" + String.join(", ",
table.sstableAvgDensityMaxDensityThresholdRatioInEachLevel) + "]");
out.println(indent + "Ratio of maximum SSTable density to
maximum density threshold in each level: [" + String.join(", ",
table.sstableMaxDensityMaxDensityThresholdRatioInEachLevel) + "]");
```
##########
test/distributed/org/apache/cassandra/distributed/test/ColumnFamilyStoreMBeansTest.java:
##########
@@ -0,0 +1,126 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.cassandra.distributed.test;
+
+import org.junit.*;
+
+import org.apache.cassandra.db.ColumnFamilyStore;
+import org.apache.cassandra.db.Keyspace;
+import org.apache.cassandra.distributed.Cluster;
+
+import java.io.IOException;
+
+import static org.apache.cassandra.distributed.api.Feature.GOSSIP;
+import static org.apache.cassandra.distributed.api.Feature.NETWORK;
+
+public class ColumnFamilyStoreMBeansTest extends TestBaseImpl
+{
+ private static Cluster CLUSTER;
+
+ @BeforeClass
+ public static void setup() throws IOException
+ {
+ CLUSTER = init(Cluster.build(1).withConfig(c ->
+ c.with(GOSSIP, NETWORK))
+ .start());
+
+ CLUSTER.schemaChange(withKeyspace("DROP KEYSPACE %s"));
+ CLUSTER.schemaChange(withKeyspace("CREATE KEYSPACE %s WITH replication
= {'class': 'SimpleStrategy', 'replication_factor': 1}"));
+ CLUSTER.schemaChange(withKeyspace("CREATE TABLE %s.cf (k text, c1
text, c2 text, PRIMARY KEY (k)) WITH compaction = {'class':
'UnifiedCompactionStrategy', 'scaling_parameters': 'L10'}"));
+
+ for (int i = 0; i < 10000; i++)
+ CLUSTER.get(1).executeInternal(withKeyspace("INSERT INTO %s.cf (k,
c1, c2) VALUES (?, 'value1', 'value2');"), Integer.toString(i));
+
+ CLUSTER.get(1).nodetool("flush");
+ }
+
+ @Test
+ public void testPerLevelAverageTokenSpace() throws Throwable
+ {
+ CLUSTER.get(1).runOnInstance(() -> {
+ ColumnFamilyStore cfs =
Keyspace.open(KEYSPACE).getColumnFamilyStore("cf");
+ double[] perLevelAvgTokenSpace = cfs.getPerLevelAvgTokenSpace();
+ assert(perLevelAvgTokenSpace.length > 0);
+ for (int i = 0; i < perLevelAvgTokenSpace.length; i++)
+ Assert.assertTrue(perLevelAvgTokenSpace[i] > 0);
+ });
+ }
+
+ @Test
+ public void testGetPerLevelMaxDensityThreshold() throws Throwable
+ {
+ CLUSTER.get(1).runOnInstance(() -> {
+ ColumnFamilyStore cfs =
Keyspace.open(KEYSPACE).getColumnFamilyStore("cf");
+ Assert.assertTrue(cfs.getPerLevelMaxDensityThreshold().length > 0);
+ });
Review Comment:
can you add actual threshold value validation?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
