You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@ozone.apache.org by GitBox <gi...@apache.org> on 2020/02/22 07:47:01 UTC

[GitHub] [hadoop-ozone] smengcl opened a new pull request #584: HDDS-3054. OzoneFileStatus#getModificationTime should return actual directory modification time when its OmKeyInfo is available

smengcl opened a new pull request #584: HDDS-3054. OzoneFileStatus#getModificationTime should return actual directory modification time when its OmKeyInfo is available
URL: https://github.com/apache/hadoop-ozone/pull/584
 
 
   ## What changes were proposed in this pull request?
   
   As of current implementation, getModificationTime() always returns "fake" modification time (current time) for directory due to the reason that a directory in Ozone might be faked from a file key.
   
   But, there are cases where real directory key exists in OzoneBucket. For example when user calls fs.mkdirs(directory). In this case, a reasonable thing to do would be getting the modification time from the OmInfoKey, rather than faking it.
   
   ## What is the link to the Apache JIRA
   
   https://issues.apache.org/jira/browse/HDDS-3054
   
   ## How was this patch tested?
   
   So far passed manual test.
   
   Will add a integration test case later.

----------------------------------------------------------------
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.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

---------------------------------------------------------------------
To unsubscribe, e-mail: ozone-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: ozone-issues-help@hadoop.apache.org


[GitHub] [hadoop-ozone] hanishakoneru commented on a change in pull request #584: HDDS-3054. OzoneFileStatus#getModificationTime should return actual directory modification time when its OmKeyInfo is available

Posted by GitBox <gi...@apache.org>.
hanishakoneru commented on a change in pull request #584: HDDS-3054. OzoneFileStatus#getModificationTime should return actual directory modification time when its OmKeyInfo is available
URL: https://github.com/apache/hadoop-ozone/pull/584#discussion_r383538246
 
 

 ##########
 File path: hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/fs/ozone/TestOzoneFileSystem.java
 ##########
 @@ -354,4 +356,45 @@ private OzoneKeyDetails getKey(Path keyPath, boolean isDirectory)
   private void assertKeyNotFoundException(IOException ex) {
     GenericTestUtils.assertExceptionContains("KEY_NOT_FOUND", ex);
   }
