Michael Blow has uploaded a new change for review.

  https://asterix-gerrit.ics.uci.edu/1820

Change subject: Remove -virtual-nc option
......................................................................

Remove -virtual-nc option

Avoid hidden option '-virtual-nc' to indicate when NCService should not
be used to start the NC, instead use ncservice.port of -1 to indicate
the same

Change-Id: I67a9a88808a3d1352b5fdd45ebd158e98dc72dba
---
M 
asterixdb/asterix-app/src/main/java/org/apache/asterix/api/common/AsterixHyracksIntegrationUtil.java
M 
asterixdb/asterix-app/src/main/java/org/apache/asterix/hyracks/bootstrap/NCApplication.java
M 
asterixdb/asterix-common/src/main/java/org/apache/asterix/common/config/NodeProperties.java
M 
asterixdb/asterix-common/src/main/java/org/apache/asterix/common/config/PropertiesAccessor.java
M 
asterixdb/asterix-runtime/src/main/java/org/apache/asterix/runtime/utils/ClusterStateManager.java
M 
hyracks-fullstack/hyracks/hyracks-control/hyracks-control-cc/src/main/java/org/apache/hyracks/control/cc/ClusterControllerService.java
M 
hyracks-fullstack/hyracks/hyracks-control/hyracks-control-common/src/main/java/org/apache/hyracks/control/common/config/ConfigManager.java
M 
hyracks-fullstack/hyracks/hyracks-control/hyracks-control-common/src/main/java/org/apache/hyracks/control/common/controllers/NCConfig.java
8 files changed, 29 insertions(+), 32 deletions(-)


  git pull ssh://asterix-gerrit.ics.uci.edu:29418/asterixdb 
refs/changes/20/1820/1

diff --git 
a/asterixdb/asterix-app/src/main/java/org/apache/asterix/api/common/AsterixHyracksIntegrationUtil.java
 
b/asterixdb/asterix-app/src/main/java/org/apache/asterix/api/common/AsterixHyracksIntegrationUtil.java
index 2a0631c..9d01d63 100644
--- 
a/asterixdb/asterix-app/src/main/java/org/apache/asterix/api/common/AsterixHyracksIntegrationUtil.java
+++ 
b/asterixdb/asterix-app/src/main/java/org/apache/asterix/api/common/AsterixHyracksIntegrationUtil.java
@@ -86,7 +86,7 @@
         final List<NodeControllerService> nodeControllers = new ArrayList<>();
         for (String nodeId : nodeNames) {
             // mark this NC as virtual in the CC's config manager, so he 
doesn't try to contact NCService...
-            configManager.set(nodeId, NCConfig.Option.VIRTUAL_NC, true);
+            configManager.set(nodeId, NCConfig.Option.NCSERVICE_PORT, 
NCConfig.NCSERVICE_PORT_DISABLED);
             final INCApplication ncApplication = createNCApplication();
             ConfigManager ncConfigManager = new ConfigManager();
             ncApplication.registerConfig(ncConfigManager);
@@ -149,7 +149,7 @@
         
ncConfig.setMessagingListenAddress(Inet4Address.getLoopbackAddress().getHostAddress());
         ncConfig.setResultTTL(120000L);
         ncConfig.setResultSweepThreshold(1000L);
-        ncConfig.setVirtualNC(true);
+        ncConfig.setVirtualNC();
         configManager.set(ControllerConfig.Option.DEFAULT_DIR, 
joinPath(getDefaultStoragePath(), "asterixdb", ncName));
         return ncConfig;
     }
diff --git 
a/asterixdb/asterix-app/src/main/java/org/apache/asterix/hyracks/bootstrap/NCApplication.java
 
