Skip to content

Commit 7777bcc

Browse files
virajjasanivirajjasani
authored
HBASE-28042 Snapshot corruptions due to non-atomic rename within same filesystem (#5372) (#5369)
Co-Authored-By: Ujjawal <ujjawal4046@gmail.com> Signed-off-by: Wellington Chevreuil <wchevreuil@apache.org> Signed-off-by: Abhey Rana <a.rana@salesforce.com> Signed-off-by: Ujjawal <ujjawal4046@gmail.com> Signed-off-by: Aman Poonia <aman.poonia.29@gmail.com>
1 parent b973ae4 commit 7777bcc

File tree

4 files changed

+89
-12
lines changed

4 files changed

+89
-12
lines changed

‎hbase-server/src/main/java/org/apache/hadoop/hbase/snapshot/SnapshotDescriptionUtils.java

Lines changed: 33 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -435,11 +435,9 @@ public static void completeSnapshot(Path snapshotDir, Path workingDir, FileSyste
435435
// if this fails
436436
URI workingURI = workingDirFs.getUri();
437437
URI rootURI = fs.getUri();
438+
438439
if (
439-
(!workingURI.getScheme().equals(rootURI.getScheme()) || workingURI.getAuthority() == null
440-
|| !workingURI.getAuthority().equals(rootURI.getAuthority())
441-
|| workingURI.getUserInfo() == null
442-
|| !workingURI.getUserInfo().equals(rootURI.getUserInfo())
440+
(shouldSkipRenameSnapshotDirectories(workingURI, rootURI)
443441
|| !fs.rename(workingDir, snapshotDir))
444442
&& !FileUtil.copy(workingDirFs, workingDir, fs, snapshotDir, true, true, conf)
445443
) {
@@ -448,6 +446,37 @@ public static void completeSnapshot(Path snapshotDir, Path workingDir, FileSyste
448446
}
449447
}
450448

449+
static boolean shouldSkipRenameSnapshotDirectories(URI workingURI, URI rootURI) {
450+
// check scheme, e.g. file, hdfs
451+
if (workingURI.getScheme() == null && rootURI.getScheme() != null) {
452+
return true;
453+
}
454+
if (workingURI.getScheme() != null && !workingURI.getScheme().equals(rootURI.getScheme())) {
455+
return true;
456+
}
457+
458+
// check Authority, e.g. localhost:port
459+
if (workingURI.getAuthority() == null && rootURI.getAuthority() != null) {
460+
return true;
461+
}
462+
if (
463+
workingURI.getAuthority() != null && !workingURI.getAuthority().equals(rootURI.getAuthority())
464+
) {
465+
return true;
466+
}
467+
468+
// check UGI/userInfo
469+
if (workingURI.getUserInfo() == null && rootURI.getUserInfo() != null) {
470+
return true;
471+
}
472+
if (
473+
workingURI.getUserInfo() != null && !workingURI.getUserInfo().equals(rootURI.getUserInfo())
474+
) {
475+
return true;
476+
}
477+
return false;
478+
}
479+
451480
/**
452481
* Check if the user is this table snapshot's owner
453482
* @param snapshot the table snapshot description

‎hbase-server/src/test/java/org/apache/hadoop/hbase/io/hfile/bucket/TestVerifyBucketCacheFile.java

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -36,8 +36,8 @@
3636
import org.apache.hadoop.conf.Configuration;
3737
import org.apache.hadoop.fs.Path;
3838
import org.apache.hadoop.hbase.HBaseClassTestRule;
39-
import org.apache.hadoop.hbase.HBaseTestingUtility;
4039
import org.apache.hadoop.hbase.HBaseConfiguration;
40+
import org.apache.hadoop.hbase.HBaseTestingUtility;
4141
import org.apache.hadoop.hbase.io.hfile.BlockCacheKey;
4242
import org.apache.hadoop.hbase.io.hfile.CacheConfig;
4343
import org.apache.hadoop.hbase.io.hfile.CacheTestUtils;

‎hbase-server/src/test/java/org/apache/hadoop/hbase/security/access/TestSnapshotScannerHDFSAclController.java

Lines changed: 6 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -158,6 +158,7 @@ public void testGrantGlobal1() throws Exception {
158158

159159
TestHDFSAclHelper.createTableAndPut(TEST_UTIL, table);
160160
snapshotAndWait(snapshot1, table);
161+
snapshotAndWait(snapshot2, table);
161162
// grant G(R)
162163
SecureTestUtil.grantGlobal(TEST_UTIL, grantUserName, READ);
163164
TestHDFSAclHelper.canUserScanSnapshot(TEST_UTIL, grantUser, snapshot1, 6);
@@ -174,8 +175,6 @@ public void testGrantGlobal1() throws Exception {
174175
// grant G(R)
175176
SecureTestUtil.grantGlobal(TEST_UTIL, grantUserName, READ);
176177
TestHDFSAclHelper.canUserScanSnapshot(TEST_UTIL, grantUser, snapshot1, 6);
177-
// take a snapshot and ACLs are inherited automatically
178-
snapshotAndWait(snapshot2, table);
179178
TestHDFSAclHelper.canUserScanSnapshot(TEST_UTIL, grantUser, snapshot2, 6);
180179
assertTrue(hasUserGlobalHdfsAcl(aclTable, grantUserName));
181180
deleteTable(table);
@@ -197,10 +196,10 @@ public void testGrantGlobal2() throws Exception {
197196
// create table in namespace1 and snapshot
198197
TestHDFSAclHelper.createTableAndPut(TEST_UTIL, table1);
199198
snapshotAndWait(snapshot1, table1);
200-
admin.grant(new UserPermission(grantUserName,
201-
Permission.newBuilder(namespace1).withActions(READ).build()), false);
202199
// grant G(W)
203200
SecureTestUtil.grantGlobal(TEST_UTIL, grantUserName, WRITE);
201+
admin.grant(new UserPermission(grantUserName,
202+
Permission.newBuilder(namespace1).withActions(READ).build()), false);
204203
// create table in namespace2 and snapshot
205204
TestHDFSAclHelper.createTableAndPut(TEST_UTIL, table2);
206205
snapshotAndWait(snapshot2, table2);
@@ -231,11 +230,11 @@ public void testGrantGlobal3() throws Exception {
231230
// grant table1(R)
232231
TestHDFSAclHelper.createTableAndPut(TEST_UTIL, table1);
233232
snapshotAndWait(snapshot1, table1);
234-
TestHDFSAclHelper.grantOnTable(TEST_UTIL, grantUserName, table1, READ);
235-
// grant G(W)
236-
SecureTestUtil.grantGlobal(TEST_UTIL, grantUserName, WRITE);
237233
TestHDFSAclHelper.createTableAndPut(TEST_UTIL, table2);
238234
snapshotAndWait(snapshot2, table2);
235+
// grant G(W)
236+
SecureTestUtil.grantGlobal(TEST_UTIL, grantUserName, WRITE);
237+
TestHDFSAclHelper.grantOnTable(TEST_UTIL, grantUserName, table1, READ);
239238
// check scan snapshot
240239
TestHDFSAclHelper.canUserScanSnapshot(TEST_UTIL, grantUser, snapshot1, 6);
241240
TestHDFSAclHelper.canUserScanSnapshot(TEST_UTIL, grantUser, snapshot2, -1);

‎hbase-server/src/test/java/org/apache/hadoop/hbase/snapshot/TestSnapshotDescriptionUtils.java

Lines changed: 49 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,7 @@
2222
import static org.junit.Assert.fail;
2323

2424
import java.io.IOException;
25+
import java.net.URI;
2526
import org.apache.hadoop.conf.Configuration;
2627
import org.apache.hadoop.fs.FileSystem;
2728
import org.apache.hadoop.fs.Path;
@@ -191,4 +192,52 @@ public void testIsWithinWorkingDir() throws IOException {
191192
assertTrue(SnapshotDescriptionUtils.isWithinDefaultWorkingDir(
192193
new Path("file:" + hbsaeDir + "/.hbase-snapshot/.tmp/snapshot"), conf));
193194
}
195+
196+
@Test
197+
public void testShouldSkipRenameSnapshotDirectories() {
198+
URI workingDirURI = URI.create("/User/test1");
199+
URI rootDirURI = URI.create("hdfs:///User/test2");
200+
201+
// should skip rename if it's not the same scheme;
202+
assertTrue(
203+
SnapshotDescriptionUtils.shouldSkipRenameSnapshotDirectories(workingDirURI, rootDirURI));
204+
205+
workingDirURI = URI.create("/User/test1");
206+
rootDirURI = URI.create("file:///User/test2");
207+
assertTrue(
208+
SnapshotDescriptionUtils.shouldSkipRenameSnapshotDirectories(workingDirURI, rootDirURI));
209+
210+
// skip rename when either scheme or authority are the not same
211+
workingDirURI = URI.create("hdfs://localhost:8020/User/test1");
212+
rootDirURI = URI.create("hdfs://otherhost:8020/User/test2");
213+
assertTrue(
214+
SnapshotDescriptionUtils.shouldSkipRenameSnapshotDirectories(workingDirURI, rootDirURI));
215+
216+
workingDirURI = URI.create("file:///User/test1");
217+
rootDirURI = URI.create("hdfs://localhost:8020/User/test2");
218+
assertTrue(
219+
SnapshotDescriptionUtils.shouldSkipRenameSnapshotDirectories(workingDirURI, rootDirURI));
220+
221+
workingDirURI = URI.create("hdfs:///User/test1");
222+
rootDirURI = URI.create("hdfs:///User/test2");
223+
assertFalse(
224+
SnapshotDescriptionUtils.shouldSkipRenameSnapshotDirectories(workingDirURI, rootDirURI));
225+
226+
workingDirURI = URI.create("hdfs://localhost:8020/User/test1");
227+
rootDirURI = URI.create("hdfs://localhost:8020/User/test2");
228+
assertFalse(
229+
SnapshotDescriptionUtils.shouldSkipRenameSnapshotDirectories(workingDirURI, rootDirURI));
230+
231+
workingDirURI = URI.create("hdfs://user:password@localhost:8020/User/test1");
232+
rootDirURI = URI.create("hdfs://user:password@localhost:8020/User/test2");
233+
assertFalse(
234+
SnapshotDescriptionUtils.shouldSkipRenameSnapshotDirectories(workingDirURI, rootDirURI));
235+
236+
// skip rename when user information is not the same
237+
workingDirURI = URI.create("hdfs://user:password@localhost:8020/User/test1");
238+
rootDirURI = URI.create("hdfs://user2:password2@localhost:8020/User/test2");
239+
assertTrue(
240+
SnapshotDescriptionUtils.shouldSkipRenameSnapshotDirectories(workingDirURI, rootDirURI));
241+
}
242+
194243
}

0 commit comments

Comments
 (0)