Skip to content

HBASE-26720 ExportSnapshot should validate the source snapshot before copying files #4216

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.

Sign up for GitHub

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

Merged
sunhelly merged 13 commits into from
Mar 16, 2022
Merged

HBASE-26720 ExportSnapshot should validate the source snapshot before copying files #4216

Changes from 10 commits
Commits
Show all changes
13 commits
Select commit Hold shift + click to select a range
f6c46cf
Merge pull request #1 from apache/master
xicm Nov 16, 2021
13e75fa
Merge branch 'apache:master' into master
xicm Mar 3, 2022
b22f86f
Merge branch 'apache:master' into master
xicm Mar 4, 2022
e497ed1
Merge branch 'apache:master' into master
xicm Mar 10, 2022
f3485e5
Merge branch 'apache:master' into master
xicm Mar 11, 2022
f4a9985
HBASE-26720, validate the source snapshot before copying files
xicm Mar 14, 2022
2eb1ed2
Merge branch 'apache:master' into master
xicm Mar 14, 2022
7573c20
Merge branch 'master' into HBASE-26720
xicm Mar 14, 2022
69d19a2
Merge branch 'apache:master' into master
xicm Mar 15, 2022
9cc15d2
Merge branch 'master' into HBASE-26720
xicm Mar 15, 2022
ed39013
update log about check of snapshot source
xicm Mar 16, 2022
4ab34d7
Merge branch 'apache:master' into master
xicm Mar 16, 2022
68ecb23
Merge branch 'master' into HBASE-26720
xicm Mar 16, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
11 changes: 11 additions & 0 deletions hbase-mapreduce/src/main/java/org/apache/hadoop/hbase/snapshot/ExportSnapshot.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -143,6 +143,8 @@ static final class Options {
"Do not verify checksum, use name+length only.");
static final Option NO_TARGET_VERIFY = new Option(null, "no-target-verify", false,
"Do not verify the integrity of the exported snapshot.");
static final Option NO_SOURCE_VERIFY = new Option(null, "no-source-verify", false,
"Do not verify the source of the snapshot.");
static final Option OVERWRITE = new Option(null, "overwrite", false,
"Rewrite the snapshot manifest if already exists.");
static final Option CHUSER = new Option(null, "chuser", true,
Expand Down Expand Up @@ -915,6 +917,7 @@ private void setPermissionParallel(final FileSystem outputFs, final short filesM
}

private boolean verifyTarget = true;
private boolean verifySource = true;
private boolean verifyChecksum = true;
private String snapshotName = null;
private String targetName = null;
Expand Down Expand Up @@ -946,6 +949,7 @@ protected void processOptions(CommandLine cmd) {
// And verifyChecksum and verifyTarget with values read from old args in processOldArgs(...).
verifyChecksum = !cmd.hasOption(Options.NO_CHECKSUM_VERIFY.getLongOpt());
verifyTarget = !cmd.hasOption(Options.NO_TARGET_VERIFY.getLongOpt());
verifySource = !cmd.hasOption(Options.NO_SOURCE_VERIFY.getLongOpt());
}

/**
Expand Down Expand Up @@ -996,6 +1000,12 @@ public int doWork() throws IOException {
LOG.debug("outputFs={}, outputRoot={}, skipTmp={}, initialOutputSnapshotDir={}",
outputFs, outputRoot.toString(), skipTmp, initialOutputSnapshotDir);

// Verify snapshot source before copying files
if (verifySource) {
LOG.info("Verify snapshot source.");
Copy link
Contributor

@virajjasani virajjasani Mar 15, 2022

Choose a reason for hiding this comment

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

How about adding srcConf, inputFs, inputRoot, snapshotDir values in the log as well?

Copy link
Contributor Author

@xicm xicm Mar 16, 2022

Choose a reason for hiding this comment

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

I added inputFs, inputRoot, snapshotDir values in the log, srcConf contains too many things that I didn't add.

verifySnapshot(srcConf, inputFs, inputRoot, snapshotDir);
}

// Find the necessary directory which need to change owner and group
Path needSetOwnerDir = SnapshotDescriptionUtils.getSnapshotRootDir(outputRoot);
if (outputFs.exists(needSetOwnerDir)) {
Expand Down Expand Up @@ -1146,6 +1156,7 @@ protected void printUsage() {
addOption(Options.TARGET_NAME);
addOption(Options.NO_CHECKSUM_VERIFY);
addOption(Options.NO_TARGET_VERIFY);
addOption(Options.NO_SOURCE_VERIFY);
addOption(Options.OVERWRITE);
addOption(Options.CHUSER);
addOption(Options.CHGROUP);
Expand Down