b/asterixdb/asterix-app/src/main/java/org/apache/asterix/hyracks/bootstrap/NCApplication.java
index 4aeb098..107b859 100644
--- 
a/asterixdb/asterix-app/src/main/java/org/apache/asterix/hyracks/bootstrap/NCApplication.java
+++ 
b/asterixdb/asterix-app/src/main/java/org/apache/asterix/hyracks/bootstrap/NCApplication.java
@@ -231,7 +231,7 @@
                 throw new IllegalStateException("No cluster configuration 
found for this instance");
             }
             NCConfig ncConfig = ((NodeControllerService) 
ncServiceCtx.getControllerService()).getConfiguration();
-            ncConfig.getConfigManager().registerVirtualNode(nodeId);
+            ncConfig.getConfigManager().ensureNode(nodeId);
             String asterixInstanceName = metadataProperties.getInstanceName();
             TransactionProperties txnProperties = 
runtimeContext.getTransactionProperties();
             Node self = null;
diff --git 
a/asterixdb/asterix-common/src/main/java/org/apache/asterix/common/config/NodeProperties.java
 
b/asterixdb/asterix-common/src/main/java/org/apache/asterix/common/config/NodeProperties.java
index 7f09b98..2e25e34 100644
--- 
a/asterixdb/asterix-common/src/main/java/org/apache/asterix/common/config/NodeProperties.java
+++ 
b/asterixdb/asterix-common/src/main/java/org/apache/asterix/common/config/NodeProperties.java
@@ -110,6 +110,6 @@
     }
 
     public boolean isVirtualNc() {
-        return accessor.getBoolean(NCConfig.Option.VIRTUAL_NC);
+        return accessor.getInt(NCConfig.Option.NCSERVICE_PORT) == 
NCConfig.NCSERVICE_PORT_DISABLED;
     }
 }
diff --git 
a/asterixdb/asterix-common/src/main/java/org/apache/asterix/common/config/PropertiesAccessor.java
 
b/asterixdb/asterix-common/src/main/java/org/apache/asterix/common/config/PropertiesAccessor.java
index d011864..eacc18b 100644
--- 
a/asterixdb/asterix-common/src/main/java/org/apache/asterix/common/config/PropertiesAccessor.java
+++ 
b/asterixdb/asterix-common/src/main/java/org/apache/asterix/common/config/PropertiesAccessor.java
@@ -137,11 +137,10 @@
                 }
                 stores.put(store.getNcId(), nodeStores);
                 nodePartitionsMap.put(store.getNcId(), nodePartitions);
-                configManager.registerVirtualNode(store.getNcId());
                 // push the store info to the config manager
                 configManager.set(store.getNcId(), NCConfig.Option.IODEVICES, 
nodeStores);
                 // marking node as virtual, as we're not using NCServices with 
old-style config
-                configManager.set(store.getNcId(), NCConfig.Option.VIRTUAL_NC, 
true);
+                configManager.set(store.getNcId(), 
NCConfig.Option.NCSERVICE_PORT, NCConfig.NCSERVICE_PORT_DISABLED);
             }
             // Get extensions
             if (asterixConfiguration.getExtensions() != null) {
diff --git 
a/asterixdb/asterix-runtime/src/main/java/org/apache/asterix/runtime/utils/ClusterStateManager.java
 
b/asterixdb/asterix-runtime/src/main/java/org/apache/asterix/runtime/utils/ClusterStateManager.java
index e86aea1..9c96d32 100644
--- 
a/asterixdb/asterix-runtime/src/main/java/org/apache/asterix/runtime/utils/ClusterStateManager.java
+++ 
b/asterixdb/asterix-runtime/src/main/java/org/apache/asterix/runtime/utils/ClusterStateManager.java
@@ -375,11 +375,16 @@
     @Override
     public synchronized void deregisterNodePartitions(String nodeId) {
         ClusterPartition [] nodePartitions = node2PartitionsMap.remove(nodeId);
-        if (LOGGER.isLoggable(Level.INFO)) {
-            LOGGER.info("Deregistering node partitions for node " + nodeId + 
": " + Arrays.toString(nodePartitions));
-        }
-        for (ClusterPartition nodePartition : nodePartitions) {
-            clusterPartitions.remove(nodePartition.getPartitionId());
+        if (nodePartitions == null) {
+            LOGGER.info("deregisterNodePartitions unknown node " + nodeId + " 
(already removed?)");
+        } else {
+            if (LOGGER.isLoggable(Level.INFO)) {
+                LOGGER.info("deregisterNodePartitions for node " + nodeId + ": 
" +
+                        Arrays.toString(nodePartitions));
+            }
+            for (ClusterPartition nodePartition : nodePartitions) {
+                clusterPartitions.remove(nodePartition.getPartitionId());
+            }
         }
     }
 }
diff --git 
a/hyracks-fullstack/hyracks/hyracks-control/hyracks-control-cc/src/main/java/org/apache/hyracks/control/cc/ClusterControllerService.java
 
b/hyracks-fullstack/hyracks/hyracks-control/hyracks-control-cc/src/main/java/org/apache/hyracks/control/cc/ClusterControllerService.java
index 8722b92..bec0bfb 100644
--- 
a/hyracks-fullstack/hyracks/hyracks-control/hyracks-control-cc/src/main/java/org/apache/hyracks/control/cc/ClusterControllerService.java
+++ 
b/hyracks-fullstack/hyracks/hyracks-control/hyracks-control-cc/src/main/java/org/apache/hyracks/control/cc/ClusterControllerService.java
@@ -20,7 +20,6 @@
 
 import java.io.File;
 import java.io.FileReader;
-import java.io.IOException;
 import java.lang.reflect.Constructor;
 import java.lang.reflect.InvocationTargetException;
 import java.net.InetAddress;
@@ -229,11 +228,11 @@
         }
     }
 
