Github user vanzin commented on a diff in the pull request:

    https://github.com/apache/spark/pull/14999#discussion_r77917894
  
    --- Diff: 
common/network-yarn/src/main/java/org/apache/spark/network/yarn/YarnShuffleService.java
 ---
    @@ -328,37 +327,51 @@ public void setRecoveryPath(Path recoveryPath) {
       }
     
       /**
    -   * Get the recovery path, this will override the default one to get our 
own maintained
    -   * recovery path.
    +   * Get the path specific to this auxiliary service to use for recovery.
    +   */ 
    +  protected Path getRecoveryPath(String fileName) {
    +    return _recoveryPath;
    +  }
    +
    +  /**
    +   * Figure out the recovery path and handle moving the DB if YARN NM 
recovery gets enabled
    +   * when it previously was not. If YARN NM recovery is enabled it uses 
that path, otherwise
    +   * it will uses a YARN local dir.
        */
    -  protected Path getRecoveryPath() {
    +  protected File initRecoveryDb(String dbFileName) {
         String[] localDirs = 
_conf.getTrimmedStrings("yarn.nodemanager.local-dirs");
         for (String dir : localDirs) {
    -      File f = new File(new Path(dir).toUri().getPath(), 
RECOVERY_FILE_NAME);
    +      File f = new File(new Path(dir).toUri().getPath(), dbFileName);
           if (f.exists()) {
             if (_recoveryPath == null) {
               // If NM recovery is not enabled, we should specify the recovery 
path using NM local
               // dirs, which is compatible with the old code.
               _recoveryPath = new Path(dir);
    +          return f;
             } else {
    -          // If NM recovery is enabled and the recovery file exists in old 
NM local dirs, which
    -          // means old version of Spark already generated the recovery 
file, we should copy the
    -          // old file in to a new recovery path for the compatibility.
    -          if (!f.renameTo(new File(_recoveryPath.toUri().getPath(), 
RECOVERY_FILE_NAME))) {
    -            // Fail to move recovery file to new path
    -            logger.error("Failed to move recovery file {} to the path {}",
    -              RECOVERY_FILE_NAME, _recoveryPath.toString());
    +          // If the recovery path is set then either NM recovery is 
enabled or another recovery
    +          // DB has been initialized. If NM recovery is enabled and had 
set the recovery path
    +          // make sure to move all DBs to the recovery path from the old 
NM local dirs.
    +          // If another DB was initialized first just make sure all the 
DBs are in the same
    +          // location.
    +          File newLoc = new File(_recoveryPath.toUri().getPath(), 
dbFileName);
    +          if (!newLoc.equals(f)) {
    +            if (!f.renameTo(newLoc)) {
    --- End diff --
    
    It's probably safer to use `Files.move` here since these might be in 
different file systems.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at [email protected] or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]

Reply via email to