You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@hbase.apache.org by vj...@apache.org on 2023/08/28 02:31:00 UTC
[hbase] branch branch-3 updated: HBASE-28042 Snapshot corruptions due to non-atomic rename within same filesystem (#5369)
This is an automated email from the ASF dual-hosted git repository.
vjasani pushed a commit to branch branch-3
in repository https://gitbox.apache.org/repos/asf/hbase.git
The following commit(s) were added to refs/heads/branch-3 by this push:
new 91fdd020d49 HBASE-28042 Snapshot corruptions due to non-atomic rename within same filesystem (#5369)
91fdd020d49 is described below
commit 91fdd020d490cc96cbb7b2713bec3096c0e0b1ac
Author: Viraj Jasani <vj...@apache.org>
AuthorDate: Sun Aug 27 19:27:05 2023 -0700
HBASE-28042 Snapshot corruptions due to non-atomic rename within same filesystem (#5369)
Co-authored-by: Ujjawal <uj...@gmail.com>
Signed-off-by: Wellington Chevreuil <wc...@apache.org>
Signed-off-by: Abhey Rana <a....@salesforce.com>
Signed-off-by: Ujjawal <uj...@gmail.com>
Signed-off-by: Aman Poonia <am...@gmail.com>
---
.../hbase/snapshot/SnapshotDescriptionUtils.java | 37 ++++++++++++++--
.../TestSnapshotScannerHDFSAclController.java | 13 +++---
.../snapshot/TestSnapshotDescriptionUtils.java | 49 ++++++++++++++++++++++
3 files changed, 88 insertions(+), 11 deletions(-)
diff --git a/hbase-server/src/main/java/org/apache/hadoop/hbase/snapshot/SnapshotDescriptionUtils.java b/hbase-server/src/main/java/org/apache/hadoop/hbase/snapshot/SnapshotDescriptionUtils.java
index 1e2bbb68c41..689cd89259b 100644
--- a/hbase-server/src/main/java/org/apache/hadoop/hbase/snapshot/SnapshotDescriptionUtils.java
+++ b/hbase-server/src/main/java/org/apache/hadoop/hbase/snapshot/SnapshotDescriptionUtils.java
@@ -419,11 +419,9 @@ public final class SnapshotDescriptionUtils {
// if this fails
URI workingURI = workingDirFs.getUri();
URI rootURI = fs.getUri();
+
if (
- (!workingURI.getScheme().equals(rootURI.getScheme()) || workingURI.getAuthority() == null
- || !workingURI.getAuthority().equals(rootURI.getAuthority())
- || workingURI.getUserInfo() == null
- || !workingURI.getUserInfo().equals(rootURI.getUserInfo())
+ (shouldSkipRenameSnapshotDirectories(workingURI, rootURI)
|| !fs.rename(workingDir, snapshotDir))
&& !FileUtil.copy(workingDirFs, workingDir, fs, snapshotDir, true, true, conf)
) {
@@ -432,6 +430,37 @@ public final class SnapshotDescriptionUtils {
}
}
+ static boolean shouldSkipRenameSnapshotDirectories(URI workingURI, URI rootURI) {
+ // check scheme, e.g. file, hdfs
+ if (workingURI.getScheme() == null && rootURI.getScheme() != null) {
+ return true;
+ }
+ if (workingURI.getScheme() != null && !workingURI.getScheme().equals(rootURI.getScheme())) {
+ return true;
+ }
+
+ // check Authority, e.g. localhost:port
+ if (workingURI.getAuthority() == null && rootURI.getAuthority() != null) {
+ return true;
+ }
+ if (
+ workingURI.getAuthority() != null && !workingURI.getAuthority().equals(rootURI.getAuthority())
+ ) {
+ return true;
+ }
+
+ // check UGI/userInfo
+ if (workingURI.getUserInfo() == null && rootURI.getUserInfo() != null) {
+ return true;
+ }
+ if (
+ workingURI.getUserInfo() != null && !workingURI.getUserInfo().equals(rootURI.getUserInfo())
+ ) {
+ return true;
+ }
+ return false;
+ }
+
/**
* Check if the user is this table snapshot's owner
* @param snapshot the table snapshot description
diff --git a/hbase-server/src/test/java/org/apache/hadoop/hbase/security/access/TestSnapshotScannerHDFSAclController.java b/hbase-server/src/test/java/org/apache/hadoop/hbase/security/access/TestSnapshotScannerHDFSAclController.java
index d79e3f30810..99d5a89ac2c 100644
--- a/hbase-server/src/test/java/org/apache/hadoop/hbase/security/access/TestSnapshotScannerHDFSAclController.java
+++ b/hbase-server/src/test/java/org/apache/hadoop/hbase/security/access/TestSnapshotScannerHDFSAclController.java
@@ -158,6 +158,7 @@ public class TestSnapshotScannerHDFSAclController {
TestHDFSAclHelper.createTableAndPut(TEST_UTIL, table);
snapshotAndWait(snapshot1, table);
+ snapshotAndWait(snapshot2, table);
// grant G(R)
SecureTestUtil.grantGlobal(TEST_UTIL, grantUserName, READ);
TestHDFSAclHelper.canUserScanSnapshot(TEST_UTIL, grantUser, snapshot1, 6);
@@ -174,8 +175,6 @@ public class TestSnapshotScannerHDFSAclController {
// grant G(R)
SecureTestUtil.grantGlobal(TEST_UTIL, grantUserName, READ);
TestHDFSAclHelper.canUserScanSnapshot(TEST_UTIL, grantUser, snapshot1, 6);
- // take a snapshot and ACLs are inherited automatically
- snapshotAndWait(snapshot2, table);
TestHDFSAclHelper.canUserScanSnapshot(TEST_UTIL, grantUser, snapshot2, 6);
assertTrue(hasUserGlobalHdfsAcl(aclTable, grantUserName));
deleteTable(table);
@@ -197,10 +196,10 @@ public class TestSnapshotScannerHDFSAclController {
// create table in namespace1 and snapshot
TestHDFSAclHelper.createTableAndPut(TEST_UTIL, table1);
snapshotAndWait(snapshot1, table1);
- admin.grant(new UserPermission(grantUserName,
- Permission.newBuilder(namespace1).withActions(READ).build()), false);
// grant G(W)
SecureTestUtil.grantGlobal(TEST_UTIL, grantUserName, WRITE);
+ admin.grant(new UserPermission(grantUserName,
+ Permission.newBuilder(namespace1).withActions(READ).build()), false);
// create table in namespace2 and snapshot
TestHDFSAclHelper.createTableAndPut(TEST_UTIL, table2);
snapshotAndWait(snapshot2, table2);
@@ -231,11 +230,11 @@ public class TestSnapshotScannerHDFSAclController {
// grant table1(R)
TestHDFSAclHelper.createTableAndPut(TEST_UTIL, table1);
snapshotAndWait(snapshot1, table1);
- TestHDFSAclHelper.grantOnTable(TEST_UTIL, grantUserName, table1, READ);
- // grant G(W)
- SecureTestUtil.grantGlobal(TEST_UTIL, grantUserName, WRITE);
TestHDFSAclHelper.createTableAndPut(TEST_UTIL, table2);
snapshotAndWait(snapshot2, table2);
+ // grant G(W)
+ SecureTestUtil.grantGlobal(TEST_UTIL, grantUserName, WRITE);
+ TestHDFSAclHelper.grantOnTable(TEST_UTIL, grantUserName, table1, READ);
// check scan snapshot
TestHDFSAclHelper.canUserScanSnapshot(TEST_UTIL, grantUser, snapshot1, 6);
TestHDFSAclHelper.canUserScanSnapshot(TEST_UTIL, grantUser, snapshot2, -1);
diff --git a/hbase-server/src/test/java/org/apache/hadoop/hbase/snapshot/TestSnapshotDescriptionUtils.java b/hbase-server/src/test/java/org/apache/hadoop/hbase/snapshot/TestSnapshotDescriptionUtils.java
index 2093baf3607..8e62ef16bbf 100644
--- a/hbase-server/src/test/java/org/apache/hadoop/hbase/snapshot/TestSnapshotDescriptionUtils.java
+++ b/hbase-server/src/test/java/org/apache/hadoop/hbase/snapshot/TestSnapshotDescriptionUtils.java
@@ -22,6 +22,7 @@ import static org.junit.Assert.assertTrue;
import static org.junit.Assert.fail;
import java.io.IOException;
+import java.net.URI;
import org.apache.hadoop.conf.Configuration;
import org.apache.hadoop.fs.FileSystem;
import org.apache.hadoop.fs.Path;
@@ -191,4 +192,52 @@ public class TestSnapshotDescriptionUtils {
assertTrue(SnapshotDescriptionUtils.isWithinDefaultWorkingDir(
new Path("file:" + hbsaeDir + "/.hbase-snapshot/.tmp/snapshot"), conf));
}
+
+ @Test
+ public void testShouldSkipRenameSnapshotDirectories() {
+ URI workingDirURI = URI.create("/User/test1");
+ URI rootDirURI = URI.create("hdfs:///User/test2");
+
+ // should skip rename if it's not the same scheme;
+ assertTrue(
+ SnapshotDescriptionUtils.shouldSkipRenameSnapshotDirectories(workingDirURI, rootDirURI));
+
+ workingDirURI = URI.create("/User/test1");
+ rootDirURI = URI.create("file:///User/test2");
+ assertTrue(
+ SnapshotDescriptionUtils.shouldSkipRenameSnapshotDirectories(workingDirURI, rootDirURI));
+
+ // skip rename when either scheme or authority are the not same
+ workingDirURI = URI.create("hdfs://localhost:8020/User/test1");
+ rootDirURI = URI.create("hdfs://otherhost:8020/User/test2");
+ assertTrue(
+ SnapshotDescriptionUtils.shouldSkipRenameSnapshotDirectories(workingDirURI, rootDirURI));
+
+ workingDirURI = URI.create("file:///User/test1");
+ rootDirURI = URI.create("hdfs://localhost:8020/User/test2");
+ assertTrue(
+ SnapshotDescriptionUtils.shouldSkipRenameSnapshotDirectories(workingDirURI, rootDirURI));
+
+ workingDirURI = URI.create("hdfs:///User/test1");
+ rootDirURI = URI.create("hdfs:///User/test2");
+ assertFalse(
+ SnapshotDescriptionUtils.shouldSkipRenameSnapshotDirectories(workingDirURI, rootDirURI));
+
+ workingDirURI = URI.create("hdfs://localhost:8020/User/test1");
+ rootDirURI = URI.create("hdfs://localhost:8020/User/test2");
+ assertFalse(
+ SnapshotDescriptionUtils.shouldSkipRenameSnapshotDirectories(workingDirURI, rootDirURI));
+
+ workingDirURI = URI.create("hdfs://user:password@localhost:8020/User/test1");
+ rootDirURI = URI.create("hdfs://user:password@localhost:8020/User/test2");
+ assertFalse(
+ SnapshotDescriptionUtils.shouldSkipRenameSnapshotDirectories(workingDirURI, rootDirURI));
+
+ // skip rename when user information is not the same
+ workingDirURI = URI.create("hdfs://user:password@localhost:8020/User/test1");
+ rootDirURI = URI.create("hdfs://user2:password2@localhost:8020/User/test2");
+ assertTrue(
+ SnapshotDescriptionUtils.shouldSkipRenameSnapshotDirectories(workingDirURI, rootDirURI));
+ }
+
}