[GitHub] drill pull request #482: DRILL-4604: Generate warning on Web UI if drillbits...
Github user sudheeshkatkam commented on a diff in the pull request:
https://github.com/apache/drill/pull/482#discussion_r70672079
--- Diff:
exec/java-exec/src/main/java/org/apache/drill/exec/server/Drillbit.java ---
@@ -207,6 +210,21 @@ private void javaPropertiesToSystemOptions() {
}
/**
+ * If cluster version is the same as default, update option to current
drillbit version.
+ */
+ private void checkAndUpdateClusterVersionOption(OptionManager
optionManager, DrillbitEndpoint drillbitEndpoint) {
+OptionValue versionOption =
optionManager.getOption(ExecConstants.CLUSTER_VERSION);
+OptionValidator validator =
SystemOptionManager.getValidator(ExecConstants.CLUSTER_VERSION);
+if (versionOption.equals(validator.getDefault())) {
+ optionManager.setOption(OptionValue.createOption(
--- End diff --
I don't think using a system option is the right way to detect version
conflicts; cluster wide upgrade and restart will cause all drillbits to
generate a warning because system options are persisted.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] drill pull request #482: DRILL-4604: Generate warning on Web UI if drillbits...
Github user arina-ielchiieva commented on a diff in the pull request:
https://github.com/apache/drill/pull/482#discussion_r70770934
--- Diff:
exec/java-exec/src/main/java/org/apache/drill/exec/server/Drillbit.java ---
@@ -207,6 +210,21 @@ private void javaPropertiesToSystemOptions() {
}
/**
+ * If cluster version is the same as default, update option to current
drillbit version.
+ */
+ private void checkAndUpdateClusterVersionOption(OptionManager
optionManager, DrillbitEndpoint drillbitEndpoint) {
+OptionValue versionOption =
optionManager.getOption(ExecConstants.CLUSTER_VERSION);
+OptionValidator validator =
SystemOptionManager.getValidator(ExecConstants.CLUSTER_VERSION);
+if (versionOption.equals(validator.getDefault())) {
+ optionManager.setOption(OptionValue.createOption(
--- End diff --
@sudheeshkatkam
this was done on purpose [1]. Version is set as system option so
administrator can update it when cluster upgrade was done. This will help
administrator to detect drillbits that were not updated.
[1]
https://issues.apache.org/jira/browse/DRILL-4604?focusedCommentId=15241083&page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel#comment-15241083
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] drill pull request #482: DRILL-4604: Generate warning on Web UI if drillbits...
Github user sudheeshkatkam commented on a diff in the pull request:
https://github.com/apache/drill/pull/482#discussion_r84972014
--- Diff:
exec/java-exec/src/main/java/org/apache/drill/exec/server/rest/DrillRoot.java
---
@@ -55,44 +59,89 @@ public Viewable getStats() {
@GET
@Path("/stats.json")
@Produces(MediaType.APPLICATION_JSON)
- public List getStatsJSON() {
-List stats = Lists.newLinkedList();
-stats.add(new Stat("Number of Drill Bits",
work.getContext().getBits().size()));
-int number = 0;
-for (CoordinationProtos.DrillbitEndpoint bit :
work.getContext().getBits()) {
- String initialized = bit.isInitialized() ? " initialized" : " not
initialized";
- stats.add(new Stat("Bit #" + number, bit.getAddress() +
initialized));
- ++number;
+ public Stats getStatsJSON() {
+final String version =
work.getContext().getOptionManager().getOption(ExecConstants.CLUSTER_VERSION).string_val;
--- End diff --
`final String version =
work.getContext().getOptionManager().getOption(ExecConstants.CLUSTER_VERSION_VALIDATOR);`
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] drill pull request #482: DRILL-4604: Generate warning on Web UI if drillbits...
Github user sudheeshkatkam commented on a diff in the pull request:
https://github.com/apache/drill/pull/482#discussion_r84975543
--- Diff:
exec/java-exec/src/main/java/org/apache/drill/exec/server/rest/DrillRoot.java
---
@@ -55,44 +59,89 @@ public Viewable getStats() {
@GET
@Path("/stats.json")
@Produces(MediaType.APPLICATION_JSON)
- public List getStatsJSON() {
-List stats = Lists.newLinkedList();
-stats.add(new Stat("Number of Drill Bits",
work.getContext().getBits().size()));
-int number = 0;
-for (CoordinationProtos.DrillbitEndpoint bit :
work.getContext().getBits()) {
- String initialized = bit.isInitialized() ? " initialized" : " not
initialized";
- stats.add(new Stat("Bit #" + number, bit.getAddress() +
initialized));
- ++number;
+ public Stats getStatsJSON() {
+final String version =
work.getContext().getOptionManager().getOption(ExecConstants.CLUSTER_VERSION).string_val;
+
+final Map props = Maps.newLinkedHashMap();
+props.put("Cluster Version", version);
+props.put("Number of Drillbits", work.getContext().getBits().size());
+CoordinationProtos.DrillbitEndpoint currentEndpoint =
work.getContext().getEndpoint();
+final String address = currentEndpoint.getAddress();
+props.put("Data Port Address", address + ":" +
currentEndpoint.getDataPort());
+props.put("User Port Address", address + ":" +
currentEndpoint.getUserPort());
+props.put("Control Port Address", address + ":" +
currentEndpoint.getControlPort());
+props.put("Maximum Direct Memory", DrillConfig.getMaxDirectMemory());
+
+return new Stats(props, collectDrillbits(version));
+ }
+
+ private Collection collectDrillbits(String version) {
+Set drillbits = Sets.newTreeSet();
+for (CoordinationProtos.DrillbitEndpoint endpoint :
work.getContext().getBits()) {
+ boolean versionMatch = version.equals(endpoint.getVersion());
+ DrillbitInfo drillbit = new DrillbitInfo(endpoint.getAddress(),
endpoint.isInitialized(), endpoint.getVersion(), versionMatch);
+ drillbits.add(drillbit);
}
-stats.add(new Stat("Data Port Address",
work.getContext().getEndpoint().getAddress() +
- ":" + work.getContext().getEndpoint().getDataPort()));
-stats.add(new Stat("User Port Address",
work.getContext().getEndpoint().getAddress() +
- ":" + work.getContext().getEndpoint().getUserPort()));
-stats.add(new Stat("Control Port Address",
work.getContext().getEndpoint().getAddress() +
- ":" + work.getContext().getEndpoint().getControlPort()));
-stats.add(new Stat("Maximum Direct Memory",
DrillConfig.getMaxDirectMemory()));
-
-return stats;
+return drillbits;
}
@XmlRootElement
- public class Stat {
-private String name;
-private Object value;
+ public static class Stats {
--- End diff --
More descriptive name?
(my bad for naming this `Stat` in the first place)
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] drill pull request #482: DRILL-4604: Generate warning on Web UI if drillbits...
Github user sudheeshkatkam commented on a diff in the pull request:
https://github.com/apache/drill/pull/482#discussion_r84976814
--- Diff:
exec/java-exec/src/main/java/org/apache/drill/exec/server/Drillbit.java ---
@@ -207,6 +210,21 @@ private void javaPropertiesToSystemOptions() {
}
/**
+ * If cluster version is the same as default, update option to current
drillbit version.
+ */
+ private void checkAndUpdateClusterVersionOption(OptionManager
optionManager, DrillbitEndpoint drillbitEndpoint) {
+OptionValue versionOption =
optionManager.getOption(ExecConstants.CLUSTER_VERSION);
+OptionValidator validator =
SystemOptionManager.getValidator(ExecConstants.CLUSTER_VERSION);
+if (versionOption.equals(validator.getDefault())) {
+ optionManager.setOption(OptionValue.createOption(
--- End diff --
Add a comment in code about what happens when multiple drillbits try to set
the cluster version (since `getOption` and `setOption` are separate calls).
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] drill pull request #482: DRILL-4604: Generate warning on Web UI if drillbits...
Github user sudheeshkatkam commented on a diff in the pull request:
https://github.com/apache/drill/pull/482#discussion_r84973313
--- Diff:
exec/java-exec/src/main/java/org/apache/drill/exec/server/rest/DrillRoot.java
---
@@ -55,44 +59,89 @@ public Viewable getStats() {
@GET
@Path("/stats.json")
@Produces(MediaType.APPLICATION_JSON)
- public List getStatsJSON() {
-List stats = Lists.newLinkedList();
-stats.add(new Stat("Number of Drill Bits",
work.getContext().getBits().size()));
-int number = 0;
-for (CoordinationProtos.DrillbitEndpoint bit :
work.getContext().getBits()) {
- String initialized = bit.isInitialized() ? " initialized" : " not
initialized";
- stats.add(new Stat("Bit #" + number, bit.getAddress() +
initialized));
- ++number;
+ public Stats getStatsJSON() {
+final String version =
work.getContext().getOptionManager().getOption(ExecConstants.CLUSTER_VERSION).string_val;
+
+final Map props = Maps.newLinkedHashMap();
+props.put("Cluster Version", version);
+props.put("Number of Drillbits", work.getContext().getBits().size());
+CoordinationProtos.DrillbitEndpoint currentEndpoint =
work.getContext().getEndpoint();
+final String address = currentEndpoint.getAddress();
+props.put("Data Port Address", address + ":" +
currentEndpoint.getDataPort());
+props.put("User Port Address", address + ":" +
currentEndpoint.getUserPort());
+props.put("Control Port Address", address + ":" +
currentEndpoint.getControlPort());
+props.put("Maximum Direct Memory", DrillConfig.getMaxDirectMemory());
+
+return new Stats(props, collectDrillbits(version));
+ }
+
+ private Collection collectDrillbits(String version) {
+Set drillbits = Sets.newTreeSet();
+for (CoordinationProtos.DrillbitEndpoint endpoint :
work.getContext().getBits()) {
+ boolean versionMatch = version.equals(endpoint.getVersion());
+ DrillbitInfo drillbit = new DrillbitInfo(endpoint.getAddress(),
endpoint.isInitialized(), endpoint.getVersion(), versionMatch);
--- End diff --
Remove misleading `endpoint.isInitialized()`. Looks like that is a check if
protobuf object is initialized, and not the drillbit.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] drill pull request #482: DRILL-4604: Generate warning on Web UI if drillbits...
Github user sudheeshkatkam commented on a diff in the pull request:
https://github.com/apache/drill/pull/482#discussion_r84973496
--- Diff:
exec/java-exec/src/main/java/org/apache/drill/exec/server/rest/DrillRoot.java
---
@@ -55,44 +59,89 @@ public Viewable getStats() {
@GET
@Path("/stats.json")
@Produces(MediaType.APPLICATION_JSON)
- public List getStatsJSON() {
-List stats = Lists.newLinkedList();
-stats.add(new Stat("Number of Drill Bits",
work.getContext().getBits().size()));
-int number = 0;
-for (CoordinationProtos.DrillbitEndpoint bit :
work.getContext().getBits()) {
- String initialized = bit.isInitialized() ? " initialized" : " not
initialized";
- stats.add(new Stat("Bit #" + number, bit.getAddress() +
initialized));
- ++number;
+ public Stats getStatsJSON() {
+final String version =
work.getContext().getOptionManager().getOption(ExecConstants.CLUSTER_VERSION).string_val;
--- End diff --
Rename `version` to `clusterVersion`
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] drill pull request #482: DRILL-4604: Generate warning on Web UI if drillbits...
Github user sudheeshkatkam commented on a diff in the pull request:
https://github.com/apache/drill/pull/482#discussion_r84970266
--- Diff:
exec/java-exec/src/main/java/org/apache/drill/exec/server/Drillbit.java ---
@@ -207,6 +210,21 @@ private void javaPropertiesToSystemOptions() {
}
/**
+ * If cluster version is the same as default, update option to current
drillbit version.
+ */
+ private void checkAndUpdateClusterVersionOption(OptionManager
optionManager, DrillbitEndpoint drillbitEndpoint) {
--- End diff --
Doc: rename `optionManager` to `systemOptions`
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] drill pull request #482: DRILL-4604: Generate warning on Web UI if drillbits...
Github user sudheeshkatkam commented on a diff in the pull request: https://github.com/apache/drill/pull/482#discussion_r84969332 --- Diff: exec/java-exec/src/main/java/org/apache/drill/exec/ExecConstants.java --- @@ -17,6 +17,7 @@ */ package org.apache.drill.exec; +import org.apache.drill.common.util.DrillVersionInfo; --- End diff -- unused --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] drill pull request #482: DRILL-4604: Generate warning on Web UI if drillbits...
Github user sudheeshkatkam commented on a diff in the pull request:
https://github.com/apache/drill/pull/482#discussion_r84975052
--- Diff:
exec/java-exec/src/main/java/org/apache/drill/exec/server/rest/DrillRoot.java
---
@@ -55,44 +59,89 @@ public Viewable getStats() {
@GET
@Path("/stats.json")
@Produces(MediaType.APPLICATION_JSON)
- public List getStatsJSON() {
-List stats = Lists.newLinkedList();
-stats.add(new Stat("Number of Drill Bits",
work.getContext().getBits().size()));
-int number = 0;
-for (CoordinationProtos.DrillbitEndpoint bit :
work.getContext().getBits()) {
- String initialized = bit.isInitialized() ? " initialized" : " not
initialized";
- stats.add(new Stat("Bit #" + number, bit.getAddress() +
initialized));
- ++number;
+ public Stats getStatsJSON() {
+final String version =
work.getContext().getOptionManager().getOption(ExecConstants.CLUSTER_VERSION).string_val;
+
+final Map props = Maps.newLinkedHashMap();
+props.put("Cluster Version", version);
+props.put("Number of Drillbits", work.getContext().getBits().size());
+CoordinationProtos.DrillbitEndpoint currentEndpoint =
work.getContext().getEndpoint();
+final String address = currentEndpoint.getAddress();
+props.put("Data Port Address", address + ":" +
currentEndpoint.getDataPort());
+props.put("User Port Address", address + ":" +
currentEndpoint.getUserPort());
+props.put("Control Port Address", address + ":" +
currentEndpoint.getControlPort());
+props.put("Maximum Direct Memory", DrillConfig.getMaxDirectMemory());
+
+return new Stats(props, collectDrillbits(version));
+ }
+
+ private Collection collectDrillbits(String version) {
+Set drillbits = Sets.newTreeSet();
+for (CoordinationProtos.DrillbitEndpoint endpoint :
work.getContext().getBits()) {
+ boolean versionMatch = version.equals(endpoint.getVersion());
+ DrillbitInfo drillbit = new DrillbitInfo(endpoint.getAddress(),
endpoint.isInitialized(), endpoint.getVersion(), versionMatch);
--- End diff --
Older versions will return `null` for `endpoint.getVersion()`; how does the
patch handle these cases?
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] drill pull request #482: DRILL-4604: Generate warning on Web UI if drillbits...
Github user arina-ielchiieva commented on a diff in the pull request:
https://github.com/apache/drill/pull/482#discussion_r85978846
--- Diff:
exec/java-exec/src/main/java/org/apache/drill/exec/server/Drillbit.java ---
@@ -207,6 +210,21 @@ private void javaPropertiesToSystemOptions() {
}
/**
+ * If cluster version is the same as default, update option to current
drillbit version.
+ */
+ private void checkAndUpdateClusterVersionOption(OptionManager
optionManager, DrillbitEndpoint drillbitEndpoint) {
+OptionValue versionOption =
optionManager.getOption(ExecConstants.CLUSTER_VERSION);
+OptionValidator validator =
SystemOptionManager.getValidator(ExecConstants.CLUSTER_VERSION);
+if (versionOption.equals(validator.getDefault())) {
+ optionManager.setOption(OptionValue.createOption(
--- End diff --
Done.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] drill pull request #482: DRILL-4604: Generate warning on Web UI if drillbits...
Github user arina-ielchiieva commented on a diff in the pull request: https://github.com/apache/drill/pull/482#discussion_r85974147 --- Diff: exec/java-exec/src/main/java/org/apache/drill/exec/ExecConstants.java --- @@ -17,6 +17,7 @@ */ package org.apache.drill.exec; +import org.apache.drill.common.util.DrillVersionInfo; --- End diff -- Done. --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] drill pull request #482: DRILL-4604: Generate warning on Web UI if drillbits...
Github user arina-ielchiieva commented on a diff in the pull request:
https://github.com/apache/drill/pull/482#discussion_r85974663
--- Diff:
exec/java-exec/src/main/java/org/apache/drill/exec/server/Drillbit.java ---
@@ -207,6 +210,21 @@ private void javaPropertiesToSystemOptions() {
}
/**
+ * If cluster version is the same as default, update option to current
drillbit version.
+ */
+ private void checkAndUpdateClusterVersionOption(OptionManager
optionManager, DrillbitEndpoint drillbitEndpoint) {
--- End diff --
Done.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] drill pull request #482: DRILL-4604: Generate warning on Web UI if drillbits...
Github user arina-ielchiieva commented on a diff in the pull request:
https://github.com/apache/drill/pull/482#discussion_r85975036
--- Diff:
exec/java-exec/src/main/java/org/apache/drill/exec/server/rest/DrillRoot.java
---
@@ -55,44 +59,89 @@ public Viewable getStats() {
@GET
@Path("/stats.json")
@Produces(MediaType.APPLICATION_JSON)
- public List getStatsJSON() {
-List stats = Lists.newLinkedList();
-stats.add(new Stat("Number of Drill Bits",
work.getContext().getBits().size()));
-int number = 0;
-for (CoordinationProtos.DrillbitEndpoint bit :
work.getContext().getBits()) {
- String initialized = bit.isInitialized() ? " initialized" : " not
initialized";
- stats.add(new Stat("Bit #" + number, bit.getAddress() +
initialized));
- ++number;
+ public Stats getStatsJSON() {
+final String version =
work.getContext().getOptionManager().getOption(ExecConstants.CLUSTER_VERSION).string_val;
--- End diff --
Done.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] drill pull request #482: DRILL-4604: Generate warning on Web UI if drillbits...
Github user arina-ielchiieva commented on a diff in the pull request:
https://github.com/apache/drill/pull/482#discussion_r86107203
--- Diff:
exec/java-exec/src/main/java/org/apache/drill/exec/server/rest/DrillRoot.java
---
@@ -55,44 +59,89 @@ public Viewable getStats() {
@GET
@Path("/stats.json")
@Produces(MediaType.APPLICATION_JSON)
- public List getStatsJSON() {
-List stats = Lists.newLinkedList();
-stats.add(new Stat("Number of Drill Bits",
work.getContext().getBits().size()));
-int number = 0;
-for (CoordinationProtos.DrillbitEndpoint bit :
work.getContext().getBits()) {
- String initialized = bit.isInitialized() ? " initialized" : " not
initialized";
- stats.add(new Stat("Bit #" + number, bit.getAddress() +
initialized));
- ++number;
+ public Stats getStatsJSON() {
+final String version =
work.getContext().getOptionManager().getOption(ExecConstants.CLUSTER_VERSION).string_val;
+
+final Map props = Maps.newLinkedHashMap();
+props.put("Cluster Version", version);
+props.put("Number of Drillbits", work.getContext().getBits().size());
+CoordinationProtos.DrillbitEndpoint currentEndpoint =
work.getContext().getEndpoint();
+final String address = currentEndpoint.getAddress();
+props.put("Data Port Address", address + ":" +
currentEndpoint.getDataPort());
+props.put("User Port Address", address + ":" +
currentEndpoint.getUserPort());
+props.put("Control Port Address", address + ":" +
currentEndpoint.getControlPort());
+props.put("Maximum Direct Memory", DrillConfig.getMaxDirectMemory());
+
+return new Stats(props, collectDrillbits(version));
+ }
+
+ private Collection collectDrillbits(String version) {
+Set drillbits = Sets.newTreeSet();
+for (CoordinationProtos.DrillbitEndpoint endpoint :
work.getContext().getBits()) {
+ boolean versionMatch = version.equals(endpoint.getVersion());
+ DrillbitInfo drillbit = new DrillbitInfo(endpoint.getAddress(),
endpoint.isInitialized(), endpoint.getVersion(), versionMatch);
+ drillbits.add(drillbit);
}
-stats.add(new Stat("Data Port Address",
work.getContext().getEndpoint().getAddress() +
- ":" + work.getContext().getEndpoint().getDataPort()));
-stats.add(new Stat("User Port Address",
work.getContext().getEndpoint().getAddress() +
- ":" + work.getContext().getEndpoint().getUserPort()));
-stats.add(new Stat("Control Port Address",
work.getContext().getEndpoint().getAddress() +
- ":" + work.getContext().getEndpoint().getControlPort()));
-stats.add(new Stat("Maximum Direct Memory",
DrillConfig.getMaxDirectMemory()));
-
-return stats;
+return drillbits;
}
@XmlRootElement
- public class Stat {
-private String name;
-private Object value;
+ public static class Stats {
--- End diff --
Renamed to ClusterInfo
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] drill pull request #482: DRILL-4604: Generate warning on Web UI if drillbits...
Github user arina-ielchiieva commented on a diff in the pull request:
https://github.com/apache/drill/pull/482#discussion_r85954983
--- Diff:
exec/java-exec/src/main/java/org/apache/drill/exec/server/rest/DrillRoot.java
---
@@ -55,44 +59,89 @@ public Viewable getStats() {
@GET
@Path("/stats.json")
@Produces(MediaType.APPLICATION_JSON)
- public List getStatsJSON() {
-List stats = Lists.newLinkedList();
-stats.add(new Stat("Number of Drill Bits",
work.getContext().getBits().size()));
-int number = 0;
-for (CoordinationProtos.DrillbitEndpoint bit :
work.getContext().getBits()) {
- String initialized = bit.isInitialized() ? " initialized" : " not
initialized";
- stats.add(new Stat("Bit #" + number, bit.getAddress() +
initialized));
- ++number;
+ public Stats getStatsJSON() {
+final String version =
work.getContext().getOptionManager().getOption(ExecConstants.CLUSTER_VERSION).string_val;
+
+final Map props = Maps.newLinkedHashMap();
+props.put("Cluster Version", version);
+props.put("Number of Drillbits", work.getContext().getBits().size());
+CoordinationProtos.DrillbitEndpoint currentEndpoint =
work.getContext().getEndpoint();
+final String address = currentEndpoint.getAddress();
+props.put("Data Port Address", address + ":" +
currentEndpoint.getDataPort());
+props.put("User Port Address", address + ":" +
currentEndpoint.getUserPort());
+props.put("Control Port Address", address + ":" +
currentEndpoint.getControlPort());
+props.put("Maximum Direct Memory", DrillConfig.getMaxDirectMemory());
+
+return new Stats(props, collectDrillbits(version));
+ }
+
+ private Collection collectDrillbits(String version) {
+Set drillbits = Sets.newTreeSet();
+for (CoordinationProtos.DrillbitEndpoint endpoint :
work.getContext().getBits()) {
+ boolean versionMatch = version.equals(endpoint.getVersion());
+ DrillbitInfo drillbit = new DrillbitInfo(endpoint.getAddress(),
endpoint.isInitialized(), endpoint.getVersion(), versionMatch);
--- End diff --
1. Removed initialized.
2. Check for null is done using freemarker:
<#if
(drillbit.getVersion())?has_content>${drillbit.getVersion()}<#else>Undefined
has_content checks if version is not null and is not empty.
http://freemarker.org/docs/ref_builtins_expert.html#ref_builtin_has_content
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] drill pull request #482: DRILL-4604: Generate warning on Web UI if drillbits...
Github user paul-rogers commented on a diff in the pull request:
https://github.com/apache/drill/pull/482#discussion_r86567189
--- Diff:
exec/java-exec/src/main/java/org/apache/drill/exec/server/rest/DrillRoot.java
---
@@ -55,44 +59,89 @@ public Viewable getStats() {
@GET
@Path("/stats.json")
@Produces(MediaType.APPLICATION_JSON)
- public List getStatsJSON() {
-List stats = Lists.newLinkedList();
-stats.add(new Stat("Number of Drill Bits",
work.getContext().getBits().size()));
-int number = 0;
-for (CoordinationProtos.DrillbitEndpoint bit :
work.getContext().getBits()) {
- String initialized = bit.isInitialized() ? " initialized" : " not
initialized";
- stats.add(new Stat("Bit #" + number, bit.getAddress() +
initialized));
- ++number;
+ public Stats getStatsJSON() {
+final String version =
work.getContext().getOptionManager().getOption(ExecConstants.CLUSTER_VERSION).string_val;
+
+final Map props = Maps.newLinkedHashMap();
+props.put("Cluster Version", version);
+props.put("Number of Drillbits", work.getContext().getBits().size());
--- End diff --
"Number of Drillbits" looks like a user-visible string. Not clear how these
properties are used. In general, property keys should be internal. Then, if
this is for the UI, the UI template should contain the user-visible label. We
generally do not want user-visible strings to appear in the body of Java code.
Seems this is for the REST API. So the properties appear as JSON
properties? Best to use names that are valid identifiers: "clusterVersion",
"drillbitCount", etc.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] drill pull request #482: DRILL-4604: Generate warning on Web UI if drillbits...
Github user paul-rogers commented on a diff in the pull request:
https://github.com/apache/drill/pull/482#discussion_r86565724
--- Diff:
exec/java-exec/src/main/java/org/apache/drill/exec/coord/zk/ZKClusterCoordinator.java
---
@@ -252,6 +252,8 @@ public DrillbitEndpoint
apply(ServiceInstance input) {
builder.append(bit.getControlPort());
builder.append(':');
builder.append(bit.getDataPort());
+ builder.append(". Version - ");
+ builder.append(bit.getVersion());
--- End diff --
Now that we have many fields, would this be better displayed as a table
with columns for each field?
Host | User Port | Control Port | Data Port | Version | ...
In this form we could also show the HTTP port and the port (forgot the
name) whose value is computed rather than configured. Eventually, we can also
show Drillbit status, etc.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] drill pull request #482: DRILL-4604: Generate warning on Web UI if drillbits...
Github user paul-rogers commented on a diff in the pull request:
https://github.com/apache/drill/pull/482#discussion_r86566500
--- Diff:
exec/java-exec/src/main/java/org/apache/drill/exec/server/Drillbit.java ---
@@ -207,6 +210,21 @@ private void javaPropertiesToSystemOptions() {
}
/**
+ * If cluster version is the same as default, update option to current
drillbit version.
+ */
+ private void checkAndUpdateClusterVersionOption(OptionManager
optionManager, DrillbitEndpoint drillbitEndpoint) {
+OptionValue versionOption =
optionManager.getOption(ExecConstants.CLUSTER_VERSION);
+OptionValidator validator =
SystemOptionManager.getValidator(ExecConstants.CLUSTER_VERSION);
+if (versionOption.equals(validator.getDefault())) {
+ optionManager.setOption(OptionValue.createOption(
--- End diff --
I'm not convinced that having the admin specify the cluster option is the
right approach. The proper approach is for each Drillbit to detect that its own
version conflicts with one or more of the others. But, for this to work,
Drillbits have to advertise their version. Do we have that ability?
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] drill pull request #482: DRILL-4604: Generate warning on Web UI if drillbits...
Github user paul-rogers commented on a diff in the pull request:
https://github.com/apache/drill/pull/482#discussion_r86573840
--- Diff: exec/java-exec/src/main/resources/rest/index.ftl ---
@@ -17,17 +17,56 @@
back
-
-
-
-<#list model as stat>
-
-${stat.getName()}
-${stat.getValue()}
-
-
-
-
+
+ <#list model.getDrillbits() as drillbit>
+<#if !drillbit.isVersionMatch()>
+
+×
+Drillbits in the cluster have different versions.
+
+ <#break>
+
+
+
+
+
+ General Info
+
+
+
+<#assign props = model.getProps()>
+<#list props?keys as key>
+
+${key}
+${props[key]}
+
+
+
+
+
+
+
+List of Drillbits
+
+
--- End diff --
Better table would be to have columns for each field rather than putting
all Drillbit data into a single column. (See note elsewhere.)
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] drill pull request #482: DRILL-4604: Generate warning on Web UI if drillbits...
Github user paul-rogers commented on a diff in the pull request:
https://github.com/apache/drill/pull/482#discussion_r86569289
--- Diff:
exec/java-exec/src/main/java/org/apache/drill/exec/server/rest/DrillRoot.java
---
@@ -55,44 +59,89 @@ public Viewable getStats() {
@GET
@Path("/stats.json")
@Produces(MediaType.APPLICATION_JSON)
- public List getStatsJSON() {
-List stats = Lists.newLinkedList();
-stats.add(new Stat("Number of Drill Bits",
work.getContext().getBits().size()));
-int number = 0;
-for (CoordinationProtos.DrillbitEndpoint bit :
work.getContext().getBits()) {
- String initialized = bit.isInitialized() ? " initialized" : " not
initialized";
- stats.add(new Stat("Bit #" + number, bit.getAddress() +
initialized));
- ++number;
+ public Stats getStatsJSON() {
+final String version =
work.getContext().getOptionManager().getOption(ExecConstants.CLUSTER_VERSION).string_val;
+
+final Map props = Maps.newLinkedHashMap();
+props.put("Cluster Version", version);
+props.put("Number of Drillbits", work.getContext().getBits().size());
+CoordinationProtos.DrillbitEndpoint currentEndpoint =
work.getContext().getEndpoint();
+final String address = currentEndpoint.getAddress();
+props.put("Data Port Address", address + ":" +
currentEndpoint.getDataPort());
+props.put("User Port Address", address + ":" +
currentEndpoint.getUserPort());
+props.put("Control Port Address", address + ":" +
currentEndpoint.getControlPort());
+props.put("Maximum Direct Memory", DrillConfig.getMaxDirectMemory());
+
+return new Stats(props, collectDrillbits(version));
+ }
+
+ private Collection collectDrillbits(String version) {
+Set drillbits = Sets.newTreeSet();
+for (CoordinationProtos.DrillbitEndpoint endpoint :
work.getContext().getBits()) {
+ boolean versionMatch = version.equals(endpoint.getVersion());
+ DrillbitInfo drillbit = new DrillbitInfo(endpoint.getAddress(),
endpoint.isInitialized(), endpoint.getVersion(), versionMatch);
+ drillbits.add(drillbit);
}
-stats.add(new Stat("Data Port Address",
work.getContext().getEndpoint().getAddress() +
- ":" + work.getContext().getEndpoint().getDataPort()));
-stats.add(new Stat("User Port Address",
work.getContext().getEndpoint().getAddress() +
- ":" + work.getContext().getEndpoint().getUserPort()));
-stats.add(new Stat("Control Port Address",
work.getContext().getEndpoint().getAddress() +
- ":" + work.getContext().getEndpoint().getControlPort()));
-stats.add(new Stat("Maximum Direct Memory",
DrillConfig.getMaxDirectMemory()));
-
-return stats;
+return drillbits;
}
@XmlRootElement
- public class Stat {
-private String name;
-private Object value;
+ public static class Stats {
+private final Map props;
+private final Collection drillbits;
@JsonCreator
-public Stat(String name, Object value) {
- this.name = name;
- this.value = value;
+public Stats(Map props, Collection
drillbits) {
+ this.props = props;
+ this.drillbits = drillbits;
+}
+
+public Map getProps() {
+ return props;
}
-public String getName() {
- return name;
+public Collection getDrillbits() {
+ return drillbits;
}
+ }
+
+ public static class DrillbitInfo implements Comparable {
+private final String address;
+private final boolean initialized;
+private final String version;
+private final boolean versionMatch;
-public Object getValue() {
- return value;
+@JsonCreator
+public DrillbitInfo(String address, boolean initialized, String
version, boolean versionMatch) {
+ this.address = address;
+ this.initialized = initialized;
+ this.version = version;
+ this.versionMatch = versionMatch;
}
+public String getAddress() {
+ return address;
+}
+
+public String isInitialized() {
+ return initialized ? "initialized" : "not initialized";
--- End diff --
boolean-to-string conversions are best handled in the web UI template so
that they can possibly be localized later. And, all the user-visible text is in
one place.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] drill pull request #482: DRILL-4604: Generate warning on Web UI if drillbits...
Github user paul-rogers commented on a diff in the pull request:
https://github.com/apache/drill/pull/482#discussion_r86573323
--- Diff: exec/java-exec/src/main/resources/rest/index.ftl ---
@@ -17,17 +17,56 @@
back
-
-
-
-<#list model as stat>
-
-${stat.getName()}
-${stat.getValue()}
-
-
-
-
+
+ <#list model.getDrillbits() as drillbit>
+<#if !drillbit.isVersionMatch()>
+
+×
+Drillbits in the cluster have different versions.
+
+ <#break>
+
+
+
+
+
+ General Info
+
+
+
+<#assign props = model.getProps()>
+<#list props?keys as key>
+
+${key}
+${props[key]}
+
+
+
+
+
+
+
+List of Drillbits
+
+
+
+ <#assign i = 1>
+ <#list model.getDrillbits() as drillbit>
+
+ Drillbit # ${i}
+ ${drillbit.getAddress()} ${drillbit.isInitialized()}
--- End diff --
See note above about avoiding inline styles.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] drill pull request #482: DRILL-4604: Generate warning on Web UI if drillbits...
Github user paul-rogers commented on a diff in the pull request:
https://github.com/apache/drill/pull/482#discussion_r86566329
--- Diff:
exec/java-exec/src/main/java/org/apache/drill/exec/coord/zk/ZKClusterCoordinator.java
---
@@ -252,6 +252,8 @@ public DrillbitEndpoint
apply(ServiceInstance input) {
builder.append(bit.getControlPort());
builder.append(':');
builder.append(bit.getDataPort());
+ builder.append(". Version - ");
+ builder.append(bit.getVersion());
--- End diff --
Actually, how is this even possible? Drill bits do not advertise their
version in ZK, do they? How will we get the version of a remote Drillbit?
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] drill pull request #482: DRILL-4604: Generate warning on Web UI if drillbits...
Github user paul-rogers commented on a diff in the pull request:
https://github.com/apache/drill/pull/482#discussion_r86573108
--- Diff: exec/java-exec/src/main/resources/rest/index.ftl ---
@@ -17,17 +17,56 @@
back
-
-
-
-<#list model as stat>
-
-${stat.getName()}
-${stat.getValue()}
-
-
-
-
+
+ <#list model.getDrillbits() as drillbit>
+<#if !drillbit.isVersionMatch()>
+
+×
+Drillbits in the cluster have different versions.
+
+ <#break>
+
+
+
+
+
+ General Info
+
+
+
+<#assign props = model.getProps()>
+<#list props?keys as key>
+
+${key}
+${props[key]}
--- End diff --
Styles should be pulled out into the style sheet. Very hard to maintain
inline styles. And, why do we need to use a different font?
This is web UI, so it is not ensured that every browser has Courier.
Instead, standard practice is to list multiple fonts, with a fallback to, say,
monospace. See [this
page](http://www.w3schools.com/cssref/css_websafe_fonts.asp).
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] drill pull request #482: DRILL-4604: Generate warning on Web UI if drillbits...
Github user paul-rogers commented on a diff in the pull request:
https://github.com/apache/drill/pull/482#discussion_r86569572
--- Diff:
exec/java-exec/src/main/java/org/apache/drill/exec/server/rest/DrillRoot.java
---
@@ -55,44 +59,89 @@ public Viewable getStats() {
@GET
@Path("/stats.json")
@Produces(MediaType.APPLICATION_JSON)
- public List getStatsJSON() {
-List stats = Lists.newLinkedList();
-stats.add(new Stat("Number of Drill Bits",
work.getContext().getBits().size()));
-int number = 0;
-for (CoordinationProtos.DrillbitEndpoint bit :
work.getContext().getBits()) {
- String initialized = bit.isInitialized() ? " initialized" : " not
initialized";
- stats.add(new Stat("Bit #" + number, bit.getAddress() +
initialized));
- ++number;
+ public Stats getStatsJSON() {
+final String version =
work.getContext().getOptionManager().getOption(ExecConstants.CLUSTER_VERSION).string_val;
+
+final Map props = Maps.newLinkedHashMap();
+props.put("Cluster Version", version);
+props.put("Number of Drillbits", work.getContext().getBits().size());
+CoordinationProtos.DrillbitEndpoint currentEndpoint =
work.getContext().getEndpoint();
+final String address = currentEndpoint.getAddress();
+props.put("Data Port Address", address + ":" +
currentEndpoint.getDataPort());
+props.put("User Port Address", address + ":" +
currentEndpoint.getUserPort());
+props.put("Control Port Address", address + ":" +
currentEndpoint.getControlPort());
+props.put("Maximum Direct Memory", DrillConfig.getMaxDirectMemory());
+
+return new Stats(props, collectDrillbits(version));
+ }
+
+ private Collection collectDrillbits(String version) {
+Set drillbits = Sets.newTreeSet();
+for (CoordinationProtos.DrillbitEndpoint endpoint :
work.getContext().getBits()) {
+ boolean versionMatch = version.equals(endpoint.getVersion());
+ DrillbitInfo drillbit = new DrillbitInfo(endpoint.getAddress(),
endpoint.isInitialized(), endpoint.getVersion(), versionMatch);
+ drillbits.add(drillbit);
}
-stats.add(new Stat("Data Port Address",
work.getContext().getEndpoint().getAddress() +
- ":" + work.getContext().getEndpoint().getDataPort()));
-stats.add(new Stat("User Port Address",
work.getContext().getEndpoint().getAddress() +
- ":" + work.getContext().getEndpoint().getUserPort()));
-stats.add(new Stat("Control Port Address",
work.getContext().getEndpoint().getAddress() +
- ":" + work.getContext().getEndpoint().getControlPort()));
-stats.add(new Stat("Maximum Direct Memory",
DrillConfig.getMaxDirectMemory()));
-
-return stats;
+return drillbits;
}
@XmlRootElement
- public class Stat {
-private String name;
-private Object value;
+ public static class Stats {
+private final Map props;
+private final Collection drillbits;
@JsonCreator
-public Stat(String name, Object value) {
- this.name = name;
- this.value = value;
+public Stats(Map props, Collection
drillbits) {
+ this.props = props;
+ this.drillbits = drillbits;
+}
+
+public Map getProps() {
+ return props;
}
-public String getName() {
- return name;
+public Collection getDrillbits() {
+ return drillbits;
}
+ }
+
+ public static class DrillbitInfo implements Comparable {
+private final String address;
+private final boolean initialized;
+private final String version;
+private final boolean versionMatch;
-public Object getValue() {
- return value;
+@JsonCreator
+public DrillbitInfo(String address, boolean initialized, String
version, boolean versionMatch) {
+ this.address = address;
+ this.initialized = initialized;
+ this.version = version;
+ this.versionMatch = versionMatch;
}
+public String getAddress() {
+ return address;
+}
+
+public String isInitialized() {
+ return initialized ? "initialized" : "not initialized";
+}
+
+public String getVersion() {
+ return version;
+}
+
+public boolean isVersionMatch() {
+ return versionMatch;
+}
+
+@Override
+public int compareTo(DrillbitInfo o) {
--- End diff --
We store a version match, and yet we also provide a comparison. What
version are we comparing here? What does it mean to have a Drillbit A that
conflicts with the cluster but does or does not conflict with Drillbits B, C,
or D? Javadoc would be helpful
---
If your proje
[GitHub] drill pull request #482: DRILL-4604: Generate warning on Web UI if drillbits...
Github user paul-rogers commented on a diff in the pull request:
https://github.com/apache/drill/pull/482#discussion_r86573607
--- Diff: exec/java-exec/src/main/resources/rest/index.ftl ---
@@ -17,17 +17,56 @@
back
-
-
-
-<#list model as stat>
-
-${stat.getName()}
-${stat.getValue()}
-
-
-
-
+
+ <#list model.getDrillbits() as drillbit>
+<#if !drillbit.isVersionMatch()>
+
+×
+Drillbits in the cluster have different versions.
+
+ <#break>
+
+
+
+
+
+ General Info
+
+
+
+<#assign props = model.getProps()>
+<#list props?keys as key>
+
+${key}
+${props[key]}
+
+
+
+
+
+
+
+List of Drillbits
+
+
+
+ <#assign i = 1>
+ <#list model.getDrillbits() as drillbit>
+
+ Drillbit # ${i}
+ ${drillbit.getAddress()} ${drillbit.isInitialized()}
+
+
+ <#if
(drillbit.getVersion())?has_content>${drillbit.getVersion()}<#else>Undefined
--- End diff --
Nice trick: ${drillbit.version()!"Undefined"}
The "!" says "if the preceding is defined and not null, use it, else use
the thing that comes next."
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] drill pull request #482: DRILL-4604: Generate warning on Web UI if drillbits...
Github user paul-rogers commented on a diff in the pull request:
https://github.com/apache/drill/pull/482#discussion_r86570460
--- Diff: exec/java-exec/src/main/resources/rest/index.ftl ---
@@ -17,17 +17,56 @@
back
-
-
-
-<#list model as stat>
-
-${stat.getName()}
-${stat.getValue()}
-
-
-
-
+
+ <#list model.getDrillbits() as drillbit>
+<#if !drillbit.isVersionMatch()>
+
+×
+Drillbits in the cluster have different versions.
--- End diff --
Can we include more info? Something like:
Cluster version is x.y.z, but Drillbits in the cluster have other versions:
a.b.c, d.e.f and i.j.k. Drill does not support clusters containing a mix of
Drillbit versions.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] drill pull request #482: DRILL-4604: Generate warning on Web UI if drillbits...
Github user paul-rogers commented on a diff in the pull request:
https://github.com/apache/drill/pull/482#discussion_r86570176
--- Diff: exec/java-exec/src/main/resources/rest/index.ftl ---
@@ -17,17 +17,56 @@
back
-
-
-
-<#list model as stat>
-
-${stat.getName()}
-${stat.getValue()}
-
-
-
-
+
+ <#list model.getDrillbits() as drillbit>
+<#if !drillbit.isVersionMatch()>
+
--- End diff --
This seems to emit a warning about version mismatch for every mismatched
Drillbit. Seems it would be better to list the Drillbits, and look for
mismatches. Then, if found, perhaps:
Highlight the mismatched version (in red or whatever) or each Drillbit,
then, after the list of bits, display the message that "One or more drill bits,
highlighted above, have versions different from the cluster version of x.y.z."
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] drill pull request #482: DRILL-4604: Generate warning on Web UI if drillbits...
Github user paul-rogers commented on a diff in the pull request:
https://github.com/apache/drill/pull/482#discussion_r86568133
--- Diff:
exec/java-exec/src/main/java/org/apache/drill/exec/server/rest/DrillRoot.java
---
@@ -55,44 +59,89 @@ public Viewable getStats() {
@GET
@Path("/stats.json")
@Produces(MediaType.APPLICATION_JSON)
- public List getStatsJSON() {
-List stats = Lists.newLinkedList();
-stats.add(new Stat("Number of Drill Bits",
work.getContext().getBits().size()));
-int number = 0;
-for (CoordinationProtos.DrillbitEndpoint bit :
work.getContext().getBits()) {
- String initialized = bit.isInitialized() ? " initialized" : " not
initialized";
- stats.add(new Stat("Bit #" + number, bit.getAddress() +
initialized));
- ++number;
+ public Stats getStatsJSON() {
--- End diff --
This message gets statistics, which is great. But, it seems to combine
per-cluster stats with per-drillbit info. Perhaps either have two messages (one
for cluster, the other for Drillbit) or use JSON structures:
```
clusterStats: { drillbitCount: .., version: ... },
drillbitStats: { userPort: ..., ... }
```
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] drill pull request #482: DRILL-4604: Generate warning on Web UI if drillbits...
Github user paul-rogers commented on a diff in the pull request:
https://github.com/apache/drill/pull/482#discussion_r86568636
--- Diff:
exec/java-exec/src/main/java/org/apache/drill/exec/server/rest/DrillRoot.java
---
@@ -55,44 +59,89 @@ public Viewable getStats() {
@GET
@Path("/stats.json")
@Produces(MediaType.APPLICATION_JSON)
- public List getStatsJSON() {
-List stats = Lists.newLinkedList();
-stats.add(new Stat("Number of Drill Bits",
work.getContext().getBits().size()));
-int number = 0;
-for (CoordinationProtos.DrillbitEndpoint bit :
work.getContext().getBits()) {
- String initialized = bit.isInitialized() ? " initialized" : " not
initialized";
- stats.add(new Stat("Bit #" + number, bit.getAddress() +
initialized));
- ++number;
+ public Stats getStatsJSON() {
+final String version =
work.getContext().getOptionManager().getOption(ExecConstants.CLUSTER_VERSION).string_val;
+
+final Map props = Maps.newLinkedHashMap();
+props.put("Cluster Version", version);
+props.put("Number of Drillbits", work.getContext().getBits().size());
+CoordinationProtos.DrillbitEndpoint currentEndpoint =
work.getContext().getEndpoint();
+final String address = currentEndpoint.getAddress();
+props.put("Data Port Address", address + ":" +
currentEndpoint.getDataPort());
+props.put("User Port Address", address + ":" +
currentEndpoint.getUserPort());
+props.put("Control Port Address", address + ":" +
currentEndpoint.getControlPort());
+props.put("Maximum Direct Memory", DrillConfig.getMaxDirectMemory());
+
+return new Stats(props, collectDrillbits(version));
+ }
+
+ private Collection collectDrillbits(String version) {
--- End diff --
Rename or document what "version" is. Presumably, this is the cluster
version?
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] drill pull request #482: DRILL-4604: Generate warning on Web UI if drillbits...
Github user paul-rogers commented on a diff in the pull request: https://github.com/apache/drill/pull/482#discussion_r86565189 --- Diff: exec/java-exec/src/main/java/org/apache/drill/exec/ExecConstants.java --- @@ -361,4 +361,11 @@ String DYNAMIC_UDF_SUPPORT_ENABLED = "exec.udf.enable_dynamic_support"; BooleanValidator DYNAMIC_UDF_SUPPORT_ENABLED_VALIDATOR = new BooleanValidator(DYNAMIC_UDF_SUPPORT_ENABLED, true, true); + + /** + * Option representing Drill cluster version + */ + String CLUSTER_VERSION = "drill.exec.cluster.version"; --- End diff -- What is a cluster version? Is this a well-defined concept? If there is such a thing, isn't it the version of all Drillbits in the cluster? It would not be a config parameter, would it? --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] drill pull request #482: DRILL-4604: Generate warning on Web UI if drillbits...
Github user paul-rogers commented on a diff in the pull request:
https://github.com/apache/drill/pull/482#discussion_r86568915
--- Diff:
exec/java-exec/src/main/java/org/apache/drill/exec/server/rest/DrillRoot.java
---
@@ -55,44 +59,89 @@ public Viewable getStats() {
@GET
@Path("/stats.json")
@Produces(MediaType.APPLICATION_JSON)
- public List getStatsJSON() {
-List stats = Lists.newLinkedList();
-stats.add(new Stat("Number of Drill Bits",
work.getContext().getBits().size()));
-int number = 0;
-for (CoordinationProtos.DrillbitEndpoint bit :
work.getContext().getBits()) {
- String initialized = bit.isInitialized() ? " initialized" : " not
initialized";
- stats.add(new Stat("Bit #" + number, bit.getAddress() +
initialized));
- ++number;
+ public Stats getStatsJSON() {
+final String version =
work.getContext().getOptionManager().getOption(ExecConstants.CLUSTER_VERSION).string_val;
+
+final Map props = Maps.newLinkedHashMap();
+props.put("Cluster Version", version);
+props.put("Number of Drillbits", work.getContext().getBits().size());
+CoordinationProtos.DrillbitEndpoint currentEndpoint =
work.getContext().getEndpoint();
+final String address = currentEndpoint.getAddress();
+props.put("Data Port Address", address + ":" +
currentEndpoint.getDataPort());
+props.put("User Port Address", address + ":" +
currentEndpoint.getUserPort());
+props.put("Control Port Address", address + ":" +
currentEndpoint.getControlPort());
+props.put("Maximum Direct Memory", DrillConfig.getMaxDirectMemory());
+
+return new Stats(props, collectDrillbits(version));
+ }
+
+ private Collection collectDrillbits(String version) {
+Set drillbits = Sets.newTreeSet();
+for (CoordinationProtos.DrillbitEndpoint endpoint :
work.getContext().getBits()) {
+ boolean versionMatch = version.equals(endpoint.getVersion());
+ DrillbitInfo drillbit = new DrillbitInfo(endpoint.getAddress(),
endpoint.isInitialized(), endpoint.getVersion(), versionMatch);
+ drillbits.add(drillbit);
}
-stats.add(new Stat("Data Port Address",
work.getContext().getEndpoint().getAddress() +
- ":" + work.getContext().getEndpoint().getDataPort()));
-stats.add(new Stat("User Port Address",
work.getContext().getEndpoint().getAddress() +
- ":" + work.getContext().getEndpoint().getUserPort()));
-stats.add(new Stat("Control Port Address",
work.getContext().getEndpoint().getAddress() +
- ":" + work.getContext().getEndpoint().getControlPort()));
-stats.add(new Stat("Maximum Direct Memory",
DrillConfig.getMaxDirectMemory()));
-
-return stats;
+return drillbits;
}
@XmlRootElement
- public class Stat {
-private String name;
-private Object value;
+ public static class Stats {
--- End diff --
Would actually be helpful to include Javadoc to explain the use of this
stuff. Is this used as the data model for the Freemarker-based web UI?
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] drill pull request #482: DRILL-4604: Generate warning on Web UI if drillbits...
Github user paul-rogers commented on a diff in the pull request:
https://github.com/apache/drill/pull/482#discussion_r86572497
--- Diff: exec/java-exec/src/main/resources/rest/index.ftl ---
@@ -17,17 +17,56 @@
back
-
-
-
-<#list model as stat>
-
-${stat.getName()}
-${stat.getValue()}
-
-
-
-
+
+ <#list model.getDrillbits() as drillbit>
+<#if !drillbit.isVersionMatch()>
+
+×
+Drillbits in the cluster have different versions.
+
+ <#break>
+
+
+
+
+
+ General Info
+
+
+
+<#assign props = model.getProps()>
+<#list props?keys as key>
+
+${key}
--- End diff --
I see what we're doing and it is clever. However, it forces us to put the
user-visible labels as keys. Since hash maps are unordered, it means that we
can't control the order of display of the properties. Two solutions.
1. Design the layout:
```
Version ...
${props.version} ...
```
Or provide two levels of dynamics:
```
class DisplayPair { String key; String label; ... }
List list = ...; list.add( "version", "Version" };
...
<#list pairs as pair>
${pair.label}
...
<#list pairs as pair>
${model[pair.key]}
```
The first is simpler and is fine: it lets the UI decide which attributes to
display, in what form.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] drill pull request #482: DRILL-4604: Generate warning on Web UI if drillbits...
Github user paul-rogers commented on a diff in the pull request:
https://github.com/apache/drill/pull/482#discussion_r8657
--- Diff:
exec/java-exec/src/main/java/org/apache/drill/exec/server/Drillbit.java ---
@@ -218,13 +218,18 @@ private void javaPropertiesToSystemOptions() {
}
/**
- * If cluster version is the same as default, update option to current
drillbit version.
+ * If cluster version is the same as default, updates option to current
drillbit version.
+ * Since getOption and setOption are separate calls, raise condition
might occur
+ * when several drillbits are registering at the same time.
+ * It is assumed that the first drillbit in cluster sets cluster version
+ * but when the raise condition occurs, it might be the second or the
third etc.
--- End diff --
raise --> race
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] drill pull request #482: DRILL-4604: Generate warning on Web UI if drillbits...
Github user paul-rogers commented on a diff in the pull request:
https://github.com/apache/drill/pull/482#discussion_r86574396
--- Diff:
exec/java-exec/src/main/java/org/apache/drill/exec/server/Drillbit.java ---
@@ -218,13 +218,18 @@ private void javaPropertiesToSystemOptions() {
}
/**
- * If cluster version is the same as default, update option to current
drillbit version.
+ * If cluster version is the same as default, updates option to current
drillbit version.
+ * Since getOption and setOption are separate calls, raise condition
might occur
--- End diff --
raise --> race
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] drill pull request #482: DRILL-4604: Generate warning on Web UI if drillbits...
Github user paul-rogers commented on a diff in the pull request:
https://github.com/apache/drill/pull/482#discussion_r86574518
--- Diff:
exec/java-exec/src/main/java/org/apache/drill/exec/server/Drillbit.java ---
@@ -218,13 +218,18 @@ private void javaPropertiesToSystemOptions() {
}
/**
- * If cluster version is the same as default, update option to current
drillbit version.
+ * If cluster version is the same as default, updates option to current
drillbit version.
+ * Since getOption and setOption are separate calls, raise condition
might occur
+ * when several drillbits are registering at the same time.
+ * It is assumed that the first drillbit in cluster sets cluster version
+ * but when the raise condition occurs, it might be the second or the
third etc.
+ * This behaviour does not impose significant impact, since the main
goal is to detect mismatching versions.
--- End diff --
mismatching --> mismatched
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] drill pull request #482: DRILL-4604: Generate warning on Web UI if drillbits...
Github user paul-rogers commented on a diff in the pull request:
https://github.com/apache/drill/pull/482#discussion_r86575850
--- Diff:
exec/java-exec/src/main/java/org/apache/drill/exec/server/Drillbit.java ---
@@ -218,13 +218,18 @@ private void javaPropertiesToSystemOptions() {
}
/**
- * If cluster version is the same as default, update option to current
drillbit version.
+ * If cluster version is the same as default, updates option to current
drillbit version.
+ * Since getOption and setOption are separate calls, raise condition
might occur
+ * when several drillbits are registering at the same time.
+ * It is assumed that the first drillbit in cluster sets cluster version
+ * but when the raise condition occurs, it might be the second or the
third etc.
+ * This behaviour does not impose significant impact, since the main
goal is to detect mismatching versions.
*/
- private void checkAndUpdateClusterVersionOption(OptionManager
optionManager, DrillbitEndpoint drillbitEndpoint) {
-OptionValue versionOption =
optionManager.getOption(ExecConstants.CLUSTER_VERSION);
+ private void checkAndUpdateClusterVersionOption(OptionManager
systemOptions, DrillbitEndpoint drillbitEndpoint) {
--- End diff --
Actually, I don't think this master version is even needed. Instead:
1. Each time a new Drillbit is registered from ZK (see
ZKClusterCoordinator),
2. Check the version of that new Drillbit against the version of this
Drillbit.
3. If mismatch, log an error.
4. In the web UI, display the version of this Drillbit.
5. In the list of Drillbits, highlight any Drillbit with a different
version.
6. If any Drillbits have a different version, display a warning message
after the list of Drillbits.
With the above approach, there is no state to be cleared if the admin
corrects the problem. With the cluster version state, it may be that someone
has to reset the old cluster version when the mismatch problem is fixed.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] drill pull request #482: DRILL-4604: Generate warning on Web UI if drillbits...
Github user paul-rogers commented on a diff in the pull request:
https://github.com/apache/drill/pull/482#discussion_r86575055
--- Diff:
exec/java-exec/src/main/java/org/apache/drill/exec/server/Drillbit.java ---
@@ -218,13 +218,18 @@ private void javaPropertiesToSystemOptions() {
}
/**
- * If cluster version is the same as default, update option to current
drillbit version.
+ * If cluster version is the same as default, updates option to current
drillbit version.
+ * Since getOption and setOption are separate calls, raise condition
might occur
+ * when several drillbits are registering at the same time.
+ * It is assumed that the first drillbit in cluster sets cluster version
+ * but when the raise condition occurs, it might be the second or the
third etc.
+ * This behaviour does not impose significant impact, since the main
goal is to detect mismatching versions.
*/
- private void checkAndUpdateClusterVersionOption(OptionManager
optionManager, DrillbitEndpoint drillbitEndpoint) {
-OptionValue versionOption =
optionManager.getOption(ExecConstants.CLUSTER_VERSION);
+ private void checkAndUpdateClusterVersionOption(OptionManager
systemOptions, DrillbitEndpoint drillbitEndpoint) {
--- End diff --
How can this work? Consider this:
1. Start a new cluster.
2. A Drillbit registers. It sets the cluster version.
3. A second Drillbit registers. Does it reset the version? Or, does it
accept that the first has already set the version?
4. Shut down the cluster.
5. Upgrade Drill to a new version.
6. Start the first Drillbit.
7. Does this Drillbit overwrite the cluster version? Or, accept the
existing (old) version?
Note that steps 3 and 7 are identical from ZK's perspective.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] drill pull request #482: DRILL-4604: Generate warning on Web UI if drillbits...
Github user arina-ielchiieva commented on a diff in the pull request:
https://github.com/apache/drill/pull/482#discussion_r86579857
--- Diff:
exec/java-exec/src/main/java/org/apache/drill/exec/server/Drillbit.java ---
@@ -218,13 +218,18 @@ private void javaPropertiesToSystemOptions() {
}
/**
- * If cluster version is the same as default, update option to current
drillbit version.
+ * If cluster version is the same as default, updates option to current
drillbit version.
+ * Since getOption and setOption are separate calls, raise condition
might occur
+ * when several drillbits are registering at the same time.
+ * It is assumed that the first drillbit in cluster sets cluster version
+ * but when the raise condition occurs, it might be the second or the
third etc.
+ * This behaviour does not impose significant impact, since the main
goal is to detect mismatching versions.
*/
- private void checkAndUpdateClusterVersionOption(OptionManager
optionManager, DrillbitEndpoint drillbitEndpoint) {
-OptionValue versionOption =
optionManager.getOption(ExecConstants.CLUSTER_VERSION);
+ private void checkAndUpdateClusterVersionOption(OptionManager
systemOptions, DrillbitEndpoint drillbitEndpoint) {
--- End diff --
@paul-rogers
your alternative state-free solution totally makes sense. I'll make
appropriate changes.
Thank you!
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] drill pull request #482: DRILL-4604: Generate warning on Web UI if drillbits...
Github user arina-ielchiieva commented on a diff in the pull request:
https://github.com/apache/drill/pull/482#discussion_r89306191
--- Diff:
exec/java-exec/src/main/java/org/apache/drill/exec/server/Drillbit.java ---
@@ -218,13 +218,18 @@ private void javaPropertiesToSystemOptions() {
}
/**
- * If cluster version is the same as default, update option to current
drillbit version.
+ * If cluster version is the same as default, updates option to current
drillbit version.
+ * Since getOption and setOption are separate calls, raise condition
might occur
+ * when several drillbits are registering at the same time.
+ * It is assumed that the first drillbit in cluster sets cluster version
+ * but when the raise condition occurs, it might be the second or the
third etc.
--- End diff --
Not relevant since method was removed.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] drill pull request #482: DRILL-4604: Generate warning on Web UI if drillbits...
Github user arina-ielchiieva commented on a diff in the pull request:
https://github.com/apache/drill/pull/482#discussion_r89306930
--- Diff: exec/java-exec/src/main/resources/rest/index.ftl ---
@@ -17,17 +17,56 @@
back
-
-
-
-<#list model as stat>
-
-${stat.getName()}
-${stat.getValue()}
-
-
-
-
+
+ <#list model.getDrillbits() as drillbit>
+<#if !drillbit.isVersionMatch()>
+
--- End diff --
Code was changes a little. But if we have drillbit version mismatch only
one warning message will be displayed. I'll add screenshots. Drillbits with
mismatching version will have their version in red label, drillbits with
matching version will have their version in green label.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] drill pull request #482: DRILL-4604: Generate warning on Web UI if drillbits...
Github user arina-ielchiieva commented on a diff in the pull request:
https://github.com/apache/drill/pull/482#discussion_r89307032
--- Diff: exec/java-exec/src/main/resources/rest/index.ftl ---
@@ -17,17 +17,56 @@
back
-
-
-
-<#list model as stat>
-
-${stat.getName()}
-${stat.getValue()}
-
-
-
-
+
+ <#list model.getDrillbits() as drillbit>
+<#if !drillbit.isVersionMatch()>
+
+×
+Drillbits in the cluster have different versions.
--- End diff --
Agree.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] drill pull request #482: DRILL-4604: Generate warning on Web UI if drillbits...
Github user arina-ielchiieva commented on a diff in the pull request: https://github.com/apache/drill/pull/482#discussion_r89306035 --- Diff: exec/java-exec/src/main/java/org/apache/drill/exec/ExecConstants.java --- @@ -361,4 +361,11 @@ String DYNAMIC_UDF_SUPPORT_ENABLED = "exec.udf.enable_dynamic_support"; BooleanValidator DYNAMIC_UDF_SUPPORT_ENABLED_VALIDATOR = new BooleanValidator(DYNAMIC_UDF_SUPPORT_ENABLED, true, true); + + /** + * Option representing Drill cluster version + */ + String CLUSTER_VERSION = "drill.exec.cluster.version"; --- End diff -- Removed as we switched to state-free solution. --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] drill pull request #482: DRILL-4604: Generate warning on Web UI if drillbits...
Github user arina-ielchiieva commented on a diff in the pull request:
https://github.com/apache/drill/pull/482#discussion_r89306198
--- Diff:
exec/java-exec/src/main/java/org/apache/drill/exec/server/Drillbit.java ---
@@ -218,13 +218,18 @@ private void javaPropertiesToSystemOptions() {
}
/**
- * If cluster version is the same as default, update option to current
drillbit version.
+ * If cluster version is the same as default, updates option to current
drillbit version.
+ * Since getOption and setOption are separate calls, raise condition
might occur
+ * when several drillbits are registering at the same time.
+ * It is assumed that the first drillbit in cluster sets cluster version
+ * but when the raise condition occurs, it might be the second or the
third etc.
+ * This behaviour does not impose significant impact, since the main
goal is to detect mismatching versions.
--- End diff --
Not relevant since method was removed.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] drill pull request #482: DRILL-4604: Generate warning on Web UI if drillbits...
Github user arina-ielchiieva commented on a diff in the pull request:
https://github.com/apache/drill/pull/482#discussion_r89314720
--- Diff:
exec/java-exec/src/main/java/org/apache/drill/exec/coord/zk/ZKClusterCoordinator.java
---
@@ -252,6 +252,8 @@ public DrillbitEndpoint
apply(ServiceInstance input) {
builder.append(bit.getControlPort());
builder.append(':');
builder.append(bit.getDataPort());
+ builder.append(". Version - ");
+ builder.append(bit.getVersion());
--- End diff --
1. Changes drillbits representation in log debug to table like structure.
2. Drillbits start advertising their version to ZK in the scope of this
Jira.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] drill pull request #482: DRILL-4604: Generate warning on Web UI if drillbits...
Github user arina-ielchiieva commented on a diff in the pull request:
https://github.com/apache/drill/pull/482#discussion_r89498779
--- Diff: exec/java-exec/src/main/resources/rest/index.ftl ---
@@ -17,17 +17,56 @@
back
-
-
-
-<#list model as stat>
-
-${stat.getName()}
-${stat.getValue()}
-
-
-
-
+
+ <#list model.getDrillbits() as drillbit>
+<#if !drillbit.isVersionMatch()>
+
+×
+Drillbits in the cluster have different versions.
+
+ <#break>
+
+
+
+
+
+ General Info
+
+
+
+<#assign props = model.getProps()>
+<#list props?keys as key>
+
+${key}
+${props[key]}
+
+
+
+
+
+
+
+List of Drillbits
+
+
--- End diff --
Agree. Screenshot will be attached.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] drill pull request #482: DRILL-4604: Generate warning on Web UI if drillbits...
Github user arina-ielchiieva commented on a diff in the pull request:
https://github.com/apache/drill/pull/482#discussion_r89306479
--- Diff:
exec/java-exec/src/main/java/org/apache/drill/exec/server/rest/DrillRoot.java
---
@@ -55,44 +59,89 @@ public Viewable getStats() {
@GET
@Path("/stats.json")
@Produces(MediaType.APPLICATION_JSON)
- public List getStatsJSON() {
-List stats = Lists.newLinkedList();
-stats.add(new Stat("Number of Drill Bits",
work.getContext().getBits().size()));
-int number = 0;
-for (CoordinationProtos.DrillbitEndpoint bit :
work.getContext().getBits()) {
- String initialized = bit.isInitialized() ? " initialized" : " not
initialized";
- stats.add(new Stat("Bit #" + number, bit.getAddress() +
initialized));
- ++number;
+ public Stats getStatsJSON() {
+final String version =
work.getContext().getOptionManager().getOption(ExecConstants.CLUSTER_VERSION).string_val;
+
+final Map props = Maps.newLinkedHashMap();
+props.put("Cluster Version", version);
+props.put("Number of Drillbits", work.getContext().getBits().size());
+CoordinationProtos.DrillbitEndpoint currentEndpoint =
work.getContext().getEndpoint();
+final String address = currentEndpoint.getAddress();
+props.put("Data Port Address", address + ":" +
currentEndpoint.getDataPort());
+props.put("User Port Address", address + ":" +
currentEndpoint.getUserPort());
+props.put("Control Port Address", address + ":" +
currentEndpoint.getControlPort());
+props.put("Maximum Direct Memory", DrillConfig.getMaxDirectMemory());
+
+return new Stats(props, collectDrillbits(version));
+ }
+
+ private Collection collectDrillbits(String version) {
+Set drillbits = Sets.newTreeSet();
+for (CoordinationProtos.DrillbitEndpoint endpoint :
work.getContext().getBits()) {
+ boolean versionMatch = version.equals(endpoint.getVersion());
+ DrillbitInfo drillbit = new DrillbitInfo(endpoint.getAddress(),
endpoint.isInitialized(), endpoint.getVersion(), versionMatch);
+ drillbits.add(drillbit);
}
-stats.add(new Stat("Data Port Address",
work.getContext().getEndpoint().getAddress() +
- ":" + work.getContext().getEndpoint().getDataPort()));
-stats.add(new Stat("User Port Address",
work.getContext().getEndpoint().getAddress() +
- ":" + work.getContext().getEndpoint().getUserPort()));
-stats.add(new Stat("Control Port Address",
work.getContext().getEndpoint().getAddress() +
- ":" + work.getContext().getEndpoint().getControlPort()));
-stats.add(new Stat("Maximum Direct Memory",
DrillConfig.getMaxDirectMemory()));
-
-return stats;
+return drillbits;
}
@XmlRootElement
- public class Stat {
-private String name;
-private Object value;
+ public static class Stats {
+private final Map props;
+private final Collection drillbits;
@JsonCreator
-public Stat(String name, Object value) {
- this.name = name;
- this.value = value;
+public Stats(Map props, Collection
drillbits) {
+ this.props = props;
+ this.drillbits = drillbits;
+}
+
+public Map getProps() {
+ return props;
}
-public String getName() {
- return name;
+public Collection getDrillbits() {
+ return drillbits;
}
+ }
+
+ public static class DrillbitInfo implements Comparable {
+private final String address;
+private final boolean initialized;
+private final String version;
+private final boolean versionMatch;
-public Object getValue() {
- return value;
+@JsonCreator
+public DrillbitInfo(String address, boolean initialized, String
version, boolean versionMatch) {
+ this.address = address;
+ this.initialized = initialized;
+ this.version = version;
+ this.versionMatch = versionMatch;
}
+public String getAddress() {
+ return address;
+}
+
+public String isInitialized() {
+ return initialized ? "initialized" : "not initialized";
+}
+
+public String getVersion() {
+ return version;
+}
+
+public boolean isVersionMatch() {
+ return versionMatch;
+}
+
+@Override
+public int compareTo(DrillbitInfo o) {
--- End diff --
We store all drillbits in Set, we need compareTo method so they are
displayed sorted. First we display current drillbit, then all drillbits with
version match and then all drillbits with version mismatch. I'll add
appropriate comments.
---
If your project is set up
[GitHub] drill pull request #482: DRILL-4604: Generate warning on Web UI if drillbits...
Github user asfgit closed the pull request at: https://github.com/apache/drill/pull/482 --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
