hadoop-yarn-issues mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Bikas Saha (JIRA)" <j...@apache.org>
Subject [jira] [Commented] (YARN-851) Share NMTokens using NMTokenCache (api-based) instead of memory based approach which is used currently.
Date Wed, 19 Jun 2013 17:42:22 GMT

    [ https://issues.apache.org/jira/browse/YARN-851?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13688199#comment-13688199

Bikas Saha commented on YARN-851:

This loop changes doesnt quite look the same as earlier. The old code is looping until the
token identifier matches. The token would get updated when the map would get updated. The
new code will also loop until the token identifier matches but the token object reference
is not being updated. So it the identifier does not match then the loop will not exit. Which
one is correct?
+    Token token = NMTokenCache.getNMToken(containerManagerBindAddr);
+    if (token == null) {
+      throw new InvalidToken("No NMToken sent for "
+          + containerManagerBindAddr);
+    }
     while (proxy != null
-        && !proxy.token.getIdentifier().equals(
-            nmTokens.get(containerManagerBindAddr).getIdentifier())) {
+        && !proxy.token.getIdentifier().equals(token.getIdentifier())) {

Why are tokens being removed from the TokenCache in the test?
-      nodeI = receivedNMTokens.keySet().iterator();
-      while (nodeI.hasNext()) {
-        nmTokens.remove(nodeI.next());
+        NMTokenCache.removeNMToken(nodeID);
+        receivedNMTokens.put(nodeID, token.getToken());

Overall comment of NMTokenCache is that its mixing up static and singleton for full functionality.
If it has an internal private single object then that object should take care of any required
synchronization etc and not the static class method. Alternatively, one can make the internal
map a private class object and not have an internal new NMTokenCache object. 
We might be better off using a concurrent map implementation instead of coarse synchronization
on the NMTokenCache method level.

Are these API meant to be public and for users? What scenario? Typo in comment.
+  /**
+   * Removes NMToken for specified node manger
+   * @param nodeAddr node address (host:port)
+   */
+  public static synchronized void removeNMToken(String nodeAddr) {
+    instance.nmTokens.remove(nodeAddr);
+  }
+  /**
+   * It will remove all the nm tokens from its cache
+   */
+  public static synchronized void clearCache() {
+    instance.nmTokens.clear();
+  }

Is this meant to be public for users? It should probably just return a java unmodifiable map
wrapped around the internal and mention in the java doc that it returns an unmodifiable map.
Unless its really required to create a snapshot of the token cache at that point in time.
+  /**
+   * It returns all NMTokens present in cache.
+   */
+  public static synchronized Map<String, Token> getAllNMTokens() {
+    Map<String, Token> nmTokens = new HashMap<String, Token>();
+    for (String key : instance.nmTokens.keySet()) {
+      nmTokens.put(key, instance.nmTokens.get(key));
+    }
+    return nmTokens;
+  }
> Share NMTokens using NMTokenCache (api-based) instead of memory based approach which
is used currently.
> -------------------------------------------------------------------------------------------------------
>                 Key: YARN-851
>                 URL: https://issues.apache.org/jira/browse/YARN-851
>             Project: Hadoop YARN
>          Issue Type: Bug
>            Reporter: Omkar Vinit Joshi
>            Assignee: Omkar Vinit Joshi
>         Attachments: YARN-851-20130618.patch
> It is a follow up ticket for YARN-694. Changing the way NMTokens are shared.

This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators
For more information on JIRA, see: http://www.atlassian.com/software/jira

View raw message