bbotella commented on code in PR #3750:
URL: https://github.com/apache/cassandra/pull/3750#discussion_r1890523734
##########
.circleci/config.yml:
##########
Review Comment:
Are the changes in circle config expected?
##########
src/java/org/apache/cassandra/service/snapshot/SnapshotDetailsTabularData.java:
##########
@@ -18,21 +18,27 @@
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",
"True size",
"Size on disk",
"Creation time",
"Expiration time",
- "Ephemeral"};
+ "Ephemeral",
+ "Raw true size"};
Review Comment:
Do we need the `true` portion of the name? It somewhat makes it look like we
have a non true size :-P
##########
src/java/org/apache/cassandra/tools/nodetool/ListSnapshots.java:
##########
@@ -81,18 +82,20 @@ public void execute(NodeProbe probe)
{
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');
Review Comment:
I'm worried that changing the "definition" of an already defined field may
break some users of this call. I understand that this "breaking change" is
gonna live in trunk for the time being, but we need to make sure this change is
properly documented.
--
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]
