[ 
https://issues.apache.org/jira/browse/HDFS-6345?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13996783#comment-13996783
 ] 

Colin Patrick McCabe commented on HDFS-6345:
--------------------------------------------

{code}
+    // Querying for a single ID
+    final Long id = filter.getId();
+    if (id != null) {
+      if (!directivesById.containsKey(id)) {
+        throw new InvalidRequestException("Did not find requested id " + id);
+      }
+      prevId = id - 1;
     }
{code}

This is a bit tricky, so I think we should add a comment to the code explaining 
why we're changing prevId.

{code}
+
     ArrayList<CacheDirectiveEntry> replies =
         new ArrayList<CacheDirectiveEntry>(NUM_PRE_ALLOCATED_ENTRIES);
     int numReplies = 0;
@@ -710,6 +718,11 @@ public void removeDirective(long id, FSPermissionChecker 
pc)
       }
       CacheDirective curDirective = cur.getValue();
       CacheDirectiveInfo info = cur.getValue().toInfo();
+      if (id != null &&
+          !(info.getId().equals(id))) {
+        // If we didn't find the requested ID, we're done
+        break;
+      }
{code}

Maybe change the comment to "if we didn't find, or filtered out the requested 
ID".  Filtering by ID is only one kind of filtering... if an additional filter 
is set which eliminates the requested item, nothing will show up.

> DFS.listCacheDirectives() should allow filtering based on cache directive ID
> ----------------------------------------------------------------------------
>
>                 Key: HDFS-6345
>                 URL: https://issues.apache.org/jira/browse/HDFS-6345
>             Project: Hadoop HDFS
>          Issue Type: Bug
>          Components: caching
>    Affects Versions: 2.4.0
>            Reporter: Lenni Kuff
>            Assignee: Andrew Wang
>         Attachments: hdfs-6345-1.patch
>
>
> DFS.listCacheDirectives() should allow filtering based on cache directive ID. 
> Currently it throws an exception.
> For example:
> {code}
> long directiveId = <some id value>;
> CacheDirectiveInfo filter = new CacheDirectiveInfo.Builder()  
>     .setId(directiveId)
>     .build();
> RemoteIterator<CacheDirectiveEntry> itr = dfs.listCacheDirectives(filter);
> {code}



--
This message was sent by Atlassian JIRA
(v6.2#6252)

Reply via email to