You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@ozone.apache.org by "Siyao Meng (Jira)" <ji...@apache.org> on 2020/02/21 15:05:00 UTC

[jira] [Updated] (HDDS-3054) OzoneFileStatus#getModificationTime should return actual directory modification time when this information is available

     [ https://issues.apache.org/jira/browse/HDDS-3054?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]

Siyao Meng updated HDDS-3054:
-----------------------------
    Description: 
As of current implementation, [{{getModificationTime()}}|https://github.com/apache/hadoop-ozone/blob/c9f26ccf9f93a052c5c0c042c57b6f87709597ae/hadoop-ozone/common/src/main/java/org/apache/hadoop/ozone/om/helpers/OzoneFileStatus.java#L90-L107] 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.

CC [~xyao]


My POC for the fix:
{code:java|title=Diff}
diff --git a/hadoop-ozone/common/src/main/java/org/apache/hadoop/ozone/om/helpers/OzoneFileStatus.java b/hadoop-ozone/common/src/main/java/org/apache/hadoop/ozone/om/helpers/OzoneFileStatus.java
index 8717946512..708e62d692 100644
--- a/hadoop-ozone/common/src/main/java/org/apache/hadoop/ozone/om/helpers/OzoneFileStatus.java
+++ b/hadoop-ozone/common/src/main/java/org/apache/hadoop/ozone/om/helpers/OzoneFileStatus.java
@@ -93,7 +93,7 @@ public FileStatus makeQualified(URI defaultUri, Path parent,
    */
   @Override
   public long getModificationTime(){
-    if (isDirectory()) {
+    if (isDirectory() && super.getModificationTime() == 0) {
       return System.currentTimeMillis();
     } else {
       return super.getModificationTime();
diff --git a/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/KeyManagerImpl.java b/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/KeyManagerImpl.java
index 1be5fb3f3c..cb8f647a41 100644
--- a/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/KeyManagerImpl.java
+++ b/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/KeyManagerImpl.java
@@ -2004,8 +2004,14 @@ public OmKeyInfo lookupFile(OmKeyArgs args, String clientAddress)
               } 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(value, 0, true));
+                  }
                   countEntries++;
                 }
                 // skip the other descendants of this child directory.
{code}

  was:
As of current implementation, [{{getModificationTime()}}|https://github.com/apache/hadoop-ozone/blob/c9f26ccf9f93a052c5c0c042c57b6f87709597ae/hadoop-ozone/common/src/main/java/org/apache/hadoop/ozone/om/helpers/OzoneFileStatus.java#L90-L107] 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.

CC [~xyao]


My POC for the fix:
{code}
diff --git a/hadoop-ozone/common/src/main/java/org/apache/hadoop/ozone/om/helpers/OzoneFileStatus.java b/hadoop-ozone/common/src/main/java/org/apache/hadoop/ozone/om/helpers/OzoneFileStatus.java
index 8717946512..708e62d692 100644
--- a/hadoop-ozone/common/src/main/java/org/apache/hadoop/ozone/om/helpers/OzoneFileStatus.java
+++ b/hadoop-ozone/common/src/main/java/org/apache/hadoop/ozone/om/helpers/OzoneFileStatus.java
@@ -93,7 +93,7 @@ public FileStatus makeQualified(URI defaultUri, Path parent,
    */
   @Override
   public long getModificationTime(){
-    if (isDirectory()) {
+    if (isDirectory() && super.getModificationTime() == 0) {
       return System.currentTimeMillis();
     } else {
       return super.getModificationTime();
diff --git a/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/KeyManagerImpl.java b/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/KeyManagerImpl.java
index 1be5fb3f3c..cb8f647a41 100644
--- a/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/KeyManagerImpl.java
+++ b/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/KeyManagerImpl.java
@@ -2004,8 +2004,14 @@ public OmKeyInfo lookupFile(OmKeyArgs args, String clientAddress)
               } 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(value, 0, true));
+                  }
                   countEntries++;
                 }
                 // skip the other descendants of this child directory.
{code}


> OzoneFileStatus#getModificationTime should return actual directory modification time when this information is available
> -----------------------------------------------------------------------------------------------------------------------
>
>                 Key: HDDS-3054
>                 URL: https://issues.apache.org/jira/browse/HDDS-3054
>             Project: Hadoop Distributed Data Store
>          Issue Type: Improvement
>            Reporter: Siyao Meng
>            Assignee: Siyao Meng
>            Priority: Major
>
> As of current implementation, [{{getModificationTime()}}|https://github.com/apache/hadoop-ozone/blob/c9f26ccf9f93a052c5c0c042c57b6f87709597ae/hadoop-ozone/common/src/main/java/org/apache/hadoop/ozone/om/helpers/OzoneFileStatus.java#L90-L107] 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.
> CC [~xyao]
> My POC for the fix:
> {code:java|title=Diff}
> diff --git a/hadoop-ozone/common/src/main/java/org/apache/hadoop/ozone/om/helpers/OzoneFileStatus.java b/hadoop-ozone/common/src/main/java/org/apache/hadoop/ozone/om/helpers/OzoneFileStatus.java
> index 8717946512..708e62d692 100644
> --- a/hadoop-ozone/common/src/main/java/org/apache/hadoop/ozone/om/helpers/OzoneFileStatus.java
> +++ b/hadoop-ozone/common/src/main/java/org/apache/hadoop/ozone/om/helpers/OzoneFileStatus.java
> @@ -93,7 +93,7 @@ public FileStatus makeQualified(URI defaultUri, Path parent,
>     */
>    @Override
>    public long getModificationTime(){
> -    if (isDirectory()) {
> +    if (isDirectory() && super.getModificationTime() == 0) {
>        return System.currentTimeMillis();
>      } else {
>        return super.getModificationTime();
> diff --git a/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/KeyManagerImpl.java b/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/KeyManagerImpl.java
> index 1be5fb3f3c..cb8f647a41 100644
> --- a/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/KeyManagerImpl.java
> +++ b/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/KeyManagerImpl.java
> @@ -2004,8 +2004,14 @@ public OmKeyInfo lookupFile(OmKeyArgs args, String clientAddress)
>                } 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(value, 0, true));
> +                  }
>                    countEntries++;
>                  }
>                  // skip the other descendants of this child directory.
> {code}



--
This message was sent by Atlassian Jira
(v8.3.4#803005)

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