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

László Bodor updated HIVE-29775:
--------------------------------
    Description: 
{{Utilities.[moveFile|https://github.com/apache/hive/blob/5e9f78815688457ff00d7b3192f7e13d458a16d6/ql/src/java/org/apache/hadoop/hive/ql/exec/Utilities.java#L1145-L1158]}}
 uses the same "probe {{exists()}} -> pick {{_copy_N}} -> {{{}rename(){}}}" 
pattern that HIVE-28822 fixes in {{{}Hive.mvFile{}}}. On filesystems whose 
rename is not atomic-if-absent (S3A and other object stores), two concurrent 
writers can pick the same destination between the probe and the rename; 
outcomes are the same two failure modes:
 * *Fail-loud* — {{S3AFileSystem.initiateRename}} throws 
{{{}FileAlreadyExistsException{}}}, which {{Utilities.moveFile}} wraps into a 
{{HiveException("Unable to move: ... to: ...")}} and hands back to the caller.
 * *Fail-silent* — both writers' internal {{exists()}} probes see the target as 
not-yet-present, both PUTs go to the same key, and the second silently 
overwrites the first (last writer wins, no error surfaces).

HIVE-28822 fixed only {{{}Hive.mvFile{}}}. This ticket applies the same 
approach to {{Utilities.moveFile}} (and the related helpers below).

 

The fix is not trivial, as {{Utilities.moveFile}} knows much less about the 
context, given its signature (kinda src, dest), while Hive.mvFile is very well 
aware of the scenario, and it only applies the workaround in case of non-ACID 
writes. But the pattern is the same; this below is not accurate:

{code}
      int suffix = 0;
      do {
        suffix++;
        if (parsedFileName.matches()) {
          dstFilePath = new Path(destDir, 
parsedFileName.makeFilenameWithCopyIndex(suffix));
        } else {
          dstFilePath = new Path(destDir, destFileName + "_" + suffix);
        }
      } while (fs.exists(dstFilePath));
    }
    if (!fs.rename(srcFile, dstFilePath)) {
      throw new HiveException("Unable to move: " + srcFile + " to: " + 
dstFilePath);
    }
{code}


  was:
{{Utilities.[moveFile|https://github.com/apache/hive/blob/5e9f78815688457ff00d7b3192f7e13d458a16d6/ql/src/java/org/apache/hadoop/hive/ql/exec/Utilities.java#L1145-L1158]}}
 uses the same "probe {{exists()}} -> pick {{_copy_N}} -> {{{}rename(){}}}" 
pattern that HIVE-28822 fixes in {{{}Hive.mvFile{}}}. On filesystems whose 
rename is not atomic-if-absent (S3A and other object stores), two concurrent 
writers can pick the same destination between the probe and the rename; 
outcomes are the same two failure modes:
 * *Fail-loud* — {{S3AFileSystem.initiateRename}} throws 
{{{}FileAlreadyExistsException{}}}, which {{Utilities.moveFile}} wraps into a 
{{HiveException("Unable to move: ... to: ...")}} and hands back to the caller.
 * *Fail-silent* — both writers' internal {{exists()}} probes see the target as 
not-yet-present, both PUTs go to the same key, and the second silently 
overwrites the first (last writer wins, no error surfaces).

HIVE-28822 fixed only {{{}Hive.mvFile{}}}. This ticket applies the same 
approach to {{Utilities.moveFile}} (and the related helpers below).

 

The fix is not trivial, as {{Utilities.moveFile}} knows much less about the 
context, given its signature (kinda src, dest), while Hive.mvFile is very well 
aware of the scenario, and it only applies the workaround in case of non-ACID 
writes. But the pattern is the same; this below is not accurate:

{code}
      int suffix = 0;
      do {
        suffix++;
        if (parsedFileName.matches()) {
          dstFilePath = new Path(destDir, 
parsedFileName.makeFilenameWithCopyIndex(suffix));
        } else {
          dstFilePath = new Path(destDir, destFileName + "_" + suffix);
        }
      } while (fs.exists(dstFilePath));
    }
    if (!fs.rename(srcFile, dstFilePath)) {
      throw new HiveException("Unable to move: " + srcFile + " to: " + 
dstFilePath);
    }
{code}


> Fix a possible exists() - pick copy_N - rename race condition in 
> Utilities.moveFile to avoid possible data loss
> ---------------------------------------------------------------------------------------------------------------
>
>                 Key: HIVE-29775
>                 URL: https://issues.apache.org/jira/browse/HIVE-29775
>             Project: Hive
>          Issue Type: Bug
>            Reporter: László Bodor
>            Priority: Major
>
> {{Utilities.[moveFile|https://github.com/apache/hive/blob/5e9f78815688457ff00d7b3192f7e13d458a16d6/ql/src/java/org/apache/hadoop/hive/ql/exec/Utilities.java#L1145-L1158]}}
>  uses the same "probe {{exists()}} -> pick {{_copy_N}} -> {{{}rename(){}}}" 
> pattern that HIVE-28822 fixes in {{{}Hive.mvFile{}}}. On filesystems whose 
> rename is not atomic-if-absent (S3A and other object stores), two concurrent 
> writers can pick the same destination between the probe and the rename; 
> outcomes are the same two failure modes:
>  * *Fail-loud* — {{S3AFileSystem.initiateRename}} throws 
> {{{}FileAlreadyExistsException{}}}, which {{Utilities.moveFile}} wraps into a 
> {{HiveException("Unable to move: ... to: ...")}} and hands back to the caller.
>  * *Fail-silent* — both writers' internal {{exists()}} probes see the target 
> as not-yet-present, both PUTs go to the same key, and the second silently 
> overwrites the first (last writer wins, no error surfaces).
> HIVE-28822 fixed only {{{}Hive.mvFile{}}}. This ticket applies the same 
> approach to {{Utilities.moveFile}} (and the related helpers below).
>  
> The fix is not trivial, as {{Utilities.moveFile}} knows much less about the 
> context, given its signature (kinda src, dest), while Hive.mvFile is very 
> well aware of the scenario, and it only applies the workaround in case of 
> non-ACID writes. But the pattern is the same; this below is not accurate:
> {code}
>       int suffix = 0;
>       do {
>         suffix++;
>         if (parsedFileName.matches()) {
>           dstFilePath = new Path(destDir, 
> parsedFileName.makeFilenameWithCopyIndex(suffix));
>         } else {
>           dstFilePath = new Path(destDir, destFileName + "_" + suffix);
>         }
>       } while (fs.exists(dstFilePath));
>     }
>     if (!fs.rename(srcFile, dstFilePath)) {
>       throw new HiveException("Unable to move: " + srcFile + " to: " + 
> dstFilePath);
>     }
> {code}



--
This message was sent by Atlassian Jira
(v8.20.10#820010)

Reply via email to