jrwest commented on code in PR #3733:
URL: https://github.com/apache/cassandra/pull/3733#discussion_r1886166443
##########
src/java/org/apache/cassandra/tools/nodetool/Status.java:
##########
@@ -24,38 +25,232 @@
import java.io.PrintStream;
import java.net.UnknownHostException;
import java.text.DecimalFormat;
+import java.util.ArrayList;
import java.util.Collection;
+import java.util.HashMap;
import java.util.Iterator;
+import java.util.LinkedHashMap;
import java.util.List;
import java.util.Map;
import java.util.SortedMap;
+import org.apache.cassandra.io.util.FileUtils;
import org.apache.cassandra.locator.EndpointSnitchInfoMBean;
+import org.apache.cassandra.locator.InetAddressAndPort;
import org.apache.cassandra.tools.NodeProbe;
import org.apache.cassandra.tools.NodeTool;
import org.apache.cassandra.tools.NodeTool.NodeToolCmd;
import org.apache.cassandra.tools.nodetool.formatter.TableBuilder;
import com.google.common.collect.ArrayListMultimap;
+import static java.util.stream.Collectors.toMap;
+
@SuppressWarnings("UseOfSystemOutOrSystemErr")
@Command(name = "status", description = "Print cluster information (state,
load, IDs, ...)")
-public class Status extends NodeToolCmd
-{
+public class Status extends NodeToolCmd {
@Arguments(usage = "[<keyspace>]", description = "The keyspace name")
private String keyspace = null;
@Option(title = "resolve_ip", name = {"-r", "--resolve-ip"}, description =
"Show node domain names instead of IPs")
private boolean resolveIp = false;
+ @Option(title = "sort",
+ name = {"-s", "--sort"},
+ description = "Sort by one of 'ip', 'load', 'owns', 'id', 'rack'
or 'state', Defaults to 'none', Default Ordering is 'asc' for 'ip', 'id',
'rack' and 'desc' for 'load', 'owns', 'state' ",
+ allowedValues = {"ip", "load", "owns", "id", "rack", "state",
"none"})
+ private SortBy sortBy = SortBy.none;
Review Comment:
Making these values enums makes the error messages when you pass an invalid
value a bit opaque. On one hand, I would say its fine to leave it because the
output tells you to check `nodetool help` which does tell you the valid values.
OTOH, having it tell you the valid values in the error message would be a
better user experience (and a removal of a rough edge nodetool often has). To
fix this though I think we would need to take the arguments as strings and then
parse them in the body of the command so we could catch the exception and throw
our own error message:
```
$ nt status --sort blah
nodetool: sort: can not convert "blah" to a SortBy
See 'nodetool help' or 'nodetool help <command>'.
$ nt status --sort ip --order blah
nodetool: sort_order: can not convert "blah" to a SortOrder
See 'nodetool help' or 'nodetool help <command>'.
```
##########
src/java/org/apache/cassandra/tools/nodetool/Status.java:
##########
@@ -24,38 +25,232 @@
import java.io.PrintStream;
import java.net.UnknownHostException;
import java.text.DecimalFormat;
+import java.util.ArrayList;
import java.util.Collection;
+import java.util.HashMap;
import java.util.Iterator;
+import java.util.LinkedHashMap;
import java.util.List;
import java.util.Map;
import java.util.SortedMap;
+import org.apache.cassandra.io.util.FileUtils;
import org.apache.cassandra.locator.EndpointSnitchInfoMBean;
+import org.apache.cassandra.locator.InetAddressAndPort;
import org.apache.cassandra.tools.NodeProbe;
import org.apache.cassandra.tools.NodeTool;
import org.apache.cassandra.tools.NodeTool.NodeToolCmd;
import org.apache.cassandra.tools.nodetool.formatter.TableBuilder;
import com.google.common.collect.ArrayListMultimap;
+import static java.util.stream.Collectors.toMap;
+
@SuppressWarnings("UseOfSystemOutOrSystemErr")
@Command(name = "status", description = "Print cluster information (state,
load, IDs, ...)")
-public class Status extends NodeToolCmd
-{
+public class Status extends NodeToolCmd {
@Arguments(usage = "[<keyspace>]", description = "The keyspace name")
private String keyspace = null;
@Option(title = "resolve_ip", name = {"-r", "--resolve-ip"}, description =
"Show node domain names instead of IPs")
private boolean resolveIp = false;
+ @Option(title = "sort",
+ name = {"-s", "--sort"},
+ description = "Sort by one of 'ip', 'load', 'owns', 'id', 'rack'
or 'state', Defaults to 'none', Default Ordering is 'asc' for 'ip', 'id',
'rack' and 'desc' for 'load', 'owns', 'state' ",
+ allowedValues = {"ip", "load", "owns", "id", "rack", "state",
"none"})
+ private SortBy sortBy = SortBy.none;
+
+ @Option(title = "sort_order",
+ name = {"-o", "--order"},
+ description = "Sort order: 'asc' for ascending, 'desc' for
descending",
+ allowedValues = {"asc", "desc"})
+ private SortOrder sortOrder = null;
+
private boolean isTokenPerNode = true;
private Collection<String> joiningNodes, leavingNodes, movingNodes,
liveNodes, unreachableNodes;
private Map<String, String> loadMap, hostIDMap;
private EndpointSnitchInfoMBean epSnitchInfo;
+ public enum SortOrder {
+ asc,
+ desc
+ }
+
+ public enum SortBy
+ {
+ none
+ {
+ @Override
+ public Map<String, List<Object>> sort(Map<String,
List<Object>> data)
+ {
+ return data;
+ }
+ },
+ ip(false)
+ {
+ @Override
+ public Map<String, List<Object>> sort(Map<String,
List<Object>> data)
+ {
+ return data.entrySet()
+ .stream()
+ .sorted((e1, e2) -> {
+ InetAddressAndPort addr1 =
(InetAddressAndPort) e1.getValue().get(0);
+ InetAddressAndPort addr2 =
(InetAddressAndPort) e2.getValue().get(0);
+
+ return
evaluateComparision(addr1.compareTo(addr2));
+ })
+ .collect(toMap(Map.Entry::getKey,
Map.Entry::getValue, (e1, e2) -> e1, LinkedHashMap::new));
+ }
+ },
+ load(true)
+ {
+ @Override
+ public Map<String, List<Object>> sort(Map<String,
List<Object>> data)
+ {
+ return data.entrySet()
+ .stream()
+ .sorted((e1, e2) -> {
+ String str1 = (String)
e1.getValue().get(3);
+ String str2 = (String)
e2.getValue().get(3);
+ // Check if str1 or str2 contains a '?'
and set a value for it.
+ boolean containsQuestionMark1 =
str1.contains("?");
+ boolean containsQuestionMark2 =
str2.contains("?");
+ if (containsQuestionMark1 &&
containsQuestionMark2) {
+ // If both contain '?', return 0 (they
are considered equal).
+ return 0;
+ }
+
+ boolean descending = descending(sortOrder);
+
+ // If str1 contains '?', ensure it's last
(or first depending on descending).
+ if (containsQuestionMark1)
+ return descending ? 1 : -1;
+
+ // If str2 contains '?', ensure it's last
(or first depending on descending).
+ if (containsQuestionMark2)
+ return descending ? -1 : 1;
+
+ // If neither contain '?', parse the file
sizes and compare.
+ long value1 =
FileUtils.parseFileSize((String) e1.getValue().get(3));
+ long value2 =
FileUtils.parseFileSize((String) e2.getValue().get(3));
+
+ return
evaluateComparision(Long.compare(value1, value2));
+ })
+ .collect(toMap(Map.Entry::getKey,
Map.Entry::getValue, (e1, e2) -> e1, LinkedHashMap::new));
+ }
+ },
+ owns(true)
+ {
+ @Override
+ public Map<String, List<Object>> sort(Map<String,
List<Object>> data)
+ {
+ return data.entrySet()
+ .stream()
+ .sorted((e1, e2) -> {
+ double value1 =
Double.parseDouble(((String) e1.getValue().get(tokenPerNode ? 4 :
5)).replace("%", ""));
+ double value2 =
Double.parseDouble(((String) e2.getValue().get(tokenPerNode ? 4 :
5)).replace("%", ""));
+
+ return
evaluateComparision(Double.compare(value1, value2));
+ })
+ .collect(toMap(Map.Entry::getKey,
Map.Entry::getValue, (e1, e2) -> e1, LinkedHashMap::new));
+ }
+ },
+ id(false)
+ {
+ @Override
+ public Map<String, List<Object>> sort(Map<String,
List<Object>> data)
+ {
+ return compareStrings(data, tokenPerNode ? 5 : 6);
+ }
+ },
+ rack(false)
+ {
+ @Override
+ public Map<String, List<Object>> sort(Map<String,
List<Object>> data)
+ {
+ return compareStrings(data, 7);
+ }
+ },
+ state(true)
+ {
+ @Override
+ public Map<String, List<Object>> sort(Map<String,
List<Object>> data)
+ {
+ return compareStrings(data, 1);
+ }
+ };
+
+ private final boolean descendingByDefault;
+ boolean tokenPerNode;
+ SortOrder sortOrder;
+
+ SortBy()
+ {
+ this(false);
+ }
+
+ SortBy(boolean descendingByDefault)
+ {
+ this.descendingByDefault = descendingByDefault;
+ }
+
+ public abstract Map<String, List<Object>> sort(Map<String,
List<Object>> data);
+
+ boolean descending(SortOrder sortOrder)
Review Comment:
should be marked protected
##########
src/java/org/apache/cassandra/tools/nodetool/Status.java:
##########
@@ -24,38 +25,232 @@
import java.io.PrintStream;
import java.net.UnknownHostException;
import java.text.DecimalFormat;
+import java.util.ArrayList;
import java.util.Collection;
+import java.util.HashMap;
import java.util.Iterator;
+import java.util.LinkedHashMap;
import java.util.List;
import java.util.Map;
import java.util.SortedMap;
+import org.apache.cassandra.io.util.FileUtils;
import org.apache.cassandra.locator.EndpointSnitchInfoMBean;
+import org.apache.cassandra.locator.InetAddressAndPort;
import org.apache.cassandra.tools.NodeProbe;
import org.apache.cassandra.tools.NodeTool;
import org.apache.cassandra.tools.NodeTool.NodeToolCmd;
import org.apache.cassandra.tools.nodetool.formatter.TableBuilder;
import com.google.common.collect.ArrayListMultimap;
+import static java.util.stream.Collectors.toMap;
+
@SuppressWarnings("UseOfSystemOutOrSystemErr")
@Command(name = "status", description = "Print cluster information (state,
load, IDs, ...)")
-public class Status extends NodeToolCmd
-{
+public class Status extends NodeToolCmd {
@Arguments(usage = "[<keyspace>]", description = "The keyspace name")
private String keyspace = null;
@Option(title = "resolve_ip", name = {"-r", "--resolve-ip"}, description =
"Show node domain names instead of IPs")
private boolean resolveIp = false;
+ @Option(title = "sort",
+ name = {"-s", "--sort"},
+ description = "Sort by one of 'ip', 'load', 'owns', 'id', 'rack'
or 'state', Defaults to 'none', Default Ordering is 'asc' for 'ip', 'id',
'rack' and 'desc' for 'load', 'owns', 'state' ",
+ allowedValues = {"ip", "load", "owns", "id", "rack", "state",
"none"})
+ private SortBy sortBy = SortBy.none;
+
+ @Option(title = "sort_order",
+ name = {"-o", "--order"},
+ description = "Sort order: 'asc' for ascending, 'desc' for
descending",
+ allowedValues = {"asc", "desc"})
+ private SortOrder sortOrder = null;
+
private boolean isTokenPerNode = true;
private Collection<String> joiningNodes, leavingNodes, movingNodes,
liveNodes, unreachableNodes;
private Map<String, String> loadMap, hostIDMap;
private EndpointSnitchInfoMBean epSnitchInfo;
+ public enum SortOrder {
+ asc,
+ desc
+ }
+
+ public enum SortBy
Review Comment:
If I am reading these right, each implementation duplicates a lot of the
same code in their `sort` implementations. The only difference is the
comparator. We could instead define the comparator and have one sort
implementation which would reduce a lot of duplication and LOC.
##########
src/java/org/apache/cassandra/tools/nodetool/Status.java:
##########
@@ -24,38 +25,232 @@
import java.io.PrintStream;
import java.net.UnknownHostException;
import java.text.DecimalFormat;
+import java.util.ArrayList;
import java.util.Collection;
+import java.util.HashMap;
import java.util.Iterator;
+import java.util.LinkedHashMap;
import java.util.List;
import java.util.Map;
import java.util.SortedMap;
+import org.apache.cassandra.io.util.FileUtils;
import org.apache.cassandra.locator.EndpointSnitchInfoMBean;
+import org.apache.cassandra.locator.InetAddressAndPort;
import org.apache.cassandra.tools.NodeProbe;
import org.apache.cassandra.tools.NodeTool;
import org.apache.cassandra.tools.NodeTool.NodeToolCmd;
import org.apache.cassandra.tools.nodetool.formatter.TableBuilder;
import com.google.common.collect.ArrayListMultimap;
+import static java.util.stream.Collectors.toMap;
+
@SuppressWarnings("UseOfSystemOutOrSystemErr")
@Command(name = "status", description = "Print cluster information (state,
load, IDs, ...)")
-public class Status extends NodeToolCmd
-{
+public class Status extends NodeToolCmd {
@Arguments(usage = "[<keyspace>]", description = "The keyspace name")
private String keyspace = null;
@Option(title = "resolve_ip", name = {"-r", "--resolve-ip"}, description =
"Show node domain names instead of IPs")
private boolean resolveIp = false;
+ @Option(title = "sort",
+ name = {"-s", "--sort"},
+ description = "Sort by one of 'ip', 'load', 'owns', 'id', 'rack'
or 'state', Defaults to 'none', Default Ordering is 'asc' for 'ip', 'id',
'rack' and 'desc' for 'load', 'owns', 'state' ",
+ allowedValues = {"ip", "load", "owns", "id", "rack", "state",
"none"})
+ private SortBy sortBy = SortBy.none;
+
+ @Option(title = "sort_order",
+ name = {"-o", "--order"},
+ description = "Sort order: 'asc' for ascending, 'desc' for
descending",
+ allowedValues = {"asc", "desc"})
+ private SortOrder sortOrder = null;
+
private boolean isTokenPerNode = true;
private Collection<String> joiningNodes, leavingNodes, movingNodes,
liveNodes, unreachableNodes;
private Map<String, String> loadMap, hostIDMap;
private EndpointSnitchInfoMBean epSnitchInfo;
+ public enum SortOrder {
+ asc,
+ desc
+ }
+
+ public enum SortBy
+ {
+ none
+ {
+ @Override
+ public Map<String, List<Object>> sort(Map<String,
List<Object>> data)
+ {
+ return data;
+ }
+ },
+ ip(false)
+ {
+ @Override
+ public Map<String, List<Object>> sort(Map<String,
List<Object>> data)
+ {
+ return data.entrySet()
+ .stream()
+ .sorted((e1, e2) -> {
+ InetAddressAndPort addr1 =
(InetAddressAndPort) e1.getValue().get(0);
+ InetAddressAndPort addr2 =
(InetAddressAndPort) e2.getValue().get(0);
+
+ return
evaluateComparision(addr1.compareTo(addr2));
+ })
+ .collect(toMap(Map.Entry::getKey,
Map.Entry::getValue, (e1, e2) -> e1, LinkedHashMap::new));
+ }
+ },
+ load(true)
+ {
+ @Override
+ public Map<String, List<Object>> sort(Map<String,
List<Object>> data)
+ {
+ return data.entrySet()
+ .stream()
+ .sorted((e1, e2) -> {
+ String str1 = (String)
e1.getValue().get(3);
+ String str2 = (String)
e2.getValue().get(3);
+ // Check if str1 or str2 contains a '?'
and set a value for it.
+ boolean containsQuestionMark1 =
str1.contains("?");
+ boolean containsQuestionMark2 =
str2.contains("?");
+ if (containsQuestionMark1 &&
containsQuestionMark2) {
+ // If both contain '?', return 0 (they
are considered equal).
+ return 0;
+ }
+
+ boolean descending = descending(sortOrder);
+
+ // If str1 contains '?', ensure it's last
(or first depending on descending).
+ if (containsQuestionMark1)
+ return descending ? 1 : -1;
+
+ // If str2 contains '?', ensure it's last
(or first depending on descending).
+ if (containsQuestionMark2)
+ return descending ? -1 : 1;
+
+ // If neither contain '?', parse the file
sizes and compare.
+ long value1 =
FileUtils.parseFileSize((String) e1.getValue().get(3));
+ long value2 =
FileUtils.parseFileSize((String) e2.getValue().get(3));
+
+ return
evaluateComparision(Long.compare(value1, value2));
+ })
+ .collect(toMap(Map.Entry::getKey,
Map.Entry::getValue, (e1, e2) -> e1, LinkedHashMap::new));
+ }
+ },
+ owns(true)
+ {
+ @Override
+ public Map<String, List<Object>> sort(Map<String,
List<Object>> data)
+ {
+ return data.entrySet()
+ .stream()
+ .sorted((e1, e2) -> {
+ double value1 =
Double.parseDouble(((String) e1.getValue().get(tokenPerNode ? 4 :
5)).replace("%", ""));
+ double value2 =
Double.parseDouble(((String) e2.getValue().get(tokenPerNode ? 4 :
5)).replace("%", ""));
+
+ return
evaluateComparision(Double.compare(value1, value2));
+ })
+ .collect(toMap(Map.Entry::getKey,
Map.Entry::getValue, (e1, e2) -> e1, LinkedHashMap::new));
+ }
+ },
+ id(false)
+ {
+ @Override
+ public Map<String, List<Object>> sort(Map<String,
List<Object>> data)
+ {
+ return compareStrings(data, tokenPerNode ? 5 : 6);
+ }
+ },
+ rack(false)
+ {
+ @Override
+ public Map<String, List<Object>> sort(Map<String,
List<Object>> data)
+ {
+ return compareStrings(data, 7);
+ }
+ },
+ state(true)
+ {
+ @Override
+ public Map<String, List<Object>> sort(Map<String,
List<Object>> data)
+ {
+ return compareStrings(data, 1);
+ }
+ };
+
+ private final boolean descendingByDefault;
+ boolean tokenPerNode;
+ SortOrder sortOrder;
+
+ SortBy()
+ {
+ this(false);
+ }
+
+ SortBy(boolean descendingByDefault)
+ {
+ this.descendingByDefault = descendingByDefault;
+ }
+
+ public abstract Map<String, List<Object>> sort(Map<String,
List<Object>> data);
+
+ boolean descending(SortOrder sortOrder)
+ {
+ return sortOrder == null ? descendingByDefault : sortOrder ==
SortOrder.desc;
+ }
+
+ SortBy sortOrder(SortOrder sortOrder)
+ {
+ this.sortOrder = sortOrder;
+ return this;
+ }
+
+ SortBy tokenPerNode(boolean tokenPerNode)
Review Comment:
should be marked private or protected
##########
src/java/org/apache/cassandra/tools/nodetool/Status.java:
##########
@@ -24,38 +25,232 @@
import java.io.PrintStream;
import java.net.UnknownHostException;
import java.text.DecimalFormat;
+import java.util.ArrayList;
import java.util.Collection;
+import java.util.HashMap;
import java.util.Iterator;
+import java.util.LinkedHashMap;
import java.util.List;
import java.util.Map;
import java.util.SortedMap;
+import org.apache.cassandra.io.util.FileUtils;
import org.apache.cassandra.locator.EndpointSnitchInfoMBean;
+import org.apache.cassandra.locator.InetAddressAndPort;
import org.apache.cassandra.tools.NodeProbe;
import org.apache.cassandra.tools.NodeTool;
import org.apache.cassandra.tools.NodeTool.NodeToolCmd;
import org.apache.cassandra.tools.nodetool.formatter.TableBuilder;
import com.google.common.collect.ArrayListMultimap;
+import static java.util.stream.Collectors.toMap;
+
@SuppressWarnings("UseOfSystemOutOrSystemErr")
@Command(name = "status", description = "Print cluster information (state,
load, IDs, ...)")
-public class Status extends NodeToolCmd
-{
+public class Status extends NodeToolCmd {
@Arguments(usage = "[<keyspace>]", description = "The keyspace name")
private String keyspace = null;
@Option(title = "resolve_ip", name = {"-r", "--resolve-ip"}, description =
"Show node domain names instead of IPs")
private boolean resolveIp = false;
+ @Option(title = "sort",
+ name = {"-s", "--sort"},
+ description = "Sort by one of 'ip', 'load', 'owns', 'id', 'rack'
or 'state', Defaults to 'none', Default Ordering is 'asc' for 'ip', 'id',
'rack' and 'desc' for 'load', 'owns', 'state' ",
+ allowedValues = {"ip", "load", "owns", "id", "rack", "state",
"none"})
+ private SortBy sortBy = SortBy.none;
+
+ @Option(title = "sort_order",
+ name = {"-o", "--order"},
+ description = "Sort order: 'asc' for ascending, 'desc' for
descending",
+ allowedValues = {"asc", "desc"})
+ private SortOrder sortOrder = null;
+
private boolean isTokenPerNode = true;
private Collection<String> joiningNodes, leavingNodes, movingNodes,
liveNodes, unreachableNodes;
private Map<String, String> loadMap, hostIDMap;
private EndpointSnitchInfoMBean epSnitchInfo;
+ public enum SortOrder {
+ asc,
+ desc
+ }
+
+ public enum SortBy
+ {
+ none
+ {
+ @Override
+ public Map<String, List<Object>> sort(Map<String,
List<Object>> data)
+ {
+ return data;
+ }
+ },
+ ip(false)
+ {
+ @Override
+ public Map<String, List<Object>> sort(Map<String,
List<Object>> data)
+ {
+ return data.entrySet()
+ .stream()
+ .sorted((e1, e2) -> {
+ InetAddressAndPort addr1 =
(InetAddressAndPort) e1.getValue().get(0);
+ InetAddressAndPort addr2 =
(InetAddressAndPort) e2.getValue().get(0);
+
+ return
evaluateComparision(addr1.compareTo(addr2));
+ })
+ .collect(toMap(Map.Entry::getKey,
Map.Entry::getValue, (e1, e2) -> e1, LinkedHashMap::new));
+ }
+ },
+ load(true)
+ {
+ @Override
+ public Map<String, List<Object>> sort(Map<String,
List<Object>> data)
+ {
+ return data.entrySet()
+ .stream()
+ .sorted((e1, e2) -> {
+ String str1 = (String)
e1.getValue().get(3);
+ String str2 = (String)
e2.getValue().get(3);
+ // Check if str1 or str2 contains a '?'
and set a value for it.
+ boolean containsQuestionMark1 =
str1.contains("?");
+ boolean containsQuestionMark2 =
str2.contains("?");
+ if (containsQuestionMark1 &&
containsQuestionMark2) {
+ // If both contain '?', return 0 (they
are considered equal).
+ return 0;
+ }
+
+ boolean descending = descending(sortOrder);
+
+ // If str1 contains '?', ensure it's last
(or first depending on descending).
+ if (containsQuestionMark1)
+ return descending ? 1 : -1;
+
+ // If str2 contains '?', ensure it's last
(or first depending on descending).
+ if (containsQuestionMark2)
+ return descending ? -1 : 1;
+
+ // If neither contain '?', parse the file
sizes and compare.
+ long value1 =
FileUtils.parseFileSize((String) e1.getValue().get(3));
+ long value2 =
FileUtils.parseFileSize((String) e2.getValue().get(3));
+
+ return
evaluateComparision(Long.compare(value1, value2));
+ })
+ .collect(toMap(Map.Entry::getKey,
Map.Entry::getValue, (e1, e2) -> e1, LinkedHashMap::new));
+ }
+ },
+ owns(true)
+ {
+ @Override
+ public Map<String, List<Object>> sort(Map<String,
List<Object>> data)
+ {
+ return data.entrySet()
+ .stream()
+ .sorted((e1, e2) -> {
+ double value1 =
Double.parseDouble(((String) e1.getValue().get(tokenPerNode ? 4 :
5)).replace("%", ""));
+ double value2 =
Double.parseDouble(((String) e2.getValue().get(tokenPerNode ? 4 :
5)).replace("%", ""));
+
+ return
evaluateComparision(Double.compare(value1, value2));
+ })
+ .collect(toMap(Map.Entry::getKey,
Map.Entry::getValue, (e1, e2) -> e1, LinkedHashMap::new));
+ }
+ },
+ id(false)
+ {
+ @Override
+ public Map<String, List<Object>> sort(Map<String,
List<Object>> data)
+ {
+ return compareStrings(data, tokenPerNode ? 5 : 6);
+ }
+ },
+ rack(false)
+ {
+ @Override
+ public Map<String, List<Object>> sort(Map<String,
List<Object>> data)
+ {
+ return compareStrings(data, 7);
+ }
+ },
+ state(true)
+ {
+ @Override
+ public Map<String, List<Object>> sort(Map<String,
List<Object>> data)
+ {
+ return compareStrings(data, 1);
+ }
+ };
+
+ private final boolean descendingByDefault;
+ boolean tokenPerNode;
+ SortOrder sortOrder;
+
+ SortBy()
+ {
+ this(false);
+ }
+
+ SortBy(boolean descendingByDefault)
+ {
+ this.descendingByDefault = descendingByDefault;
+ }
+
+ public abstract Map<String, List<Object>> sort(Map<String,
List<Object>> data);
+
+ boolean descending(SortOrder sortOrder)
+ {
+ return sortOrder == null ? descendingByDefault : sortOrder ==
SortOrder.desc;
+ }
+
+ SortBy sortOrder(SortOrder sortOrder)
Review Comment:
should be marked private or protected
##########
src/java/org/apache/cassandra/tools/nodetool/Status.java:
##########
@@ -24,38 +25,232 @@
import java.io.PrintStream;
import java.net.UnknownHostException;
import java.text.DecimalFormat;
+import java.util.ArrayList;
import java.util.Collection;
+import java.util.HashMap;
import java.util.Iterator;
+import java.util.LinkedHashMap;
import java.util.List;
import java.util.Map;
import java.util.SortedMap;
+import org.apache.cassandra.io.util.FileUtils;
import org.apache.cassandra.locator.EndpointSnitchInfoMBean;
+import org.apache.cassandra.locator.InetAddressAndPort;
import org.apache.cassandra.tools.NodeProbe;
import org.apache.cassandra.tools.NodeTool;
import org.apache.cassandra.tools.NodeTool.NodeToolCmd;
import org.apache.cassandra.tools.nodetool.formatter.TableBuilder;
import com.google.common.collect.ArrayListMultimap;
+import static java.util.stream.Collectors.toMap;
+
@SuppressWarnings("UseOfSystemOutOrSystemErr")
@Command(name = "status", description = "Print cluster information (state,
load, IDs, ...)")
-public class Status extends NodeToolCmd
-{
+public class Status extends NodeToolCmd {
@Arguments(usage = "[<keyspace>]", description = "The keyspace name")
private String keyspace = null;
@Option(title = "resolve_ip", name = {"-r", "--resolve-ip"}, description =
"Show node domain names instead of IPs")
private boolean resolveIp = false;
+ @Option(title = "sort",
+ name = {"-s", "--sort"},
+ description = "Sort by one of 'ip', 'load', 'owns', 'id', 'rack'
or 'state', Defaults to 'none', Default Ordering is 'asc' for 'ip', 'id',
'rack' and 'desc' for 'load', 'owns', 'state' ",
+ allowedValues = {"ip", "load", "owns", "id", "rack", "state",
"none"})
+ private SortBy sortBy = SortBy.none;
+
+ @Option(title = "sort_order",
+ name = {"-o", "--order"},
+ description = "Sort order: 'asc' for ascending, 'desc' for
descending",
+ allowedValues = {"asc", "desc"})
+ private SortOrder sortOrder = null;
+
private boolean isTokenPerNode = true;
private Collection<String> joiningNodes, leavingNodes, movingNodes,
liveNodes, unreachableNodes;
private Map<String, String> loadMap, hostIDMap;
private EndpointSnitchInfoMBean epSnitchInfo;
+ public enum SortOrder {
+ asc,
+ desc
+ }
+
+ public enum SortBy
+ {
+ none
+ {
+ @Override
+ public Map<String, List<Object>> sort(Map<String,
List<Object>> data)
+ {
+ return data;
+ }
+ },
+ ip(false)
+ {
+ @Override
+ public Map<String, List<Object>> sort(Map<String,
List<Object>> data)
+ {
+ return data.entrySet()
+ .stream()
+ .sorted((e1, e2) -> {
+ InetAddressAndPort addr1 =
(InetAddressAndPort) e1.getValue().get(0);
+ InetAddressAndPort addr2 =
(InetAddressAndPort) e2.getValue().get(0);
+
+ return
evaluateComparision(addr1.compareTo(addr2));
+ })
+ .collect(toMap(Map.Entry::getKey,
Map.Entry::getValue, (e1, e2) -> e1, LinkedHashMap::new));
+ }
+ },
+ load(true)
+ {
+ @Override
+ public Map<String, List<Object>> sort(Map<String,
List<Object>> data)
+ {
+ return data.entrySet()
+ .stream()
+ .sorted((e1, e2) -> {
+ String str1 = (String)
e1.getValue().get(3);
+ String str2 = (String)
e2.getValue().get(3);
+ // Check if str1 or str2 contains a '?'
and set a value for it.
+ boolean containsQuestionMark1 =
str1.contains("?");
+ boolean containsQuestionMark2 =
str2.contains("?");
+ if (containsQuestionMark1 &&
containsQuestionMark2) {
+ // If both contain '?', return 0 (they
are considered equal).
+ return 0;
+ }
+
+ boolean descending = descending(sortOrder);
+
+ // If str1 contains '?', ensure it's last
(or first depending on descending).
+ if (containsQuestionMark1)
+ return descending ? 1 : -1;
+
+ // If str2 contains '?', ensure it's last
(or first depending on descending).
+ if (containsQuestionMark2)
+ return descending ? -1 : 1;
+
+ // If neither contain '?', parse the file
sizes and compare.
+ long value1 =
FileUtils.parseFileSize((String) e1.getValue().get(3));
+ long value2 =
FileUtils.parseFileSize((String) e2.getValue().get(3));
+
+ return
evaluateComparision(Long.compare(value1, value2));
+ })
+ .collect(toMap(Map.Entry::getKey,
Map.Entry::getValue, (e1, e2) -> e1, LinkedHashMap::new));
+ }
+ },
+ owns(true)
+ {
+ @Override
+ public Map<String, List<Object>> sort(Map<String,
List<Object>> data)
+ {
+ return data.entrySet()
+ .stream()
+ .sorted((e1, e2) -> {
+ double value1 =
Double.parseDouble(((String) e1.getValue().get(tokenPerNode ? 4 :
5)).replace("%", ""));
+ double value2 =
Double.parseDouble(((String) e2.getValue().get(tokenPerNode ? 4 :
5)).replace("%", ""));
+
+ return
evaluateComparision(Double.compare(value1, value2));
+ })
+ .collect(toMap(Map.Entry::getKey,
Map.Entry::getValue, (e1, e2) -> e1, LinkedHashMap::new));
+ }
+ },
+ id(false)
+ {
+ @Override
+ public Map<String, List<Object>> sort(Map<String,
List<Object>> data)
+ {
+ return compareStrings(data, tokenPerNode ? 5 : 6);
+ }
+ },
+ rack(false)
+ {
+ @Override
+ public Map<String, List<Object>> sort(Map<String,
List<Object>> data)
+ {
+ return compareStrings(data, 7);
+ }
+ },
+ state(true)
+ {
+ @Override
+ public Map<String, List<Object>> sort(Map<String,
List<Object>> data)
+ {
+ return compareStrings(data, 1);
+ }
+ };
+
+ private final boolean descendingByDefault;
+ boolean tokenPerNode;
+ SortOrder sortOrder;
+
+ SortBy()
+ {
+ this(false);
+ }
+
+ SortBy(boolean descendingByDefault)
+ {
+ this.descendingByDefault = descendingByDefault;
+ }
+
+ public abstract Map<String, List<Object>> sort(Map<String,
List<Object>> data);
+
+ boolean descending(SortOrder sortOrder)
+ {
+ return sortOrder == null ? descendingByDefault : sortOrder ==
SortOrder.desc;
+ }
+
+ SortBy sortOrder(SortOrder sortOrder)
+ {
+ this.sortOrder = sortOrder;
+ return this;
+ }
+
+ SortBy tokenPerNode(boolean tokenPerNode)
+ {
+ this.tokenPerNode = tokenPerNode;
+ return this;
+ }
+
+ int evaluateComparision(int comparisionResult)
Review Comment:
should be marked protected
##########
src/java/org/apache/cassandra/tools/nodetool/Status.java:
##########
@@ -24,38 +25,232 @@
import java.io.PrintStream;
import java.net.UnknownHostException;
import java.text.DecimalFormat;
+import java.util.ArrayList;
import java.util.Collection;
+import java.util.HashMap;
import java.util.Iterator;
+import java.util.LinkedHashMap;
import java.util.List;
import java.util.Map;
import java.util.SortedMap;
+import org.apache.cassandra.io.util.FileUtils;
import org.apache.cassandra.locator.EndpointSnitchInfoMBean;
+import org.apache.cassandra.locator.InetAddressAndPort;
import org.apache.cassandra.tools.NodeProbe;
import org.apache.cassandra.tools.NodeTool;
import org.apache.cassandra.tools.NodeTool.NodeToolCmd;
import org.apache.cassandra.tools.nodetool.formatter.TableBuilder;
import com.google.common.collect.ArrayListMultimap;
+import static java.util.stream.Collectors.toMap;
+
@SuppressWarnings("UseOfSystemOutOrSystemErr")
@Command(name = "status", description = "Print cluster information (state,
load, IDs, ...)")
-public class Status extends NodeToolCmd
-{
+public class Status extends NodeToolCmd {
@Arguments(usage = "[<keyspace>]", description = "The keyspace name")
private String keyspace = null;
@Option(title = "resolve_ip", name = {"-r", "--resolve-ip"}, description =
"Show node domain names instead of IPs")
private boolean resolveIp = false;
+ @Option(title = "sort",
+ name = {"-s", "--sort"},
+ description = "Sort by one of 'ip', 'load', 'owns', 'id', 'rack'
or 'state', Defaults to 'none', Default Ordering is 'asc' for 'ip', 'id',
'rack' and 'desc' for 'load', 'owns', 'state' ",
+ allowedValues = {"ip", "load", "owns", "id", "rack", "state",
"none"})
+ private SortBy sortBy = SortBy.none;
+
+ @Option(title = "sort_order",
+ name = {"-o", "--order"},
+ description = "Sort order: 'asc' for ascending, 'desc' for
descending",
+ allowedValues = {"asc", "desc"})
+ private SortOrder sortOrder = null;
+
private boolean isTokenPerNode = true;
private Collection<String> joiningNodes, leavingNodes, movingNodes,
liveNodes, unreachableNodes;
private Map<String, String> loadMap, hostIDMap;
private EndpointSnitchInfoMBean epSnitchInfo;
+ public enum SortOrder {
+ asc,
+ desc
+ }
+
+ public enum SortBy
+ {
+ none
+ {
+ @Override
+ public Map<String, List<Object>> sort(Map<String,
List<Object>> data)
+ {
+ return data;
+ }
+ },
+ ip(false)
+ {
+ @Override
+ public Map<String, List<Object>> sort(Map<String,
List<Object>> data)
+ {
+ return data.entrySet()
+ .stream()
+ .sorted((e1, e2) -> {
+ InetAddressAndPort addr1 =
(InetAddressAndPort) e1.getValue().get(0);
+ InetAddressAndPort addr2 =
(InetAddressAndPort) e2.getValue().get(0);
+
+ return
evaluateComparision(addr1.compareTo(addr2));
+ })
+ .collect(toMap(Map.Entry::getKey,
Map.Entry::getValue, (e1, e2) -> e1, LinkedHashMap::new));
+ }
+ },
+ load(true)
+ {
+ @Override
+ public Map<String, List<Object>> sort(Map<String,
List<Object>> data)
+ {
+ return data.entrySet()
+ .stream()
+ .sorted((e1, e2) -> {
+ String str1 = (String)
e1.getValue().get(3);
+ String str2 = (String)
e2.getValue().get(3);
+ // Check if str1 or str2 contains a '?'
and set a value for it.
+ boolean containsQuestionMark1 =
str1.contains("?");
+ boolean containsQuestionMark2 =
str2.contains("?");
+ if (containsQuestionMark1 &&
containsQuestionMark2) {
+ // If both contain '?', return 0 (they
are considered equal).
+ return 0;
+ }
+
+ boolean descending = descending(sortOrder);
+
+ // If str1 contains '?', ensure it's last
(or first depending on descending).
+ if (containsQuestionMark1)
+ return descending ? 1 : -1;
+
+ // If str2 contains '?', ensure it's last
(or first depending on descending).
+ if (containsQuestionMark2)
+ return descending ? -1 : 1;
+
+ // If neither contain '?', parse the file
sizes and compare.
+ long value1 =
FileUtils.parseFileSize((String) e1.getValue().get(3));
+ long value2 =
FileUtils.parseFileSize((String) e2.getValue().get(3));
+
+ return
evaluateComparision(Long.compare(value1, value2));
+ })
+ .collect(toMap(Map.Entry::getKey,
Map.Entry::getValue, (e1, e2) -> e1, LinkedHashMap::new));
+ }
+ },
+ owns(true)
+ {
+ @Override
+ public Map<String, List<Object>> sort(Map<String,
List<Object>> data)
+ {
+ return data.entrySet()
+ .stream()
+ .sorted((e1, e2) -> {
+ double value1 =
Double.parseDouble(((String) e1.getValue().get(tokenPerNode ? 4 :
5)).replace("%", ""));
+ double value2 =
Double.parseDouble(((String) e2.getValue().get(tokenPerNode ? 4 :
5)).replace("%", ""));
+
+ return
evaluateComparision(Double.compare(value1, value2));
+ })
+ .collect(toMap(Map.Entry::getKey,
Map.Entry::getValue, (e1, e2) -> e1, LinkedHashMap::new));
+ }
+ },
+ id(false)
+ {
+ @Override
+ public Map<String, List<Object>> sort(Map<String,
List<Object>> data)
+ {
+ return compareStrings(data, tokenPerNode ? 5 : 6);
+ }
+ },
+ rack(false)
+ {
+ @Override
+ public Map<String, List<Object>> sort(Map<String,
List<Object>> data)
+ {
+ return compareStrings(data, 7);
+ }
+ },
+ state(true)
+ {
+ @Override
+ public Map<String, List<Object>> sort(Map<String,
List<Object>> data)
+ {
+ return compareStrings(data, 1);
+ }
+ };
+
+ private final boolean descendingByDefault;
+ boolean tokenPerNode;
+ SortOrder sortOrder;
+
+ SortBy()
+ {
+ this(false);
+ }
+
+ SortBy(boolean descendingByDefault)
+ {
+ this.descendingByDefault = descendingByDefault;
+ }
+
+ public abstract Map<String, List<Object>> sort(Map<String,
List<Object>> data);
+
+ boolean descending(SortOrder sortOrder)
+ {
+ return sortOrder == null ? descendingByDefault : sortOrder ==
SortOrder.desc;
+ }
+
+ SortBy sortOrder(SortOrder sortOrder)
+ {
+ this.sortOrder = sortOrder;
+ return this;
+ }
+
+ SortBy tokenPerNode(boolean tokenPerNode)
+ {
+ this.tokenPerNode = tokenPerNode;
+ return this;
+ }
+
+ int evaluateComparision(int comparisionResult)
+ {
+ if (comparisionResult < 0)
+ return descending(sortOrder) ? 1 : -1;
+ if (comparisionResult > 0)
+ return descending(sortOrder) ? -1 : 1;
+
+ return 0;
+ }
+
+ LinkedHashMap<String, List<Object>> compareStrings(Map<String,
List<Object>> data, int index)
+ {
+ return data.entrySet()
+ .stream()
+ .sorted((e1, e2) -> {
+ String str1 = (String) e1.getValue().get(index);
+ String str2 = (String) e2.getValue().get(index);
+ return evaluateComparision(str1.compareTo(str2));
+ })
+ .collect(toMap(Map.Entry::getKey, Map.Entry::getValue,
(e1, e2) -> e1, LinkedHashMap::new));
+ }
+ }
+
+ private void validateArguments(PrintStream out)
Review Comment:
minor nit: I find this cleaner after `execute` since thats the function
folks look for first when they read the code
##########
src/java/org/apache/cassandra/tools/nodetool/Status.java:
##########
@@ -24,38 +25,232 @@
import java.io.PrintStream;
import java.net.UnknownHostException;
import java.text.DecimalFormat;
+import java.util.ArrayList;
import java.util.Collection;
+import java.util.HashMap;
import java.util.Iterator;
+import java.util.LinkedHashMap;
import java.util.List;
import java.util.Map;
import java.util.SortedMap;
+import org.apache.cassandra.io.util.FileUtils;
import org.apache.cassandra.locator.EndpointSnitchInfoMBean;
+import org.apache.cassandra.locator.InetAddressAndPort;
import org.apache.cassandra.tools.NodeProbe;
import org.apache.cassandra.tools.NodeTool;
import org.apache.cassandra.tools.NodeTool.NodeToolCmd;
import org.apache.cassandra.tools.nodetool.formatter.TableBuilder;
import com.google.common.collect.ArrayListMultimap;
+import static java.util.stream.Collectors.toMap;
+
@SuppressWarnings("UseOfSystemOutOrSystemErr")
@Command(name = "status", description = "Print cluster information (state,
load, IDs, ...)")
-public class Status extends NodeToolCmd
-{
+public class Status extends NodeToolCmd {
@Arguments(usage = "[<keyspace>]", description = "The keyspace name")
private String keyspace = null;
@Option(title = "resolve_ip", name = {"-r", "--resolve-ip"}, description =
"Show node domain names instead of IPs")
private boolean resolveIp = false;
+ @Option(title = "sort",
+ name = {"-s", "--sort"},
+ description = "Sort by one of 'ip', 'load', 'owns', 'id', 'rack'
or 'state', Defaults to 'none', Default Ordering is 'asc' for 'ip', 'id',
'rack' and 'desc' for 'load', 'owns', 'state' ",
+ allowedValues = {"ip", "load", "owns", "id", "rack", "state",
"none"})
+ private SortBy sortBy = SortBy.none;
+
+ @Option(title = "sort_order",
+ name = {"-o", "--order"},
+ description = "Sort order: 'asc' for ascending, 'desc' for
descending",
+ allowedValues = {"asc", "desc"})
+ private SortOrder sortOrder = null;
+
private boolean isTokenPerNode = true;
private Collection<String> joiningNodes, leavingNodes, movingNodes,
liveNodes, unreachableNodes;
private Map<String, String> loadMap, hostIDMap;
private EndpointSnitchInfoMBean epSnitchInfo;
+ public enum SortOrder {
+ asc,
+ desc
+ }
+
+ public enum SortBy
+ {
+ none
+ {
+ @Override
+ public Map<String, List<Object>> sort(Map<String,
List<Object>> data)
+ {
+ return data;
+ }
+ },
+ ip(false)
+ {
+ @Override
+ public Map<String, List<Object>> sort(Map<String,
List<Object>> data)
+ {
+ return data.entrySet()
+ .stream()
+ .sorted((e1, e2) -> {
+ InetAddressAndPort addr1 =
(InetAddressAndPort) e1.getValue().get(0);
+ InetAddressAndPort addr2 =
(InetAddressAndPort) e2.getValue().get(0);
+
+ return
evaluateComparision(addr1.compareTo(addr2));
+ })
+ .collect(toMap(Map.Entry::getKey,
Map.Entry::getValue, (e1, e2) -> e1, LinkedHashMap::new));
+ }
+ },
+ load(true)
+ {
+ @Override
+ public Map<String, List<Object>> sort(Map<String,
List<Object>> data)
+ {
+ return data.entrySet()
+ .stream()
+ .sorted((e1, e2) -> {
+ String str1 = (String)
e1.getValue().get(3);
+ String str2 = (String)
e2.getValue().get(3);
+ // Check if str1 or str2 contains a '?'
and set a value for it.
+ boolean containsQuestionMark1 =
str1.contains("?");
+ boolean containsQuestionMark2 =
str2.contains("?");
+ if (containsQuestionMark1 &&
containsQuestionMark2) {
+ // If both contain '?', return 0 (they
are considered equal).
+ return 0;
+ }
+
+ boolean descending = descending(sortOrder);
+
+ // If str1 contains '?', ensure it's last
(or first depending on descending).
+ if (containsQuestionMark1)
+ return descending ? 1 : -1;
+
+ // If str2 contains '?', ensure it's last
(or first depending on descending).
+ if (containsQuestionMark2)
+ return descending ? -1 : 1;
+
+ // If neither contain '?', parse the file
sizes and compare.
+ long value1 =
FileUtils.parseFileSize((String) e1.getValue().get(3));
+ long value2 =
FileUtils.parseFileSize((String) e2.getValue().get(3));
+
+ return
evaluateComparision(Long.compare(value1, value2));
+ })
+ .collect(toMap(Map.Entry::getKey,
Map.Entry::getValue, (e1, e2) -> e1, LinkedHashMap::new));
+ }
+ },
+ owns(true)
+ {
+ @Override
+ public Map<String, List<Object>> sort(Map<String,
List<Object>> data)
+ {
+ return data.entrySet()
+ .stream()
+ .sorted((e1, e2) -> {
+ double value1 =
Double.parseDouble(((String) e1.getValue().get(tokenPerNode ? 4 :
5)).replace("%", ""));
+ double value2 =
Double.parseDouble(((String) e2.getValue().get(tokenPerNode ? 4 :
5)).replace("%", ""));
+
+ return
evaluateComparision(Double.compare(value1, value2));
+ })
+ .collect(toMap(Map.Entry::getKey,
Map.Entry::getValue, (e1, e2) -> e1, LinkedHashMap::new));
+ }
+ },
+ id(false)
+ {
+ @Override
+ public Map<String, List<Object>> sort(Map<String,
List<Object>> data)
+ {
+ return compareStrings(data, tokenPerNode ? 5 : 6);
+ }
+ },
+ rack(false)
+ {
+ @Override
+ public Map<String, List<Object>> sort(Map<String,
List<Object>> data)
+ {
+ return compareStrings(data, 7);
+ }
+ },
+ state(true)
+ {
+ @Override
+ public Map<String, List<Object>> sort(Map<String,
List<Object>> data)
+ {
+ return compareStrings(data, 1);
+ }
+ };
+
+ private final boolean descendingByDefault;
+ boolean tokenPerNode;
+ SortOrder sortOrder;
+
+ SortBy()
+ {
+ this(false);
+ }
+
+ SortBy(boolean descendingByDefault)
+ {
+ this.descendingByDefault = descendingByDefault;
+ }
+
+ public abstract Map<String, List<Object>> sort(Map<String,
List<Object>> data);
+
+ boolean descending(SortOrder sortOrder)
+ {
+ return sortOrder == null ? descendingByDefault : sortOrder ==
SortOrder.desc;
+ }
+
+ SortBy sortOrder(SortOrder sortOrder)
+ {
+ this.sortOrder = sortOrder;
+ return this;
+ }
+
+ SortBy tokenPerNode(boolean tokenPerNode)
+ {
+ this.tokenPerNode = tokenPerNode;
+ return this;
+ }
+
+ int evaluateComparision(int comparisionResult)
+ {
+ if (comparisionResult < 0)
+ return descending(sortOrder) ? 1 : -1;
+ if (comparisionResult > 0)
+ return descending(sortOrder) ? -1 : 1;
+
+ return 0;
+ }
+
+ LinkedHashMap<String, List<Object>> compareStrings(Map<String,
List<Object>> data, int index)
Review Comment:
should be marked protected
--
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]