[jira] [Commented] (HDFS-10480) Add an admin command to list currently open files

[Yiqun Lin (JIRA)]

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

Yiqun Lin commented on HDFS-10480:
----------------------------------

Thanks [~manojg] for helping filling JIRAs. 
I take some time to review the unit tests in the patch. I have some minor 
comments:

1.In {{TestDFSAdmin#verifyOpenFilesListing}},  line 676 {{LOG.info(out);}}, can 
we add one line like {{LOG.info("Open files: ")}} before this? it will help us 
understand what are printed in the following lines.
2.In {{TestDFSAdmin#verifyOpenFilesListing}}, line 677, the name 
{{openFilePath}} should be {{closedFilePath}}.
3.In {{TestListOpenFiles#verifyOpenFiles}}, line 145, {{"open files not 
listed!"}} should be {{" open files not listed!"}}. There is one space missing.
4. Can you update the param name {{count}} to {{numFilesToCreate}} in method 
{{createOpenFiles(Path parentDir, String fileNamePrefix, int count)}}? The same 
to {{closeFiles}} method, we can use {{numFilesToClose}}. It will be better to 
understand.
5.The operation {{count--}} is missing in the following codes.
{code}
  private void closeFiles(Map<String, FSDataOutputStream> openFiles,
      int count) throws IOException {
    for (Iterator<Entry<String, FSDataOutputStream>> it =
         openFiles.entrySet().iterator(); it.hasNext(); ) {
      Entry<String, FSDataOutputStream> entry = it.next();
      entry.getValue().close();
      LOG.info("Closed file: " + entry.getKey());
      it.remove();              <===== One line missing here: count--
      if (count == 0) {
        break;
      }
    }
  }
{code}
6. I am a little confused for the following codes:
{code}
  public void testListOpenFilesViaNameNodeRPC() throws Exception {
    Map<String, FSDataOutputStream> openFiles = new HashMap<>();
    Path baseDir = new Path("/testListOpenFiles");
    fs.mkdirs(baseDir);

    createFiles(baseDir, "closed", 10);
    verifyOpenFiles(openFiles);
    BatchedEntries<OpenFileEntry> openFileEntryBatchedEntries =
        nnRpc.listOpenFiles(0);
    assertTrue("Open files list should be empty!",
        openFileEntryBatchedEntries.size() == 0);

    openFiles.putAll(createOpenFiles(baseDir, "open-1", 1));
    verifyOpenFiles(openFiles);

    openFiles.putAll(createOpenFiles(baseDir, "open-2",
        (BATCH_SIZE * 2 + BATCH_SIZE / 2)));
    verifyOpenFiles(openFiles);

    closeFiles(openFiles, openFiles.size() / 2);
                                                          <==== here should do 
verifyOpenFiles(openFiles) since we have closed some files and should check the 
open files again. what are following lines intended to do? In addition, the 
file name prefix is same.
    openFiles.putAll(createOpenFiles(baseDir, "open-2",
        (BATCH_SIZE * 5)));                  
    verifyOpenFiles(openFiles);
  }
{code}

> Add an admin command to list currently open files
> -------------------------------------------------
>
>                 Key: HDFS-10480
>                 URL: https://issues.apache.org/jira/browse/HDFS-10480
>             Project: Hadoop HDFS
>          Issue Type: Improvement
>            Reporter: Kihwal Lee
>            Assignee: Manoj Govindassamy
>         Attachments: HDFS-10480.02.patch, HDFS-10480-trunk-1.patch, 
> HDFS-10480-trunk.patch
>
>
> Currently there is no easy way to obtain the list of active leases or files 
> being written. It will be nice if we have an admin command to list open files 
> and their lease holders.



--
This message was sent by Atlassian JIRA
(v6.3.15#6346)

---------------------------------------------------------------------
To unsubscribe, e-mail: hdfs-issues-unsubscr...@hadoop.apache.org
For additional commands, e-mail: hdfs-issues-h...@hadoop.apache.org