This is an automated email from the ASF dual-hosted git repository. smiklosovic pushed a commit to branch trunk in repository https://gitbox.apache.org/repos/asf/cassandra.git
commit b15a1f04b61e11bc3b9719c06bffb9fa672a25cd Author: Stefan Miklosovic <[email protected]> AuthorDate: Wed Dec 18 11:58:30 2024 +0100 Propagate true size of snapshot in SnapshotDetailsTabularData to not call JMX twice in nodetool listsnapshots This patch also opportunistically adds a new MBean method for SnapshotManager to return true snapshot size for a particular snapshot. patch by Stefan Miklosovic; reviewed by Maxim Muzafarov, Bernardo Botella for CASSANDRA-20149 --- CHANGES.txt | 1 + .../snapshot/SnapshotDetailsTabularData.java | 25 +++++---- .../service/snapshot/SnapshotManager.java | 9 ++++ .../service/snapshot/SnapshotManagerMBean.java | 12 ++++- .../cassandra/tools/nodetool/ListSnapshots.java | 21 ++++---- .../apache/cassandra/db/ColumnFamilyStoreTest.java | 62 ++++++++++++---------- 6 files changed, 84 insertions(+), 46 deletions(-) diff --git a/CHANGES.txt b/CHANGES.txt index 02a0be0571..b20e0f8b1f 100644 --- a/CHANGES.txt +++ b/CHANGES.txt @@ -1,4 +1,5 @@ 5.1 + * Propagate true size of snapshot in SnapshotDetailsTabularData to not call JMX twice in nodetool listsnapshots (CASSANDRA-20149) * Implementation of CEP-43 (CASSANDRA-19964) * Periodically disconnect roles that are revoked or have LOGIN=FALSE set (CASSANDRA-19385) * AST library for CQL-based fuzz tests (CASSANDRA-20198) diff --git a/src/java/org/apache/cassandra/service/snapshot/SnapshotDetailsTabularData.java b/src/java/org/apache/cassandra/service/snapshot/SnapshotDetailsTabularData.java index 23c84e5056..3d9609f2e7 100644 --- a/src/java/org/apache/cassandra/service/snapshot/SnapshotDetailsTabularData.java +++ b/src/java/org/apache/cassandra/service/snapshot/SnapshotDetailsTabularData.java @@ -18,13 +18,18 @@ package org.apache.cassandra.service.snapshot; import java.util.Set; -import javax.management.openmbean.*; +import javax.management.openmbean.CompositeDataSupport; +import javax.management.openmbean.CompositeType; +import javax.management.openmbean.OpenDataException; +import javax.management.openmbean.OpenType; +import javax.management.openmbean.SimpleType; +import javax.management.openmbean.TabularDataSupport; +import javax.management.openmbean.TabularType; import org.apache.cassandra.io.util.FileUtils; public class SnapshotDetailsTabularData { - private static final String[] ITEM_NAMES = new String[]{"Snapshot name", "Keyspace name", "Column family name", @@ -32,7 +37,8 @@ public class SnapshotDetailsTabularData "Size on disk", "Creation time", "Expiration time", - "Ephemeral"}; + "Ephemeral", + "Raw true size"}; private static final String[] ITEM_DESCS = new String[]{"snapshot_name", "keyspace_name", @@ -41,7 +47,8 @@ public class SnapshotDetailsTabularData "TotalDiskSpaceUsed", "created_at", "expires_at", - "ephemeral"}; + "ephemeral", + "raw_true_disk_space_used"}; private static final String TYPE_NAME = "SnapshotDetails"; @@ -57,7 +64,7 @@ public class SnapshotDetailsTabularData { try { - ITEM_TYPES = new OpenType[]{ SimpleType.STRING, SimpleType.STRING, SimpleType.STRING, SimpleType.STRING, SimpleType.STRING, SimpleType.STRING, SimpleType.STRING, SimpleType.STRING }; + ITEM_TYPES = new OpenType[]{ SimpleType.STRING, SimpleType.STRING, SimpleType.STRING, SimpleType.STRING, SimpleType.STRING, SimpleType.STRING, SimpleType.STRING, SimpleType.STRING, SimpleType.LONG }; COMPOSITE_TYPE = new CompositeType(TYPE_NAME, ROW_DESC, ITEM_NAMES, ITEM_DESCS, ITEM_TYPES); @@ -74,14 +81,14 @@ public class SnapshotDetailsTabularData { try { - final String totalSize = FileUtils.stringifyFileSize(details.computeSizeOnDiskBytes()); + String totalSize = FileUtils.stringifyFileSize(details.computeSizeOnDiskBytes()); long trueSizeBytes = details.computeTrueSizeBytes(files); - final String liveSize = FileUtils.stringifyFileSize(details.computeTrueSizeBytes()); + String liveSize = FileUtils.stringifyFileSize(trueSizeBytes); String createdAt = safeToString(details.getCreatedAt()); String expiresAt = safeToString(details.getExpiresAt()); String ephemeral = Boolean.toString(details.isEphemeral()); result.put(new CompositeDataSupport(COMPOSITE_TYPE, ITEM_NAMES, - new Object[]{ details.getTag(), details.getKeyspaceName(), details.getTableName(), liveSize, totalSize, createdAt, expiresAt, ephemeral })); + new Object[]{ details.getTag(), details.getKeyspaceName(), details.getTableName(), liveSize, totalSize, createdAt, expiresAt, ephemeral, trueSizeBytes })); } catch (OpenDataException e) { @@ -93,4 +100,4 @@ public class SnapshotDetailsTabularData { return object == null ? null : object.toString(); } -} \ No newline at end of file +} diff --git a/src/java/org/apache/cassandra/service/snapshot/SnapshotManager.java b/src/java/org/apache/cassandra/service/snapshot/SnapshotManager.java index bea0c04669..f7fa1e4ece 100644 --- a/src/java/org/apache/cassandra/service/snapshot/SnapshotManager.java +++ b/src/java/org/apache/cassandra/service/snapshot/SnapshotManager.java @@ -471,6 +471,15 @@ public class SnapshotManager implements SnapshotManagerMBean, INotificationConsu return new TrueSnapshotSizeTask(this, s -> s.getKeyspaceName().equals(keyspace) && s.getTableName().equals(table)).call(); } + @Override + public long getTrueSnapshotsSize(String keyspace, String table, String snapshotName) + { + return new TrueSnapshotSizeTask(this, + s -> s.getKeyspaceName().equals(keyspace) + && s.getTableName().equals(table) + && s.getTag().equals(snapshotName)).call(); + } + @Override public void setSnapshotLinksPerSecond(long throttle) { diff --git a/src/java/org/apache/cassandra/service/snapshot/SnapshotManagerMBean.java b/src/java/org/apache/cassandra/service/snapshot/SnapshotManagerMBean.java index 2d69a7ca97..214adcc445 100644 --- a/src/java/org/apache/cassandra/service/snapshot/SnapshotManagerMBean.java +++ b/src/java/org/apache/cassandra/service/snapshot/SnapshotManagerMBean.java @@ -78,7 +78,7 @@ public interface SnapshotManagerMBean long getTrueSnapshotsSize(String keyspace); /** - * Get the true size take by all snapshots in given keyspace and table. + * Get the true size taken by all snapshots in given keyspace and table. * * @param keyspace keyspace to get true size of all snapshots of * @param table table in a keyspace to get true size of all snapshots of @@ -86,6 +86,16 @@ public interface SnapshotManagerMBean */ long getTrueSnapshotsSize(String keyspace, String table); + /** + * Get the true size of a snapshot in given keyspace and table. + * + * @param keyspace keyspace to get true size of all snapshots of + * @param table table in a keyspace to get true size of all snapshots of + * @param snapshotName name of snapshot in given keyspace and table to get true size of + * @return true size of all snapshots in given keyspace and table + */ + long getTrueSnapshotsSize(String keyspace, String table, String snapshotName); + /** * Set the current hardlink-per-second throttle for snapshots * A setting of zero indicates no throttling diff --git a/src/java/org/apache/cassandra/tools/nodetool/ListSnapshots.java b/src/java/org/apache/cassandra/tools/nodetool/ListSnapshots.java index 6a1c71d386..e1b61d4b51 100644 --- a/src/java/org/apache/cassandra/tools/nodetool/ListSnapshots.java +++ b/src/java/org/apache/cassandra/tools/nodetool/ListSnapshots.java @@ -25,7 +25,6 @@ import java.util.Set; import javax.management.openmbean.TabularData; import io.airlift.airline.Command; - import io.airlift.airline.Option; import org.apache.cassandra.io.util.FileUtils; import org.apache.cassandra.tools.NodeProbe; @@ -64,15 +63,17 @@ public class ListSnapshots extends NodeToolCmd return; } - final long trueSnapshotsSize = probe.trueSnapshotsSize(); TableBuilder table = new TableBuilder(); // display column names only once - final List<String> indexNames = snapshotDetails.entrySet().iterator().next().getValue().getTabularType().getIndexNames(); + List<String> indexNames = snapshotDetails.entrySet().iterator().next().getValue().getTabularType().getIndexNames(); + indexNames.subList(0, indexNames.size() - 1); if (includeEphemeral) - table.add(indexNames.toArray(new String[indexNames.size()])); - else table.add(indexNames.subList(0, indexNames.size() - 1).toArray(new String[indexNames.size() - 1])); + else + table.add(indexNames.subList(0, indexNames.size() - 2).toArray(new String[indexNames.size() - 2])); + + long totalTrueDiskSize = 0; for (final Map.Entry<String, TabularData> snapshotDetail : snapshotDetails.entrySet()) { @@ -81,18 +82,20 @@ public class ListSnapshots extends NodeToolCmd { final List<?> value = (List<?>) eachValue; if (includeEphemeral) - table.add(value.toArray(new String[value.size()])); - else table.add(value.subList(0, value.size() - 1).toArray(new String[value.size() - 1])); + else + table.add(value.subList(0, value.size() - 2).toArray(new String[value.size() - 2])); + + totalTrueDiskSize += (Long) value.get(value.size() - 1); } } table.printTo(out); - out.println("\nTotal TrueDiskSpaceUsed: " + FileUtils.stringifyFileSize(trueSnapshotsSize) + '\n'); + out.println("\nTotal TrueDiskSpaceUsed: " + FileUtils.stringifyFileSize(totalTrueDiskSize) + '\n'); } catch (Exception e) { throw new RuntimeException("Error during list snapshot", e); } } -} \ No newline at end of file +} diff --git a/test/unit/org/apache/cassandra/db/ColumnFamilyStoreTest.java b/test/unit/org/apache/cassandra/db/ColumnFamilyStoreTest.java index ba08437cd6..7f5b28c759 100644 --- a/test/unit/org/apache/cassandra/db/ColumnFamilyStoreTest.java +++ b/test/unit/org/apache/cassandra/db/ColumnFamilyStoreTest.java @@ -382,18 +382,21 @@ public class ColumnFamilyStoreTest assertThat(cfs.trueSnapshotsSize()).isZero(); - SnapshotManager.instance.takeSnapshot("snapshot_without_index", cfs.getKeyspaceTableName()); + SnapshotManager.instance.takeSnapshot("snapshot0_without_index", cfs.getKeyspaceTableName()); - long firstSnapshotsSize = cfs.trueSnapshotsSize(); + long sizeOfOneSnapshot = cfs.trueSnapshotsSize(); + // test that true snapshot size computed from manager is same as true snapshot size from cfs with one snapshot + long trueSnapshotSizeViaManagerWithoutIndex = SnapshotManager.instance.getTrueSnapshotsSize(cfs.getKeyspaceName(), cfs.getTableName(), "snapshot0_without_index"); + Assert.assertEquals(sizeOfOneSnapshot, trueSnapshotSizeViaManagerWithoutIndex); Map<String, TableSnapshot> listedSnapshots = Util.listSnapshots(cfs); - assertThat(firstSnapshotsSize).isPositive(); + assertThat(sizeOfOneSnapshot).isPositive(); assertThat(listedSnapshots.size()).isEqualTo(1); - assertThat(listedSnapshots.get("snapshot_without_index")).isNotNull(); - long withoutIndexSize = listedSnapshots.get("snapshot_without_index").computeSizeOnDiskBytes(); - long withoutIndexTrueSize = listedSnapshots.get("snapshot_without_index").computeTrueSizeBytes(); + assertThat(listedSnapshots.get("snapshot0_without_index")).isNotNull(); + long withoutIndexSize = listedSnapshots.get("snapshot0_without_index").computeSizeOnDiskBytes(); + long withoutIndexTrueSize = listedSnapshots.get("snapshot0_without_index").computeTrueSizeBytes(); assertThat(withoutIndexSize).isGreaterThan(withoutIndexTrueSize); - assertEquals(firstSnapshotsSize, withoutIndexTrueSize); + assertEquals(sizeOfOneSnapshot, withoutIndexTrueSize); // add index, trueSnapshotSize should reflect that ColumnIdentifier col = ColumnIdentifier.getInterned("val", true); @@ -414,36 +417,41 @@ public class ColumnFamilyStoreTest rebuildIndices(cfs); - SnapshotManager.instance.takeSnapshot("snapshot_with_index", new HashMap<>(), cfs.getKeyspaceTableName()); + SnapshotManager.instance.takeSnapshot("snapshot1_with_index", new HashMap<>(), cfs.getKeyspaceTableName()); - long secondSnapshotSize = cfs.trueSnapshotsSize(); - Map<String, TableSnapshot> secondListedSnapshots = Util.listSnapshots(cfs); - assertThat(secondSnapshotSize).isPositive(); - assertThat(secondSnapshotSize).isGreaterThan(firstSnapshotsSize); + long sizeOfTwoSnapshots = cfs.trueSnapshotsSize(); + long trueSnapshotSizeViaManagerWithIndex = SnapshotManager.instance.getTrueSnapshotsSize(cfs.getKeyspaceName(), cfs.getTableName(), "snapshot1_with_index"); - assertThat(secondListedSnapshots.size()).isEqualTo(2); - assertThat(secondListedSnapshots.get("snapshot_with_index")).isNotNull(); - assertThat(secondListedSnapshots.get("snapshot_without_index")).isNotNull(); + assertThat(trueSnapshotSizeViaManagerWithIndex).isGreaterThan(trueSnapshotSizeViaManagerWithoutIndex); - long withIndexSize = secondListedSnapshots.get("snapshot_with_index").computeSizeOnDiskBytes(); - long withIndexTrueSize = secondListedSnapshots.get("snapshot_with_index").computeTrueSizeBytes(); + assertThat(sizeOfTwoSnapshots).isPositive(); + assertThat(sizeOfTwoSnapshots).isGreaterThan(sizeOfOneSnapshot); + + listedSnapshots = Util.listSnapshots(cfs); + assertThat(listedSnapshots.size()).isEqualTo(2); + assertThat(listedSnapshots.get("snapshot1_with_index")).isNotNull(); + assertThat(listedSnapshots.get("snapshot0_without_index")).isNotNull(); + + long withIndexSize = listedSnapshots.get("snapshot1_with_index").computeSizeOnDiskBytes(); + long withIndexTrueSize = listedSnapshots.get("snapshot1_with_index").computeTrueSizeBytes(); assertThat(withIndexSize).isGreaterThan(withIndexTrueSize); - assertEquals(secondSnapshotSize, withIndexTrueSize + withoutIndexTrueSize); + assertEquals(sizeOfTwoSnapshots, withIndexTrueSize + withoutIndexTrueSize); // taking another one is basically a copy of the previous - SnapshotManager.instance.takeSnapshot("another_snapshot_with_index", new HashMap<>(), cfs.getKeyspaceTableName()); + SnapshotManager.instance.takeSnapshot("snapshot2_with_index", new HashMap<>(), cfs.getKeyspaceTableName()); - long thirdSnapshotSize = cfs.trueSnapshotsSize(); - Map<String, TableSnapshot> thirdListedSnapshots = Util.listSnapshots(cfs); - assertThat(thirdSnapshotSize).isPositive(); - assertThat(thirdSnapshotSize).isGreaterThan(secondSnapshotSize); + long sizeOfThreeSnapshots = cfs.trueSnapshotsSize(); + assertThat(sizeOfThreeSnapshots).isPositive(); + assertThat(sizeOfThreeSnapshots).isGreaterThan(sizeOfTwoSnapshots); - long anotherWithIndexSize = thirdListedSnapshots.get("another_snapshot_with_index").computeSizeOnDiskBytes(); - long anotherWithIndexTrueSize = thirdListedSnapshots.get("another_snapshot_with_index").computeTrueSizeBytes(); + listedSnapshots = Util.listSnapshots(cfs); + long anotherWithIndexSize = listedSnapshots.get("snapshot2_with_index").computeSizeOnDiskBytes(); + long anotherWithIndexTrueSize = listedSnapshots.get("snapshot2_with_index").computeTrueSizeBytes(); - assertEquals(withIndexSize, anotherWithIndexSize); - assertEquals(withIndexTrueSize, anotherWithIndexTrueSize); + // TODO CASSANDRA-20209 + assertTrue(withIndexSize == anotherWithIndexSize || (withIndexSize + 4 == anotherWithIndexSize) || (withIndexSize - 4 == anotherWithIndexSize)); + assertTrue(withIndexTrueSize == anotherWithIndexTrueSize || (withIndexTrueSize + 4 == anotherWithIndexTrueSize) || (withIndexTrueSize - 4 == anotherWithIndexTrueSize)); } private void rebuildIndices(ColumnFamilyStore cfs) --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