-    private Map<String, Pair<String, Integer>> getNCServices() throws 
IOException {
+    private Map<String, Pair<String, Integer>> getNCServices() {
         Map<String, Pair<String, Integer>> ncMap = new TreeMap<>();
         for (String ncId : configManager.getNodeNames()) {
             IApplicationConfig ncConfig = 
configManager.getNodeEffectiveConfig(ncId);
-            if (!ncConfig.getBoolean(NCConfig.Option.VIRTUAL_NC)) {
+            if (ncConfig.getInt(NCConfig.Option.NCSERVICE_PORT) != 
NCConfig.NCSERVICE_PORT_DISABLED) {
                 ncMap.put(ncId, 
Pair.of(ncConfig.getString(NCConfig.Option.NCSERVICE_ADDRESS),
                         ncConfig.getInt(NCConfig.Option.NCSERVICE_PORT)));
             }
@@ -241,7 +240,7 @@
         return ncMap;
     }
 
-    private void connectNCs() throws IOException {
+    private void connectNCs() {
         getNCServices().entrySet().forEach(ncService -> {
             final TriggerNCWork triggerWork = new 
TriggerNCWork(ClusterControllerService.this,
                     ncService.getValue().getLeft(), 
ncService.getValue().getRight(), ncService.getKey());
diff --git 
a/hyracks-fullstack/hyracks/hyracks-control/hyracks-control-common/src/main/java/org/apache/hyracks/control/common/config/ConfigManager.java
 
b/hyracks-fullstack/hyracks/hyracks-control/hyracks-control-common/src/main/java/org/apache/hyracks/control/common/config/ConfigManager.java
index 2a83bb7..fcaee6d 100644
--- 
a/hyracks-fullstack/hyracks/hyracks-control/hyracks-control-common/src/main/java/org/apache/hyracks/control/common/config/ConfigManager.java
+++ 
b/hyracks-fullstack/hyracks/hyracks-control/hyracks-control-common/src/main/java/org/apache/hyracks/control/common/config/ConfigManager.java
@@ -159,8 +159,8 @@
                 : nodeSpecificMap.computeIfAbsent(node, 
this::createNodeSpecificMap);
     }
 
-    public void registerVirtualNode(String nodeId) {
-        LOGGER.fine("registerVirtualNode: " + nodeId);
+    public void ensureNode(String nodeId) {
+        LOGGER.fine("ensureNode: " + nodeId);
         nodeSpecificMap.computeIfAbsent(nodeId, this::createNodeSpecificMap);
     }
 
diff --git 
a/hyracks-fullstack/hyracks/hyracks-control/hyracks-control-common/src/main/java/org/apache/hyracks/control/common/controllers/NCConfig.java
 
b/hyracks-fullstack/hyracks/hyracks-control/hyracks-control-common/src/main/java/org/apache/hyracks/control/common/controllers/NCConfig.java
index 86cc19e..42726a7 100644
--- 
a/hyracks-fullstack/hyracks/hyracks-control/hyracks-control-common/src/main/java/org/apache/hyracks/control/common/controllers/NCConfig.java
+++ 
b/hyracks-fullstack/hyracks/hyracks-control/hyracks-control-common/src/main/java/org/apache/hyracks/control/common/controllers/NCConfig.java
@@ -18,7 +18,6 @@
  */
 package org.apache.hyracks.control.common.controllers;
 
-import static org.apache.hyracks.control.common.config.OptionTypes.BOOLEAN;
 import static org.apache.hyracks.control.common.config.OptionTypes.INTEGER;
 import static 
org.apache.hyracks.control.common.config.OptionTypes.INTEGER_BYTE_UNIT;
 import static org.apache.hyracks.control.common.config.OptionTypes.LONG;
@@ -77,8 +76,7 @@
         APP_CLASS(STRING, (String) null),
         NCSERVICE_PID(INTEGER, -1),
         COMMAND(STRING, "hyracksnc"),
-        JVM_ARGS(STRING, (String) null),
-        VIRTUAL_NC(BOOLEAN, false);
+        JVM_ARGS(STRING, (String) null);
 
         private final IOptionType parser;
         private final String defaultValueDescription;
@@ -127,7 +125,8 @@
                 case NCSERVICE_ADDRESS:
                     return "Address the CC should use to contact the NCService 
associated with this NC";
                 case NCSERVICE_PORT:
-                    return "Port the CC should use to contact the NCService 
associated with this NC";
+                    return "Port the CC should use to contact the NCService 
associated with this NC (-1 to not use " +
+                            "NCService to start this NC)";
                 case CLUSTER_ADDRESS:
                     return "Cluster Controller address (required unless 
specified in config file)";
                 case CLUSTER_PORT:
@@ -189,8 +188,6 @@
                     return "Command NCService should invoke to start the 
NCDriver";
                 case JVM_ARGS:
                     return "JVM args to pass to the NCDriver";
-                case VIRTUAL_NC:
-                    return "A flag indicating if this NC is running on virtual 
cluster";
                 default:
                     throw new IllegalStateException("NYI: " + this);
             }
@@ -211,16 +208,13 @@
         }
 
         @Override
-        public boolean hidden() {
-            return this == VIRTUAL_NC;
-        }
-
-        @Override
         public String usageDefaultOverride(IApplicationConfig accessor, 
Function<IOption, String> optionPrinter) {
             return defaultValueDescription;
         }
 
     }
+
+    public static final int NCSERVICE_PORT_DISABLED = -1;
 
     private List<String> appArgs = new ArrayList<>();
 
@@ -504,11 +498,11 @@
         configManager.set(nodeId, Option.NCSERVICE_PID, ncservicePid);
     }
 
-    public boolean getVirtualNC() {
-        return appConfig.getBoolean(Option.VIRTUAL_NC);
+    public boolean isVirtualNC() {
+        return appConfig.getInt(NCConfig.Option.NCSERVICE_PORT) == 
NCConfig.NCSERVICE_PORT_DISABLED;
     }
 
-    public void setVirtualNC(boolean virtualNC) {
-        configManager.set(nodeId, Option.VIRTUAL_NC, virtualNC);
+    public void setVirtualNC() {
+        configManager.set(nodeId, Option.NCSERVICE_PORT, 
NCSERVICE_PORT_DISABLED);
     }
 }

-- 
To view, visit https://asterix-gerrit.ics.uci.edu/1820
To unsubscribe, visit https://asterix-gerrit.ics.uci.edu/settings

Gerrit-MessageType: newchange
Gerrit-Change-Id: I67a9a88808a3d1352b5fdd45ebd158e98dc72dba
Gerrit-PatchSet: 1
Gerrit-Project: asterixdb
Gerrit-Branch: master
Gerrit-Owner: Michael Blow <[email protected]>

Reply via email to