omalley commented on code in PR #4311:
URL: https://github.com/apache/hadoop/pull/4311#discussion_r957851531


##########
hadoop-hdfs-project/hadoop-hdfs-client/src/main/java/org/apache/hadoop/hdfs/ClientGSIContext.java:
##########
@@ -37,8 +37,17 @@
 @InterfaceStability.Evolving
 public class ClientGSIContext implements AlignmentContext {
 
-  private final LongAccumulator lastSeenStateId =
-      new LongAccumulator(Math::max, Long.MIN_VALUE);
+  private final NamespaceStateId lastSeenStateId;

Review Comment:
   I'd replace this with a straight long. We need synchronization for the 
routerFederatedState also, so we might as well use a lock for both of them.



##########
hadoop-hdfs-project/hadoop-hdfs-client/src/main/java/org/apache/hadoop/hdfs/NamespaceStateId.java:
##########
@@ -0,0 +1,42 @@
+/**

Review Comment:
   This class doesn't add much value. Do we actually need it?



##########
hadoop-hdfs-project/hadoop-hdfs-client/src/main/java/org/apache/hadoop/hdfs/ClientGSIContext.java:
##########
@@ -66,15 +75,24 @@ public void 
updateResponseState(RpcResponseHeaderProto.Builder header) {
    */
   @Override
   public void receiveResponseState(RpcResponseHeaderProto header) {
-    lastSeenStateId.accumulate(header.getStateId());
+    if (header.hasRouterFederatedState()) {
+      routerFederatedState = header.getRouterFederatedState();
+    } else {
+      lastSeenStateId.update(header.getStateId());
+    }
   }
 
   /**
    * Client side implementation for providing state alignment info in requests.
    */
   @Override
   public void updateRequestState(RpcRequestHeaderProto.Builder header) {

Review Comment:
   this one too



##########
hadoop-hdfs-project/hadoop-hdfs-rbf/src/main/java/org/apache/hadoop/hdfs/server/federation/router/FederatedNamespaceIds.java:
##########
@@ -0,0 +1,113 @@
+/**
+ * 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.hadoop.hdfs.server.federation.router;
+
+import java.util.Collections;
+import java.util.Map;
+import java.util.concurrent.ConcurrentHashMap;
+import java.util.concurrent.locks.ReentrantLock;
+import org.apache.hadoop.classification.VisibleForTesting;
+import org.apache.hadoop.hdfs.NamespaceStateId;
+import org.apache.hadoop.ipc.protobuf.RpcHeaderProtos;
+import 
org.apache.hadoop.hdfs.federation.protocol.proto.HdfsServerFederationProtos.RouterFederatedStateProto;
+import org.apache.hadoop.thirdparty.protobuf.InvalidProtocolBufferException;
+import org.apache.hadoop.thirdparty.protobuf.ByteString;
+
+
+/**
+ * Collection of last-seen namespace state Ids for a set of namespaces.
+ * A single NamespaceStateId is shared by all outgoing connections to a 
particular namespace.
+ * Router clients share and query the entire collection.
+ */
+public class FederatedNamespaceIds {
+  private final Map<String, NamespaceStateId> namespaceIdMap = new 
ConcurrentHashMap<>();
+  private final ReentrantLock lock = new ReentrantLock();
+
+  public void 
updateStateUsingRequestHeader(RpcHeaderProtos.RpcRequestHeaderProto header) {
+    if (header.hasRouterFederatedState()) {
+      RouterFederatedStateProto federatedState;
+      try {
+        federatedState = 
RouterFederatedStateProto.parseFrom(header.getRouterFederatedState());
+      } catch (InvalidProtocolBufferException e) {
+        throw new RuntimeException(e);
+      }
+      lock.lock();
+      try {
+        federatedState.getNamespaceStateIdsMap().forEach((nsId, stateId) -> {
+          if (!namespaceIdMap.containsKey(nsId)) {
+            namespaceIdMap.putIfAbsent(nsId, new NamespaceStateId());
+          }
+          namespaceIdMap.get(nsId).update(stateId);
+        });
+      } finally {
+        lock.unlock();
+      }
+
+    }
+  }
+
+  public void 
setResponseHeaderState(RpcHeaderProtos.RpcResponseHeaderProto.Builder 
headerBuilder) {
+    RouterFederatedStateProto.Builder federatedStateBuilder =
+        RouterFederatedStateProto.newBuilder();
+    lock.lock();
+    try {
+      namespaceIdMap.forEach((k, v) -> 
federatedStateBuilder.putNamespaceStateIds(k, v.get()));
+    } finally {
+      lock.unlock();
+    }
+    
headerBuilder.setRouterFederatedState(federatedStateBuilder.build().toByteString());
+  }
+
+  public NamespaceStateId getNamespaceId(String nsId) {
+    lock.lock();
+    try {
+      namespaceIdMap.putIfAbsent(nsId, new NamespaceStateId());

Review Comment:
   This is another place where we should use computeIfAbsent.



##########
hadoop-hdfs-project/hadoop-hdfs-rbf/src/main/java/org/apache/hadoop/hdfs/server/federation/router/FederatedNamespaceIds.java:
##########
@@ -0,0 +1,113 @@
+/**
+ * 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.hadoop.hdfs.server.federation.router;
+
+import java.util.Collections;
+import java.util.Map;
+import java.util.concurrent.ConcurrentHashMap;
+import java.util.concurrent.locks.ReentrantLock;
+import org.apache.hadoop.classification.VisibleForTesting;
+import org.apache.hadoop.hdfs.NamespaceStateId;
+import org.apache.hadoop.ipc.protobuf.RpcHeaderProtos;
+import 
org.apache.hadoop.hdfs.federation.protocol.proto.HdfsServerFederationProtos.RouterFederatedStateProto;
+import org.apache.hadoop.thirdparty.protobuf.InvalidProtocolBufferException;
+import org.apache.hadoop.thirdparty.protobuf.ByteString;
+
+
+/**
+ * Collection of last-seen namespace state Ids for a set of namespaces.
+ * A single NamespaceStateId is shared by all outgoing connections to a 
particular namespace.
+ * Router clients share and query the entire collection.
+ */
+public class FederatedNamespaceIds {
+  private final Map<String, NamespaceStateId> namespaceIdMap = new 
ConcurrentHashMap<>();
+  private final ReentrantLock lock = new ReentrantLock();
+
+  public void 
updateStateUsingRequestHeader(RpcHeaderProtos.RpcRequestHeaderProto header) {
+    if (header.hasRouterFederatedState()) {
+      RouterFederatedStateProto federatedState;
+      try {
+        federatedState = 
RouterFederatedStateProto.parseFrom(header.getRouterFederatedState());
+      } catch (InvalidProtocolBufferException e) {
+        throw new RuntimeException(e);
+      }
+      lock.lock();
+      try {
+        federatedState.getNamespaceStateIdsMap().forEach((nsId, stateId) -> {
+          if (!namespaceIdMap.containsKey(nsId)) {
+            namespaceIdMap.putIfAbsent(nsId, new NamespaceStateId());
+          }
+          namespaceIdMap.get(nsId).update(stateId);
+        });
+      } finally {
+        lock.unlock();
+      }
+
+    }
+  }
+
+  public void 
setResponseHeaderState(RpcHeaderProtos.RpcResponseHeaderProto.Builder 
headerBuilder) {
+    RouterFederatedStateProto.Builder federatedStateBuilder =
+        RouterFederatedStateProto.newBuilder();
+    lock.lock();
+    try {
+      namespaceIdMap.forEach((k, v) -> 
federatedStateBuilder.putNamespaceStateIds(k, v.get()));
+    } finally {
+      lock.unlock();
+    }
+    
headerBuilder.setRouterFederatedState(federatedStateBuilder.build().toByteString());
+  }
+
+  public NamespaceStateId getNamespaceId(String nsId) {
+    lock.lock();
+    try {
+      namespaceIdMap.putIfAbsent(nsId, new NamespaceStateId());
+    } finally {
+      lock.unlock();
+    }
+    return namespaceIdMap.get(nsId);
+  }
+
+  public void removeNamespaceId(String nsId) {
+    lock.lock();
+    try {
+      namespaceIdMap.remove(nsId);
+    } finally {
+      lock.unlock();
+    }
+  }
+
+  /**
+   * Utility function to view state of routerFederatedState field in RPC 
headers.
+   */
+  @VisibleForTesting
+  public static Map<String, Long> getRouterFederatedStateMap(ByteString 
byteString) {

Review Comment:
   Since this is just used for testing, could we move it to one of the test 
classes?



##########
hadoop-hdfs-project/hadoop-hdfs-client/src/main/java/org/apache/hadoop/hdfs/ClientGSIContext.java:
##########
@@ -37,8 +37,17 @@
 @InterfaceStability.Evolving
 public class ClientGSIContext implements AlignmentContext {
 
-  private final LongAccumulator lastSeenStateId =
-      new LongAccumulator(Math::max, Long.MIN_VALUE);
+  private final NamespaceStateId lastSeenStateId;

Review Comment:
   You will also need to add a Math.max to the update.



##########
hadoop-hdfs-project/hadoop-hdfs-rbf/src/main/java/org/apache/hadoop/hdfs/server/federation/router/FederatedNamespaceIds.java:
##########
@@ -0,0 +1,113 @@
+/**
+ * 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.hadoop.hdfs.server.federation.router;
+
+import java.util.Collections;
+import java.util.Map;
+import java.util.concurrent.ConcurrentHashMap;
+import java.util.concurrent.locks.ReentrantLock;
+import org.apache.hadoop.classification.VisibleForTesting;
+import org.apache.hadoop.hdfs.NamespaceStateId;
+import org.apache.hadoop.ipc.protobuf.RpcHeaderProtos;
+import 
org.apache.hadoop.hdfs.federation.protocol.proto.HdfsServerFederationProtos.RouterFederatedStateProto;
+import org.apache.hadoop.thirdparty.protobuf.InvalidProtocolBufferException;
+import org.apache.hadoop.thirdparty.protobuf.ByteString;
+
+
+/**
+ * Collection of last-seen namespace state Ids for a set of namespaces.
+ * A single NamespaceStateId is shared by all outgoing connections to a 
particular namespace.
+ * Router clients share and query the entire collection.
+ */
+public class FederatedNamespaceIds {
+  private final Map<String, NamespaceStateId> namespaceIdMap = new 
ConcurrentHashMap<>();
+  private final ReentrantLock lock = new ReentrantLock();
+
+  public void 
updateStateUsingRequestHeader(RpcHeaderProtos.RpcRequestHeaderProto header) {
+    if (header.hasRouterFederatedState()) {
+      RouterFederatedStateProto federatedState;
+      try {
+        federatedState = 
RouterFederatedStateProto.parseFrom(header.getRouterFederatedState());
+      } catch (InvalidProtocolBufferException e) {
+        throw new RuntimeException(e);
+      }
+      lock.lock();
+      try {
+        federatedState.getNamespaceStateIdsMap().forEach((nsId, stateId) -> {
+          if (!namespaceIdMap.containsKey(nsId)) {
+            namespaceIdMap.putIfAbsent(nsId, new NamespaceStateId());
+          }
+          namespaceIdMap.get(nsId).update(stateId);
+        });

Review Comment:
   One other problem with this, is that since values from the clients are 
updating the global map, it would be easy for a malicious client to poison a 
router with a high value that would effectively disable a namespace for that 
router.



##########
hadoop-hdfs-project/hadoop-hdfs-client/src/main/java/org/apache/hadoop/hdfs/ClientGSIContext.java:
##########
@@ -66,15 +75,24 @@ public void 
updateResponseState(RpcResponseHeaderProto.Builder header) {
    */
   @Override
   public void receiveResponseState(RpcResponseHeaderProto header) {

Review Comment:
   this method needs to be synchronized



##########
hadoop-hdfs-project/hadoop-hdfs-rbf/src/main/java/org/apache/hadoop/hdfs/server/federation/router/FederatedNamespaceIds.java:
##########
@@ -0,0 +1,113 @@
+/**
+ * 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.hadoop.hdfs.server.federation.router;
+
+import java.util.Collections;
+import java.util.Map;
+import java.util.concurrent.ConcurrentHashMap;
+import java.util.concurrent.locks.ReentrantLock;
+import org.apache.hadoop.classification.VisibleForTesting;
+import org.apache.hadoop.hdfs.NamespaceStateId;
+import org.apache.hadoop.ipc.protobuf.RpcHeaderProtos;
+import 
org.apache.hadoop.hdfs.federation.protocol.proto.HdfsServerFederationProtos.RouterFederatedStateProto;
+import org.apache.hadoop.thirdparty.protobuf.InvalidProtocolBufferException;
+import org.apache.hadoop.thirdparty.protobuf.ByteString;
+
+
+/**
+ * Collection of last-seen namespace state Ids for a set of namespaces.
+ * A single NamespaceStateId is shared by all outgoing connections to a 
particular namespace.
+ * Router clients share and query the entire collection.
+ */
+public class FederatedNamespaceIds {
+  private final Map<String, NamespaceStateId> namespaceIdMap = new 
ConcurrentHashMap<>();
+  private final ReentrantLock lock = new ReentrantLock();

Review Comment:
   With the concurrenthashmap, I don't think we need the explicit lock.



##########
hadoop-hdfs-project/hadoop-hdfs-rbf/src/main/java/org/apache/hadoop/hdfs/server/federation/router/FederatedNamespaceIds.java:
##########
@@ -0,0 +1,113 @@
+/**
+ * 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.hadoop.hdfs.server.federation.router;
+
+import java.util.Collections;
+import java.util.Map;
+import java.util.concurrent.ConcurrentHashMap;
+import java.util.concurrent.locks.ReentrantLock;
+import org.apache.hadoop.classification.VisibleForTesting;
+import org.apache.hadoop.hdfs.NamespaceStateId;
+import org.apache.hadoop.ipc.protobuf.RpcHeaderProtos;
+import 
org.apache.hadoop.hdfs.federation.protocol.proto.HdfsServerFederationProtos.RouterFederatedStateProto;
+import org.apache.hadoop.thirdparty.protobuf.InvalidProtocolBufferException;
+import org.apache.hadoop.thirdparty.protobuf.ByteString;
+
+
+/**
+ * Collection of last-seen namespace state Ids for a set of namespaces.
+ * A single NamespaceStateId is shared by all outgoing connections to a 
particular namespace.
+ * Router clients share and query the entire collection.
+ */
+public class FederatedNamespaceIds {
+  private final Map<String, NamespaceStateId> namespaceIdMap = new 
ConcurrentHashMap<>();
+  private final ReentrantLock lock = new ReentrantLock();
+
+  public void 
updateStateUsingRequestHeader(RpcHeaderProtos.RpcRequestHeaderProto header) {
+    if (header.hasRouterFederatedState()) {
+      RouterFederatedStateProto federatedState;
+      try {
+        federatedState = 
RouterFederatedStateProto.parseFrom(header.getRouterFederatedState());
+      } catch (InvalidProtocolBufferException e) {
+        throw new RuntimeException(e);
+      }
+      lock.lock();
+      try {
+        federatedState.getNamespaceStateIdsMap().forEach((nsId, stateId) -> {
+          if (!namespaceIdMap.containsKey(nsId)) {
+            namespaceIdMap.putIfAbsent(nsId, new NamespaceStateId());
+          }
+          namespaceIdMap.get(nsId).update(stateId);

Review Comment:
   We should use namespaceIdMap.computeIfAbsent(...) instead of get and make 
the new NamespaceStateId as a lambda.



##########
hadoop-hdfs-project/hadoop-hdfs-rbf/src/main/java/org/apache/hadoop/hdfs/server/federation/router/FederatedNamespaceIds.java:
##########
@@ -0,0 +1,113 @@
+/**
+ * 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.hadoop.hdfs.server.federation.router;
+
+import java.util.Collections;
+import java.util.Map;
+import java.util.concurrent.ConcurrentHashMap;
+import java.util.concurrent.locks.ReentrantLock;
+import org.apache.hadoop.classification.VisibleForTesting;
+import org.apache.hadoop.hdfs.NamespaceStateId;
+import org.apache.hadoop.ipc.protobuf.RpcHeaderProtos;
+import 
org.apache.hadoop.hdfs.federation.protocol.proto.HdfsServerFederationProtos.RouterFederatedStateProto;
+import org.apache.hadoop.thirdparty.protobuf.InvalidProtocolBufferException;
+import org.apache.hadoop.thirdparty.protobuf.ByteString;
+
+
+/**
+ * Collection of last-seen namespace state Ids for a set of namespaces.
+ * A single NamespaceStateId is shared by all outgoing connections to a 
particular namespace.
+ * Router clients share and query the entire collection.
+ */
+public class FederatedNamespaceIds {
+  private final Map<String, NamespaceStateId> namespaceIdMap = new 
ConcurrentHashMap<>();
+  private final ReentrantLock lock = new ReentrantLock();
+
+  public void 
updateStateUsingRequestHeader(RpcHeaderProtos.RpcRequestHeaderProto header) {
+    if (header.hasRouterFederatedState()) {
+      RouterFederatedStateProto federatedState;
+      try {
+        federatedState = 
RouterFederatedStateProto.parseFrom(header.getRouterFederatedState());
+      } catch (InvalidProtocolBufferException e) {
+        throw new RuntimeException(e);
+      }
+      lock.lock();
+      try {
+        federatedState.getNamespaceStateIdsMap().forEach((nsId, stateId) -> {
+          if (!namespaceIdMap.containsKey(nsId)) {
+            namespaceIdMap.putIfAbsent(nsId, new NamespaceStateId());
+          }
+          namespaceIdMap.get(nsId).update(stateId);
+        });

Review Comment:
   To transfer the information, we could probably use a thread local to pass 
the value to send.



##########
hadoop-hdfs-project/hadoop-hdfs-rbf/src/main/java/org/apache/hadoop/hdfs/server/federation/router/FederatedNamespaceIds.java:
##########
@@ -0,0 +1,113 @@
+/**
+ * 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.hadoop.hdfs.server.federation.router;
+
+import java.util.Collections;
+import java.util.Map;
+import java.util.concurrent.ConcurrentHashMap;
+import java.util.concurrent.locks.ReentrantLock;
+import org.apache.hadoop.classification.VisibleForTesting;
+import org.apache.hadoop.hdfs.NamespaceStateId;
+import org.apache.hadoop.ipc.protobuf.RpcHeaderProtos;
+import 
org.apache.hadoop.hdfs.federation.protocol.proto.HdfsServerFederationProtos.RouterFederatedStateProto;
+import org.apache.hadoop.thirdparty.protobuf.InvalidProtocolBufferException;
+import org.apache.hadoop.thirdparty.protobuf.ByteString;
+
+
+/**
+ * Collection of last-seen namespace state Ids for a set of namespaces.
+ * A single NamespaceStateId is shared by all outgoing connections to a 
particular namespace.
+ * Router clients share and query the entire collection.
+ */
+public class FederatedNamespaceIds {
+  private final Map<String, NamespaceStateId> namespaceIdMap = new 
ConcurrentHashMap<>();
+  private final ReentrantLock lock = new ReentrantLock();
+
+  public void 
updateStateUsingRequestHeader(RpcHeaderProtos.RpcRequestHeaderProto header) {
+    if (header.hasRouterFederatedState()) {
+      RouterFederatedStateProto federatedState;
+      try {
+        federatedState = 
RouterFederatedStateProto.parseFrom(header.getRouterFederatedState());
+      } catch (InvalidProtocolBufferException e) {
+        throw new RuntimeException(e);
+      }
+      lock.lock();
+      try {
+        federatedState.getNamespaceStateIdsMap().forEach((nsId, stateId) -> {
+          if (!namespaceIdMap.containsKey(nsId)) {
+            namespaceIdMap.putIfAbsent(nsId, new NamespaceStateId());
+          }
+          namespaceIdMap.get(nsId).update(stateId);
+        });

Review Comment:
   I guess one approach would be:
   
   - values from the NN update the map.
   - we compute the value to send to the NN with the larger of the value from 
the client and the global map.
   
   Thoughts?



##########
hadoop-hdfs-project/hadoop-hdfs-rbf/src/main/java/org/apache/hadoop/hdfs/server/federation/router/RouterStateIdContext.java:
##########
@@ -0,0 +1,94 @@
+/**
+ * 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.hadoop.hdfs.server.federation.router;
+
+import java.lang.reflect.Method;
+import java.util.HashSet;
+
+import org.apache.hadoop.classification.InterfaceAudience;
+import org.apache.hadoop.classification.InterfaceStability;
+import org.apache.hadoop.hdfs.protocol.ClientProtocol;
+import org.apache.hadoop.hdfs.server.namenode.ha.ReadOnly;
+import org.apache.hadoop.ipc.AlignmentContext;
+import org.apache.hadoop.ipc.RetriableException;
+import org.apache.hadoop.ipc.protobuf.RpcHeaderProtos.RpcRequestHeaderProto;
+import org.apache.hadoop.ipc.protobuf.RpcHeaderProtos.RpcResponseHeaderProto;
+
+import static 
org.apache.hadoop.ipc.RpcConstants.REQUEST_HEADER_NAMESPACE_STATEIDS_SET;
+
+/**
+ * This is the router implementation responsible for passing
+ * client state id to next level.

Review Comment:
   I assume this is the one that is limited to a single namespace?



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]


---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]

Reply via email to