Michael Blow has submitted this change and it was merged.

Change subject: [NO ISSUE] Make NodeManager.remove() idempotent
......................................................................


[NO ISSUE] Make NodeManager.remove() idempotent

Change-Id: I0e2b14526be49c9817c0d64836fcf57647ac59b2
Reviewed-on: https://asterix-gerrit.ics.uci.edu/2414
Reviewed-by: Murtadha Hubail <mhub...@apache.org>
Integration-Tests: Michael Blow <mb...@apache.org>
Tested-by: Michael Blow <mb...@apache.org>
Contrib: Jenkins <jenk...@fulliautomatix.ics.uci.edu>
Tested-by: Jenkins <jenk...@fulliautomatix.ics.uci.edu>
---
M 
hyracks-fullstack/hyracks/hyracks-control/hyracks-control-cc/src/main/java/org/apache/hyracks/control/cc/cluster/INodeManager.java
M 
hyracks-fullstack/hyracks/hyracks-control/hyracks-control-cc/src/main/java/org/apache/hyracks/control/cc/cluster/NodeManager.java
A 
hyracks-fullstack/hyracks/hyracks-util/src/main/java/org/apache/hyracks/util/annotations/Idempotent.java
3 files changed, 45 insertions(+), 4 deletions(-)

Approvals:
  Jenkins: Verified; 
  Michael Blow: Verified; Verified
  Murtadha Hubail: Looks good to me, approved



diff --git 
a/hyracks-fullstack/hyracks/hyracks-control/hyracks-control-cc/src/main/java/org/apache/hyracks/control/cc/cluster/INodeManager.java
 
b/hyracks-fullstack/hyracks/hyracks-control/hyracks-control-cc/src/main/java/org/apache/hyracks/control/cc/cluster/INodeManager.java
index 40d81f8..eb54cc3 100644
--- 
a/hyracks-fullstack/hyracks/hyracks-control/hyracks-control-cc/src/main/java/org/apache/hyracks/control/cc/cluster/INodeManager.java
+++ 
b/hyracks-fullstack/hyracks/hyracks-control/hyracks-control-cc/src/main/java/org/apache/hyracks/control/cc/cluster/INodeManager.java
@@ -29,6 +29,7 @@
 import org.apache.hyracks.api.exceptions.HyracksException;
 import org.apache.hyracks.api.job.JobId;
 import org.apache.hyracks.control.cc.NodeControllerState;
+import org.apache.hyracks.util.annotations.Idempotent;
 
 /**
  * This interface provides abstractions for a node manager, which manages the 
node membership in a cluster.
@@ -102,13 +103,14 @@
     void addNode(String nodeId, NodeControllerState ncState) throws 
HyracksException;
 
     /**
-     * Removes one node from the cluster.
+     * Removes one node from the cluster.  This method is idempotent.
      *
      * @param nodeId,
      *            the node id.
      * @throws HyracksException
      *             when the IP address given in the node state is not valid
      */
+    @Idempotent
     void removeNode(String nodeId) throws HyracksException;
 
 }
diff --git 
a/hyracks-fullstack/hyracks/hyracks-control/hyracks-control-cc/src/main/java/org/apache/hyracks/control/cc/cluster/NodeManager.java
 
b/hyracks-fullstack/hyracks/hyracks-control/hyracks-control-cc/src/main/java/org/apache/hyracks/control/cc/cluster/NodeManager.java
index 77e7bf7..8246cad 100644
--- 
a/hyracks-fullstack/hyracks/hyracks-control/hyracks-control-cc/src/main/java/org/apache/hyracks/control/cc/cluster/NodeManager.java
+++ 
b/hyracks-fullstack/hyracks/hyracks-control/hyracks-control-cc/src/main/java/org/apache/hyracks/control/cc/cluster/NodeManager.java
@@ -48,6 +48,7 @@
 import org.apache.hyracks.control.common.ipc.CCNCFunctions.AbortCCJobsFunction;
 import org.apache.hyracks.ipc.api.IIPCHandle;
 import org.apache.hyracks.ipc.exceptions.IPCException;
+import org.apache.hyracks.util.annotations.Idempotent;
 import org.apache.hyracks.util.annotations.NotThreadSafe;
 import org.apache.logging.log4j.LogManager;
 import org.apache.logging.log4j.Logger;
@@ -129,11 +130,15 @@
     }
 
     @Override
+    @Idempotent
     public void removeNode(String nodeId) throws HyracksException {
         NodeControllerState ncState = nodeRegistry.remove(nodeId);
-        removeNodeFromIpAddressMap(nodeId, ncState);
-
-        // Updates the cluster capacity.
+        if (ncState == null) {
+            LOGGER.warn("request to remove unknown node {}; ignoring", nodeId);
+        } else {
+            removeNodeFromIpAddressMap(nodeId, ncState);
+        }
+        // Updates the cluster capacity (idempotent)
         resourceManager.update(nodeId, new NodeCapacity(0L, 0));
     }
 
diff --git 
a/hyracks-fullstack/hyracks/hyracks-util/src/main/java/org/apache/hyracks/util/annotations/Idempotent.java
 
b/hyracks-fullstack/hyracks/hyracks-util/src/main/java/org/apache/hyracks/util/annotations/Idempotent.java
new file mode 100644
index 0000000..21e28ba
--- /dev/null
+++ 
b/hyracks-fullstack/hyracks/hyracks-util/src/main/java/org/apache/hyracks/util/annotations/Idempotent.java
@@ -0,0 +1,34 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied.  See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.apache.hyracks.util.annotations;
+
+import java.lang.annotation.Documented;
+import java.lang.annotation.ElementType;
+import java.lang.annotation.Retention;
+import java.lang.annotation.RetentionPolicy;
+import java.lang.annotation.Target;
+
+/**
+ * The method on which this annotation is applied is idempotent.
+ */
+@Documented
+@Target({ ElementType.METHOD })
+@Retention(RetentionPolicy.SOURCE)
+public @interface Idempotent {
+}
\ No newline at end of file

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

Gerrit-MessageType: merged
Gerrit-Change-Id: I0e2b14526be49c9817c0d64836fcf57647ac59b2
Gerrit-PatchSet: 2
Gerrit-Project: asterixdb
Gerrit-Branch: master
Gerrit-Owner: Michael Blow <mb...@apache.org>
Gerrit-Reviewer: Anon. E. Moose #1000171
Gerrit-Reviewer: Jenkins <jenk...@fulliautomatix.ics.uci.edu>
Gerrit-Reviewer: Michael Blow <mb...@apache.org>
Gerrit-Reviewer: Murtadha Hubail <mhub...@apache.org>

Reply via email to