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

[WIP][SPARK-40464][YARN] Support automatic data format conversion for shuffle state db #38220

Closed
wants to merge 1 commit into from

Conversation

panbingkun
Copy link
Contributor

What changes were proposed in this pull request?

Why are the changes needed?

Does this PR introduce any user-facing change?

No.

How was this patch tested?

Pass GA.

LOGGER.warn("Migrate DBBackend from {}({}) to {}({})",
fromDbBackend.name(), fromFile.getCanonicalPath(),
toDbBackend.name(), toFile.getCanonicalPath());
DB fromDb = initDB(fromDbBackend, fromFile, version, mapper);
Copy link
Contributor

Choose a reason for hiding this comment

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

Should close the fromDb and toDb after used

Preconditions.checkNotNull(_recoveryPath,
"recovery path should not be null if NM recovery is enabled");

DBBackend registeredExecutorsDbBackend = findExistedDbBackend(RECOVERY_FILE_NAME);
Copy link
Contributor

Choose a reason for hiding this comment

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

line 503 ~ line 511 can be extract into a general method and put into the helper class

@@ -493,6 +496,49 @@ protected Path getRecoveryPath(String fileName) {
return _recoveryPath;
}

private void migrateRecoveryDb() throws IOException {
Preconditions.checkNotNull(_recoveryPath,
Copy link
Contributor

Choose a reason for hiding this comment

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

should not be a checkNotNull, I think it should be _recoveryPath

@LuciferYang
Copy link
Contributor

  1. How to deal with the old database after migration? Delete or Rename?
  2. If there are multiple old databases at the same time, what should we do?
  3. Add configuration to control migration behavior?
  4. How to test?

@github-actions github-actions bot added the CORE label Oct 12, 2022
@LuciferYang
Copy link
Contributor

@panbingkun Any updates of this one ?

@AmplabJenkins
Copy link

Can one of the admins verify this patch?

@panbingkun
Copy link
Contributor Author

@panbingkun Any updates of this one ?

I will update it weekend.

@github-actions
Copy link

We're closing this PR because it hasn't been updated in a while. This isn't a judgement on the merit of the PR in any way. It's just a way of keeping the PR queue manageable.
If you'd like to revive this PR, please reopen it and ask a committer to remove the Stale tag!

@github-actions github-actions bot added the Stale label Jan 29, 2023
@github-actions github-actions bot closed this Jan 30, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
3 participants