+
+  private void testGetDirectoryModificationTime() throws IOException {
+    Path mdir1 = new Path("/mdir1");
+    Path mdir11 = new Path(mdir1, "mdir11");
+    Path mdir111 = new Path(mdir11, "mdir111");
+    fs.mkdirs(mdir111);
+    rootItemCount++; // mdir1
+
+    // Case 1: Dir key exist on server
+    FileStatus[] fileStatuses = o3fs.listStatus(mdir11);
+    // Above listStatus result should only have one entry: mdir111
+    assertEquals(1, fileStatuses.length);
+    assertEquals(mdir111.toString(),
+        fileStatuses[0].getPath().toUri().getPath());
+    assertTrue(fileStatuses[0].isDirectory());
+    // The dir key is actually created on server,
+    // so modification time should always be the same value.
+    long modificationTime = fileStatuses[0].getModificationTime();
+    // Check modification time in a small loop, it should always be the same
+    for (int i = 0; i < 5; i++) {
+      fileStatuses = o3fs.listStatus(mdir11);
+      assertEquals(modificationTime, fileStatuses[0].getModificationTime());
+    }
+
+    // Case 2: Dir key doesn't exist on server
+    fileStatuses = o3fs.listStatus(mdir1);
+    // Above listStatus result should only have one entry: mdir11
+    assertEquals(1, fileStatuses.length);
+    assertEquals(mdir11.toString(),
+        fileStatuses[0].getPath().toUri().getPath());
+    assertTrue(fileStatuses[0].isDirectory());
+    // Since the dir key doesn't exist on server, the modification time is
+    // set to current time upon every listStatus request.
+    modificationTime = fileStatuses[0].getModificationTime();
+    // Check modification time in a small loop, it should be slightly larger
+    // each time
+    for (int i = 0; i < 5; i++) {
+      fileStatuses = o3fs.listStatus(mdir1);
+      assertTrue(modificationTime <= fileStatuses[0].getModificationTime());
 
 Review comment:
   Let's add a sleep here too.

----------------------------------------------------------------
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.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

---------------------------------------------------------------------
To unsubscribe, e-mail: ozone-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: ozone-issues-help@hadoop.apache.org


[GitHub] [hadoop-ozone] hanishakoneru commented on a change in pull request #584: HDDS-3054. OzoneFileStatus#getModificationTime should return actual directory modification time when its OmKeyInfo is available

Posted by GitBox <gi...@apache.org>.
hanishakoneru commented on a change in pull request #584: HDDS-3054. OzoneFileStatus#getModificationTime should return actual directory modification time when its OmKeyInfo is available
URL: https://github.com/apache/hadoop-ozone/pull/584#discussion_r383537802
 
 

 ##########
 File path: hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/fs/ozone/TestOzoneFileSystem.java
 ##########
 @@ -354,4 +356,45 @@ private OzoneKeyDetails getKey(Path keyPath, boolean isDirectory)
   private void assertKeyNotFoundException(IOException ex) {
     GenericTestUtils.assertExceptionContains("KEY_NOT_FOUND", ex);
   }
+
+  private void testGetDirectoryModificationTime() throws IOException {
+    Path mdir1 = new Path("/mdir1");
+    Path mdir11 = new Path(mdir1, "mdir11");
+    Path mdir111 = new Path(mdir11, "mdir111");
+    fs.mkdirs(mdir111);
+    rootItemCount++; // mdir1
+
+    // Case 1: Dir key exist on server
+    FileStatus[] fileStatuses = o3fs.listStatus(mdir11);
+    // Above listStatus result should only have one entry: mdir111
+    assertEquals(1, fileStatuses.length);
+    assertEquals(mdir111.toString(),
+        fileStatuses[0].getPath().toUri().getPath());
+    assertTrue(fileStatuses[0].isDirectory());
+    // The dir key is actually created on server,
+    // so modification time should always be the same value.
+    long modificationTime = fileStatuses[0].getModificationTime();
+    // Check modification time in a small loop, it should always be the same
+    for (int i = 0; i < 5; i++) {
+      fileStatuses = o3fs.listStatus(mdir11);
+      assertEquals(modificationTime, fileStatuses[0].getModificationTime());
 
 Review comment:
   Can we add a small sleep (maybe 100-200 ms) between each check.

----------------------------------------------------------------
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.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

---------------------------------------------------------------------
To unsubscribe, e-mail: ozone-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: ozone-issues-help@hadoop.apache.org


[GitHub] [hadoop-ozone] smengcl commented on a change in pull request #584: HDDS-3054. OzoneFileStatus#getModificationTime should return actual directory modification time when its OmKeyInfo is available

Posted by GitBox <gi...@apache.org>.
smengcl commented on a change in pull request #584: HDDS-3054. OzoneFileStatus#getModificationTime should return actual directory modification time when its OmKeyInfo is available
URL: https://github.com/apache/hadoop-ozone/pull/584#discussion_r384707298
 
 

 ##########
 File path: hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/KeyManagerImpl.java
 ##########
 @@ -2005,15 +2016,21 @@ public OmKeyInfo lookupFile(OmKeyArgs args, String clientAddress)
               if (isFile) {
                 if (!deletedKeySet.contains(entryInDb)) {
                   cacheKeyMap.put(entryInDb,
-                      new OzoneFileStatus(value, scmBlockSize, !isFile));
+                      new OzoneFileStatus(omKeyInfo, scmBlockSize, !isFile));
                   countEntries++;
                 }
                 iterator.next();
               } else {
                 // if entry is a directory
                 if (!deletedKeySet.contains(entryInDb)) {
-                  cacheKeyMap.put(entryInDb,
-                      new OzoneFileStatus(immediateChild));
+                  if (!entryKeyName.equals(immediateChild)) {
+                    cacheKeyMap.put(entryInDb,
+                        new OzoneFileStatus(immediateChild));
+                  } else {
+                    // If entryKeyName matches dir name, we have the info
+                    cacheKeyMap.put(entryInDb,
+                        new OzoneFileStatus(omKeyInfo, 0, true));
 
 Review comment:
   > Could you please explain this change.
   
   `entryKeyName` is retrieved from a DB entry (actually exists), while `immediateChild` is generated using the information of parent dir and a key in it:
   ```java
   String immediateChild = OzoneFSUtils.getImmediateChild(entryKeyName, keyName);
   ```
   As seen in `getImmediateChild()`'s javadoc:
   ```java
     /**
      * The function returns immediate child of given ancestor in a particular
      * descendant. For example if ancestor is /a/b and descendant is /a/b/c/d/e
      * the function should return /a/b/c/. If the descendant itself is the
      * immediate child then it is returned as is without adding a trailing slash.
      * This is done to distinguish files from a directory as in ozone files do
      * not carry a trailing slash.
      */
     public static String getImmediateChild(String descendant, String ancestor) {
   ```
   
   Therefore, when the dir key actually exists (case 1 in my unit test), `entryKeyName` will be an actual dir key. In this case, `immediateChild` equals `entryKeyName`. Hence my logic here.

----------------------------------------------------------------
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.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

---------------------------------------------------------------------
To unsubscribe, e-mail: ozone-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: ozone-issues-help@hadoop.apache.org


[GitHub] [hadoop-ozone] smengcl commented on a change in pull request #584: HDDS-3054. OzoneFileStatus#getModificationTime should return actual directory modification time when its OmKeyInfo is available

Posted by GitBox <gi...@apache.org>.
smengcl commented on a change in pull request #584: HDDS-3054. OzoneFileStatus#getModificationTime should return actual directory modification time when its OmKeyInfo is available
URL: https://github.com/apache/hadoop-ozone/pull/584#discussion_r384706707
 
 

 ##########
 File path: hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/fs/ozone/TestOzoneFileSystem.java
 ##########
 @@ -354,4 +356,45 @@ private OzoneKeyDetails getKey(Path keyPath, boolean isDirectory)
   private void assertKeyNotFoundException(IOException ex) {
     GenericTestUtils.assertExceptionContains("KEY_NOT_FOUND", ex);
   }
+
+  private void testGetDirectoryModificationTime() throws IOException {
+    Path mdir1 = new Path("/mdir1");
+    Path mdir11 = new Path(mdir1, "mdir11");
+    Path mdir111 = new Path(mdir11, "mdir111");
+    fs.mkdirs(mdir111);
+    rootItemCount++; // mdir1
+
+    // Case 1: Dir key exist on server
+    FileStatus[] fileStatuses = o3fs.listStatus(mdir11);
+    // Above listStatus result should only have one entry: mdir111
+    assertEquals(1, fileStatuses.length);
+    assertEquals(mdir111.toString(),
+        fileStatuses[0].getPath().toUri().getPath());
+    assertTrue(fileStatuses[0].isDirectory());
+    // The dir key is actually created on server,
+    // so modification time should always be the same value.
+    long modificationTime = fileStatuses[0].getModificationTime();
+    // Check modification time in a small loop, it should always be the same
+    for (int i = 0; i < 5; i++) {
+      fileStatuses = o3fs.listStatus(mdir11);
+      assertEquals(modificationTime, fileStatuses[0].getModificationTime());
 
 Review comment:
   Hi @hanishakoneru , I think sleep is unnecessary since it always have ~4 ms difference between each check. Sleep would just slow down the whole test.

----------------------------------------------------------------
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.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

---------------------------------------------------------------------
To unsubscribe, e-mail: ozone-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: ozone-issues-help@hadoop.apache.org


[GitHub] [hadoop-ozone] hanishakoneru commented on a change in pull request #584: HDDS-3054. OzoneFileStatus#getModificationTime should return actual directory modification time when its OmKeyInfo is available

Posted by GitBox <gi...@apache.org>.
hanishakoneru commented on a change in pull request #584: HDDS-3054. OzoneFileStatus#getModificationTime should return actual directory modification time when its OmKeyInfo is available
URL: https://github.com/apache/hadoop-ozone/pull/584#discussion_r384034908
 
 

 ##########
 File path: hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/KeyManagerImpl.java
 ##########
 @@ -2005,15 +2016,21 @@ public OmKeyInfo lookupFile(OmKeyArgs args, String clientAddress)
               if (isFile) {
                 if (!deletedKeySet.contains(entryInDb)) {
                   cacheKeyMap.put(entryInDb,
-                      new OzoneFileStatus(value, scmBlockSize, !isFile));
+                      new OzoneFileStatus(omKeyInfo, scmBlockSize, !isFile));
                   countEntries++;
                 }
                 iterator.next();
               } else {
                 // if entry is a directory
                 if (!deletedKeySet.contains(entryInDb)) {
-                  cacheKeyMap.put(entryInDb,
-                      new OzoneFileStatus(immediateChild));
+                  if (!entryKeyName.equals(immediateChild)) {
+                    cacheKeyMap.put(entryInDb,
+                        new OzoneFileStatus(immediateChild));
+                  } else {
+                    // If entryKeyName matches dir name, we have the info
+                    cacheKeyMap.put(entryInDb,
+                        new OzoneFileStatus(omKeyInfo, 0, true));
 
 Review comment:
   Could you please explain this change.

----------------------------------------------------------------
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.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

---------------------------------------------------------------------
To unsubscribe, e-mail: ozone-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: ozone-issues-help@hadoop.apache.org


[GitHub] [hadoop-ozone] hanishakoneru commented on issue #584: HDDS-3054. OzoneFileStatus#getModificationTime should return actual directory modification time when its OmKeyInfo is available

Posted by GitBox <gi...@apache.org>.
hanishakoneru commented on issue #584: HDDS-3054. OzoneFileStatus#getModificationTime should return actual directory modification time when its OmKeyInfo is available
URL: https://github.com/apache/hadoop-ozone/pull/584#issuecomment-592741594
 
 
   LGTM. +1

----------------------------------------------------------------
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.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

---------------------------------------------------------------------
To unsubscribe, e-mail: ozone-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: ozone-issues-help@hadoop.apache.org


[GitHub] [hadoop-ozone] hanishakoneru merged pull request #584: HDDS-3054. OzoneFileStatus#getModificationTime should return actual directory modification time when its OmKeyInfo is available

Posted by GitBox <gi...@apache.org>.
hanishakoneru merged pull request #584: HDDS-3054. OzoneFileStatus#getModificationTime should return actual directory modification time when its OmKeyInfo is available
URL: https://github.com/apache/hadoop-ozone/pull/584
 
 
   

----------------------------------------------------------------
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.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

---------------------------------------------------------------------
To unsubscribe, e-mail: ozone-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: ozone-issues-help@hadoop.apache.org


[GitHub] [hadoop-ozone] smengcl commented on a change in pull request #584: HDDS-3054. OzoneFileStatus#getModificationTime should return actual directory modification time when its OmKeyInfo is available

Posted by GitBox <gi...@apache.org>.
smengcl commented on a change in pull request #584: HDDS-3054. OzoneFileStatus#getModificationTime should return actual directory modification time when its OmKeyInfo is available
URL: https://github.com/apache/hadoop-ozone/pull/584#discussion_r384707165
 
 

 ##########
 File path: hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/fs/ozone/TestOzoneFileSystem.java
 ##########
 @@ -354,4 +356,45 @@ private OzoneKeyDetails getKey(Path keyPath, boolean isDirectory)
   private void assertKeyNotFoundException(IOException ex) {
     GenericTestUtils.assertExceptionContains("KEY_NOT_FOUND", ex);
   }
+
+  private void testGetDirectoryModificationTime() throws IOException {
+    Path mdir1 = new Path("/mdir1");
+    Path mdir11 = new Path(mdir1, "mdir11");
+    Path mdir111 = new Path(mdir11, "mdir111");
+    fs.mkdirs(mdir111);
+    rootItemCount++; // mdir1
+
+    // Case 1: Dir key exist on server
+    FileStatus[] fileStatuses = o3fs.listStatus(mdir11);
+    // Above listStatus result should only have one entry: mdir111
+    assertEquals(1, fileStatuses.length);
+    assertEquals(mdir111.toString(),
+        fileStatuses[0].getPath().toUri().getPath());
+    assertTrue(fileStatuses[0].isDirectory());
+    // The dir key is actually created on server,
+    // so modification time should always be the same value.
+    long modificationTime = fileStatuses[0].getModificationTime();
+    // Check modification time in a small loop, it should always be the same
+    for (int i = 0; i < 5; i++) {
+      fileStatuses = o3fs.listStatus(mdir11);
+      assertEquals(modificationTime, fileStatuses[0].getModificationTime());
+    }
+
+    // Case 2: Dir key doesn't exist on server
+    fileStatuses = o3fs.listStatus(mdir1);
+    // Above listStatus result should only have one entry: mdir11
+    assertEquals(1, fileStatuses.length);
+    assertEquals(mdir11.toString(),
+        fileStatuses[0].getPath().toUri().getPath());
+    assertTrue(fileStatuses[0].isDirectory());
+    // Since the dir key doesn't exist on server, the modification time is
+    // set to current time upon every listStatus request.
+    modificationTime = fileStatuses[0].getModificationTime();
+    // Check modification time in a small loop, it should be slightly larger
+    // each time
+    for (int i = 0; i < 5; i++) {
+      fileStatuses = o3fs.listStatus(mdir1);
+      assertTrue(modificationTime <= fileStatuses[0].getModificationTime());
 
 Review comment:
   Checkstyle complains the listStatus exceeded 150 LOC and I have to make space for my change

----------------------------------------------------------------
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.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

---------------------------------------------------------------------
To unsubscribe, e-mail: ozone-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: ozone-issues-help@hadoop.apache.org


[GitHub] [hadoop-ozone] smengcl commented on a change in pull request #584: HDDS-3054. OzoneFileStatus#getModificationTime should return actual directory modification time when its OmKeyInfo is available

Posted by GitBox <gi...@apache.org>.
smengcl commented on a change in pull request #584: HDDS-3054. OzoneFileStatus#getModificationTime should return actual directory modification time when its OmKeyInfo is available
URL: https://github.com/apache/hadoop-ozone/pull/584#discussion_r384707165
 
 

 ##########
 File path: hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/fs/ozone/TestOzoneFileSystem.java
 ##########
 @@ -354,4 +356,45 @@ private OzoneKeyDetails getKey(Path keyPath, boolean isDirectory)
   private void assertKeyNotFoundException(IOException ex) {
     GenericTestUtils.assertExceptionContains("KEY_NOT_FOUND", ex);
   }
+
+  private void testGetDirectoryModificationTime() throws IOException {
+    Path mdir1 = new Path("/mdir1");
+    Path mdir11 = new Path(mdir1, "mdir11");
+    Path mdir111 = new Path(mdir11, "mdir111");
+    fs.mkdirs(mdir111);
+    rootItemCount++; // mdir1
+
+    // Case 1: Dir key exist on server
+    FileStatus[] fileStatuses = o3fs.listStatus(mdir11);
+    // Above listStatus result should only have one entry: mdir111
+    assertEquals(1, fileStatuses.length);
+    assertEquals(mdir111.toString(),
+        fileStatuses[0].getPath().toUri().getPath());
+    assertTrue(fileStatuses[0].isDirectory());
+    // The dir key is actually created on server,
+    // so modification time should always be the same value.
+    long modificationTime = fileStatuses[0].getModificationTime();
+    // Check modification time in a small loop, it should always be the same
+    for (int i = 0; i < 5; i++) {
+      fileStatuses = o3fs.listStatus(mdir11);
+      assertEquals(modificationTime, fileStatuses[0].getModificationTime());
+    }
+
+    // Case 2: Dir key doesn't exist on server
+    fileStatuses = o3fs.listStatus(mdir1);
+    // Above listStatus result should only have one entry: mdir11
+    assertEquals(1, fileStatuses.length);
+    assertEquals(mdir11.toString(),
+        fileStatuses[0].getPath().toUri().getPath());
+    assertTrue(fileStatuses[0].isDirectory());
+    // Since the dir key doesn't exist on server, the modification time is
+    // set to current time upon every listStatus request.
+    modificationTime = fileStatuses[0].getModificationTime();
+    // Check modification time in a small loop, it should be slightly larger
+    // each time
+    for (int i = 0; i < 5; i++) {
+      fileStatuses = o3fs.listStatus(mdir1);
+      assertTrue(modificationTime <= fileStatuses[0].getModificationTime());
 
 Review comment:
   Checkstyle complains the listStatus exceeded 150 LOC and I have to make space for my change

----------------------------------------------------------------
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.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

---------------------------------------------------------------------
To unsubscribe, e-mail: ozone-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: ozone-issues-help@hadoop.apache.org


[GitHub] [hadoop-ozone] smengcl commented on a change in pull request #584: HDDS-3054. OzoneFileStatus#getModificationTime should return actual directory modification time when its OmKeyInfo is available

Posted by GitBox <gi...@apache.org>.
smengcl commented on a change in pull request #584: HDDS-3054. OzoneFileStatus#getModificationTime should return actual directory modification time when its OmKeyInfo is available
URL: https://github.com/apache/hadoop-ozone/pull/584#discussion_r384706707
 
 

 ##########
 File path: hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/fs/ozone/TestOzoneFileSystem.java
 ##########
 @@ -354,4 +356,45 @@ private OzoneKeyDetails getKey(Path keyPath, boolean isDirectory)
   private void assertKeyNotFoundException(IOException ex) {
     GenericTestUtils.assertExceptionContains("KEY_NOT_FOUND", ex);
   }
+
+  private void testGetDirectoryModificationTime() throws IOException {
+    Path mdir1 = new Path("/mdir1");
+    Path mdir11 = new Path(mdir1, "mdir11");
+    Path mdir111 = new Path(mdir11, "mdir111");
+    fs.mkdirs(mdir111);
+    rootItemCount++; // mdir1
+
+    // Case 1: Dir key exist on server
+    FileStatus[] fileStatuses = o3fs.listStatus(mdir11);
+    // Above listStatus result should only have one entry: mdir111
+    assertEquals(1, fileStatuses.length);
+    assertEquals(mdir111.toString(),
+        fileStatuses[0].getPath().toUri().getPath());
+    assertTrue(fileStatuses[0].isDirectory());
+    // The dir key is actually created on server,
+    // so modification time should always be the same value.
+    long modificationTime = fileStatuses[0].getModificationTime();
+    // Check modification time in a small loop, it should always be the same
+    for (int i = 0; i < 5; i++) {
+      fileStatuses = o3fs.listStatus(mdir11);
+      assertEquals(modificationTime, fileStatuses[0].getModificationTime());
 
 Review comment:
   I'll add a small(10ms) sleep in the loop.

----------------------------------------------------------------
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.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

---------------------------------------------------------------------
To unsubscribe, e-mail: ozone-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: ozone-issues-help@hadoop.apache.org


[GitHub] [hadoop-ozone] smengcl commented on a change in pull request #584: HDDS-3054. OzoneFileStatus#getModificationTime should return actual directory modification time when its OmKeyInfo is available

Posted by GitBox <gi...@apache.org>.
smengcl commented on a change in pull request #584: HDDS-3054. OzoneFileStatus#getModificationTime should return actual directory modification time when its OmKeyInfo is available
URL: https://github.com/apache/hadoop-ozone/pull/584#discussion_r384707298
 
 

 ##########
 File path: hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/KeyManagerImpl.java
 ##########
 @@ -2005,15 +2016,21 @@ public OmKeyInfo lookupFile(OmKeyArgs args, String clientAddress)
               if (isFile) {
                 if (!deletedKeySet.contains(entryInDb)) {
                   cacheKeyMap.put(entryInDb,
-                      new OzoneFileStatus(value, scmBlockSize, !isFile));
+                      new OzoneFileStatus(omKeyInfo, scmBlockSize, !isFile));
                   countEntries++;
                 }
                 iterator.next();
               } else {
                 // if entry is a directory
                 if (!deletedKeySet.contains(entryInDb)) {
-                  cacheKeyMap.put(entryInDb,
-                      new OzoneFileStatus(immediateChild));
+                  if (!entryKeyName.equals(immediateChild)) {
+                    cacheKeyMap.put(entryInDb,
+                        new OzoneFileStatus(immediateChild));
+                  } else {
+                    // If entryKeyName matches dir name, we have the info
+                    cacheKeyMap.put(entryInDb,
+                        new OzoneFileStatus(omKeyInfo, 0, true));
 
 Review comment:
   > Could you please explain this change.
   
   Checkstyle complains the listStatus exceeded 150 LOC and I have to make space for my change

----------------------------------------------------------------
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.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

---------------------------------------------------------------------
To unsubscribe, e-mail: ozone-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: ozone-issues-help@hadoop.apache.org


[GitHub] [hadoop-ozone] hanishakoneru commented on a change in pull request #584: HDDS-3054. OzoneFileStatus#getModificationTime should return actual directory modification time when its OmKeyInfo is available

Posted by GitBox <gi...@apache.org>.
hanishakoneru commented on a change in pull request #584: HDDS-3054. OzoneFileStatus#getModificationTime should return actual directory modification time when its OmKeyInfo is available
URL: https://github.com/apache/hadoop-ozone/pull/584#discussion_r385935041
 
 

 ##########
 File path: hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/KeyManagerImpl.java
 ##########
 @@ -2005,15 +2016,21 @@ public OmKeyInfo lookupFile(OmKeyArgs args, String clientAddress)
               if (isFile) {
                 if (!deletedKeySet.contains(entryInDb)) {
                   cacheKeyMap.put(entryInDb,
-                      new OzoneFileStatus(value, scmBlockSize, !isFile));
+                      new OzoneFileStatus(omKeyInfo, scmBlockSize, !isFile));
                   countEntries++;
                 }
                 iterator.next();
               } else {
                 // if entry is a directory
                 if (!deletedKeySet.contains(entryInDb)) {
-                  cacheKeyMap.put(entryInDb,
-                      new OzoneFileStatus(immediateChild));
+                  if (!entryKeyName.equals(immediateChild)) {
+                    cacheKeyMap.put(entryInDb,
+                        new OzoneFileStatus(immediateChild));
+                  } else {
+                    // If entryKeyName matches dir name, we have the info
+                    cacheKeyMap.put(entryInDb,
+                        new OzoneFileStatus(omKeyInfo, 0, true));
 
 Review comment:
   Thanks for explaining Siyao.

----------------------------------------------------------------
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.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

---------------------------------------------------------------------
To unsubscribe, e-mail: ozone-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: ozone-issues-help@hadoop.apache.org


[GitHub] [hadoop-ozone] smengcl commented on a change in pull request #584: HDDS-3054. OzoneFileStatus#getModificationTime should return actual directory modification time when its OmKeyInfo is available

Posted by GitBox <gi...@apache.org>.
smengcl commented on a change in pull request #584: HDDS-3054. OzoneFileStatus#getModificationTime should return actual directory modification time when its OmKeyInfo is available
URL: https://github.com/apache/hadoop-ozone/pull/584#discussion_r384707298
 
 

 ##########
 File path: hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/KeyManagerImpl.java
 ##########
 @@ -2005,15 +2016,21 @@ public OmKeyInfo lookupFile(OmKeyArgs args, String clientAddress)
               if (isFile) {
                 if (!deletedKeySet.contains(entryInDb)) {
                   cacheKeyMap.put(entryInDb,
-                      new OzoneFileStatus(value, scmBlockSize, !isFile));
+                      new OzoneFileStatus(omKeyInfo, scmBlockSize, !isFile));
                   countEntries++;
                 }
                 iterator.next();
               } else {
                 // if entry is a directory
                 if (!deletedKeySet.contains(entryInDb)) {
-                  cacheKeyMap.put(entryInDb,
-                      new OzoneFileStatus(immediateChild));
+                  if (!entryKeyName.equals(immediateChild)) {
+                    cacheKeyMap.put(entryInDb,
+                        new OzoneFileStatus(immediateChild));
+                  } else {
+                    // If entryKeyName matches dir name, we have the info
+                    cacheKeyMap.put(entryInDb,
+                        new OzoneFileStatus(omKeyInfo, 0, true));
 
 Review comment:
   > Could you please explain this change.
   
   `entryKeyName` is retrieved from a DB entry (actually exists), while `immediateChild` is generated using the information of parent dir and a key in it:
   ```java
   String immediateChild = OzoneFSUtils.getImmediateChild(entryKeyName, keyName);
   ```
   As seen in `getImmediateChild()`'s javadoc:
   ```java
     /**
      * The function returns immediate child of given ancestor in a particular
      * descendant. For example if ancestor is /a/b and descendant is /a/b/c/d/e
      * the function should return /a/b/c/. If the descendant itself is the
      * immediate child then it is returned as is without adding a trailing slash.
      * This is done to distinguish files from a directory as in ozone files do
      * not carry a trailing slash.
      */
     public static String getImmediateChild(String descendant, String ancestor) {
   ```

----------------------------------------------------------------
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.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

---------------------------------------------------------------------
To unsubscribe, e-mail: ozone-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: ozone-issues-help@hadoop.apache.org