Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[SPARK-45542][CORE] Replace setSafeMode(HdfsConstants.SafeModeAction, boolean) with setSafeMode(SafeModeAction, boolean) #43377

Closed
wants to merge 1 commit into from

Conversation

LuciferYang
Copy link
Contributor

@LuciferYang LuciferYang commented Oct 15, 2023

What changes were proposed in this pull request?

The function FsHistoryProvider#isFsInSafeMode is using the deprecated API

https://github.com/apache/hadoop/blob/1be78238728da9266a4f88195058f08fd012bf9c/hadoop-hdfs-project/hadoop-hdfs-client/src/main/java/org/apache/hadoop/hdfs/DistributedFileSystem.java#L1702-L1722

/**
   * Enter, leave or get safe mode.
   *
   * @param action
   *          One of SafeModeAction.ENTER, SafeModeAction.LEAVE and
   *          SafeModeAction.GET.
   * @param isChecked
   *          If true check only for Active NNs status, else check first NN's
   *          status.
   *
   * @see org.apache.hadoop.hdfs.protocol.ClientProtocol#setSafeMode(HdfsConstants.SafeModeAction,
   * boolean)
   *
   * @deprecated please instead use
   *               {@link DistributedFileSystem#setSafeMode(SafeModeAction, boolean)}.
   */
  @Deprecated
  public boolean setSafeMode(HdfsConstants.SafeModeAction action,
      boolean isChecked) throws IOException {
    return dfs.setSafeMode(action, isChecked);
  }

This pr change to use

https://github.com/apache/hadoop/blob/1be78238728da9266a4f88195058f08fd012bf9c/hadoop-hdfs-project/hadoop-hdfs-client/src/main/java/org/apache/hadoop/hdfs/DistributedFileSystem.java#L1646-L1661

 /**
   * Enter, leave or get safe mode.
   *
   * @param action
   *          One of SafeModeAction.ENTER, SafeModeAction.LEAVE and
   *          SafeModeAction.GET.
   * @param isChecked
   *          If true check only for Active NNs status, else check first NN's
   *          status.
   */
  @Override
  @SuppressWarnings("deprecation")
  public boolean setSafeMode(SafeModeAction action, boolean isChecked)
      throws IOException {
    return this.setSafeMode(convertToClientProtocolSafeModeAction(action), isChecked);
  }

instead of it.

The conversion relationship from HdfsConstants.SafeModeAction to SafeModeAction refers to convertToClientProtocolSafeModeAction method.

https://github.com/apache/hadoop/blob/1be78238728da9266a4f88195058f08fd012bf9c/hadoop-hdfs-project/hadoop-hdfs-client/src/main/java/org/apache/hadoop/hdfs/DistributedFileSystem.java#L1663-L1686

/**
   * Translating the {@link SafeModeAction} into {@link HdfsConstants.SafeModeAction}
   * that is used by {@link DFSClient#setSafeMode(HdfsConstants.SafeModeAction, boolean)}.
   *
   * @param action any supported action listed in {@link SafeModeAction}.
   * @return the converted {@link HdfsConstants.SafeModeAction}.
   * @throws UnsupportedOperationException if the provided {@link SafeModeAction} cannot be
   *           translated.
   */
  private static HdfsConstants.SafeModeAction convertToClientProtocolSafeModeAction(
      SafeModeAction action) {
    switch (action) {
    case ENTER:
      return HdfsConstants.SafeModeAction.SAFEMODE_ENTER;
    case LEAVE:
      return HdfsConstants.SafeModeAction.SAFEMODE_LEAVE;
    case FORCE_EXIT:
      return HdfsConstants.SafeModeAction.SAFEMODE_FORCE_EXIT;
    case GET:
      return HdfsConstants.SafeModeAction.SAFEMODE_GET;
    default:
      throw new UnsupportedOperationException("Unsupported safe mode action " + action);
    }
  }

Why are the changes needed?

Clean up deprecated API usage

Does this PR introduce any user-facing change?

No

How was this patch tested?

Pass GitHub Actions

Was this patch authored or co-authored using generative AI tooling?

No

@github-actions github-actions bot added the CORE label Oct 15, 2023
Copy link
Contributor

@beliefer beliefer left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM.

@LuciferYang
Copy link
Contributor Author

cc @dongjoon-hyun Could you please take a look If you have time. Thanks ~

@dongjoon-hyun
Copy link
Member

Sure, @LuciferYang .

Copy link
Member

@dongjoon-hyun dongjoon-hyun left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

+1, LGTM.

@dongjoon-hyun
Copy link
Member

Merged to master for Apache Spark 4.0.0.

@LuciferYang
Copy link
Contributor Author

Thanks @dongjoon-hyun and @beliefer ~

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
3 participants