Skip to content

Commit 66aedca

Browse files
taklwutaklwu
committed
HBASE-27495 Improve HFileLinkCleaner to validate back reference links ahead the next traverse (#4887)
Signed-off-by: Peter Somogyi <[email protected]> Signed-off-by: Wellington Ramos Chevreuil <[email protected]>
1 parent daa39a4 commit 66aedca

2 files changed

Lines changed: 105 additions & 42 deletions

File tree

‎hbase-server/src/main/java/org/apache/hadoop/hbase/master/cleaner/HFileLinkCleaner.java‎

Lines changed: 16 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -90,10 +90,23 @@ public boolean isFileDeletable(FileStatus fStat) {
9090
}
9191

9292
// HFile is deletable only if has no links
93-
Path backRefDir = null;
93+
Path backRefDir = HFileLink.getBackReferencesDir(parentDir, filePath.getName());
9494
try {
95-
backRefDir = HFileLink.getBackReferencesDir(parentDir, filePath.getName());
96-
return CommonFSUtils.listStatus(fs, backRefDir) == null;
95+
FileStatus[] fileStatuses = CommonFSUtils.listStatus(fs, backRefDir);
96+
// for empty reference directory, retain the logic to be deletable
97+
if (fileStatuses == null) {
98+
return true;
99+
}
100+
// reuse the found back reference files, check if the forward reference exists.
101+
// with this optimization, the chore could save one round compute time if we're visiting
102+
// the archive HFile earlier than the HFile Link
103+
for (FileStatus fileStatus : fileStatuses) {
104+
if (!isFileDeletable(fileStatus)) {
105+
return false;
106+
}
107+
}
108+
// all the found back reference files are clear, we can delete it.
109+
return true;
97110
} catch (IOException e) {
98111
if (LOG.isDebugEnabled()) {
99112
LOG.debug("Couldn't get the references, not deleting file, just in case. filePath="

‎hbase-server/src/test/java/org/apache/hadoop/hbase/master/cleaner/TestHFileLinkCleaner.java‎

Lines changed: 89 additions & 39 deletions
Original file line numberDiff line numberDiff line change
@@ -43,7 +43,9 @@
4343
import org.apache.hadoop.hbase.util.CommonFSUtils;
4444
import org.apache.hadoop.hbase.util.HFileArchiveUtil;
4545
import org.apache.hadoop.hbase.zookeeper.ZKWatcher;
46+
import org.junit.After;
4647
import org.junit.AfterClass;
48+
import org.junit.Before;
4749
import org.junit.BeforeClass;
4850
import org.junit.ClassRule;
4951
import org.junit.Rule;
@@ -61,9 +63,28 @@ public class TestHFileLinkCleaner {
6163
public static final HBaseClassTestRule CLASS_RULE =
6264
HBaseClassTestRule.forClass(TestHFileLinkCleaner.class);
6365

66+
private Configuration conf;
67+
private Path rootDir;
68+
private FileSystem fs;
69+
private TableName tableName;
70+
private TableName tableLinkName;
71+
private String hfileName;
72+
private String familyName;
73+
private RegionInfo hri;
74+
private RegionInfo hriLink;
75+
private Path archiveDir;
76+
private Path archiveStoreDir;
77+
private Path familyPath;
78+
private Path hfilePath;
79+
private Path familyLinkPath;
80+
private String hfileLinkName;
81+
private Path linkBackRefDir;
82+
private Path linkBackRef;
83+
private FileStatus[] backRefs;
84+
private HFileCleaner cleaner;
6485
private final static HBaseTestingUtility TEST_UTIL = new HBaseTestingUtility();
65-
6686
private static DirScanPool POOL;
87+
private static final long TTL = 1000;
6788

6889
@Rule
6990
public TestName name = new TestName();
@@ -78,49 +99,71 @@ public static void tearDown() {
7899
POOL.shutdownNow();
79100
}
80101

81-
@Test
82-
public void testHFileLinkCleaning() throws Exception {
83-
Configuration conf = TEST_UTIL.getConfiguration();
102+
@Before
103+
public void configureDirectoriesAndLinks() throws IOException {
104+
conf = TEST_UTIL.getConfiguration();
84105
CommonFSUtils.setRootDir(conf, TEST_UTIL.getDataTestDir());
85106
conf.set(HFileCleaner.MASTER_HFILE_CLEANER_PLUGINS, HFileLinkCleaner.class.getName());
86-
Path rootDir = CommonFSUtils.getRootDir(conf);
87-
FileSystem fs = FileSystem.get(conf);
107+
rootDir = CommonFSUtils.getRootDir(conf);
108+
fs = FileSystem.get(conf);
88109

89-
final TableName tableName = TableName.valueOf(name.getMethodName());
90-
final TableName tableLinkName = TableName.valueOf(name.getMethodName() + "-link");
91-
final String hfileName = "1234567890";
92-
final String familyName = "cf";
110+
tableName = TableName.valueOf(name.getMethodName());
111+
tableLinkName = TableName.valueOf(name.getMethodName() + "-link");
112+
hfileName = "1234567890";
113+
familyName = "cf";
93114

94-
RegionInfo hri = RegionInfoBuilder.newBuilder(tableName).build();
95-
RegionInfo hriLink = RegionInfoBuilder.newBuilder(tableLinkName).build();
115+
hri = RegionInfoBuilder.newBuilder(tableName).build();
116+
hriLink = RegionInfoBuilder.newBuilder(tableLinkName).build();
96117

97-
Path archiveDir = HFileArchiveUtil.getArchivePath(conf);
98-
Path archiveStoreDir =
118+
archiveDir = HFileArchiveUtil.getArchivePath(conf);
119+
archiveStoreDir =
99120
HFileArchiveUtil.getStoreArchivePath(conf, tableName, hri.getEncodedName(), familyName);
100121

101122
// Create hfile /hbase/table-link/region/cf/getEncodedName.HFILE(conf);
102-
Path familyPath = getFamilyDirPath(archiveDir, tableName, hri.getEncodedName(), familyName);
123+
familyPath = getFamilyDirPath(archiveDir, tableName, hri.getEncodedName(), familyName);
103124
fs.mkdirs(familyPath);
104-
Path hfilePath = new Path(familyPath, hfileName);
125+
hfilePath = new Path(familyPath, hfileName);
105126
fs.createNewFile(hfilePath);
106127

128+
createLink(true);
129+
130+
// Initialize cleaner
131+
conf.setLong(TimeToLiveHFileCleaner.TTL_CONF_KEY, TTL);
132+
Server server = new DummyServer();
133+
cleaner = new HFileCleaner(1000, server, conf, fs, archiveDir, POOL);
134+
}
135+
136+
private void createLink(boolean createBackReference) throws IOException {
107137
// Create link to hfile
108-
Path familyLinkPath =
109-
getFamilyDirPath(rootDir, tableLinkName, hriLink.getEncodedName(), familyName);
138+
familyLinkPath = getFamilyDirPath(rootDir, tableLinkName, hriLink.getEncodedName(), familyName);
110139
fs.mkdirs(familyLinkPath);
111-
HFileLink.create(conf, fs, familyLinkPath, hri, hfileName);
112-
Path linkBackRefDir = HFileLink.getBackReferencesDir(archiveStoreDir, hfileName);
140+
hfileLinkName = HFileLink.create(conf, fs, familyLinkPath, hri, hfileName, createBackReference);
141+
linkBackRefDir = HFileLink.getBackReferencesDir(archiveStoreDir, hfileName);
113142
assertTrue(fs.exists(linkBackRefDir));
114-
FileStatus[] backRefs = fs.listStatus(linkBackRefDir);
143+
backRefs = fs.listStatus(linkBackRefDir);
115144
assertEquals(1, backRefs.length);
116-
Path linkBackRef = backRefs[0].getPath();
145+
linkBackRef = backRefs[0].getPath();
146+
}
117147

118-
// Initialize cleaner
119-
final long ttl = 1000;
120-
conf.setLong(TimeToLiveHFileCleaner.TTL_CONF_KEY, ttl);
121-
Server server = new DummyServer();
122-
HFileCleaner cleaner = new HFileCleaner(1000, server, conf, fs, archiveDir, POOL);
148+
@After
149+
public void cleanup() throws IOException, InterruptedException {
150+
// HFile can be removed
151+
Thread.sleep(TTL * 2);
152+
cleaner.chore();
153+
assertFalse("HFile should be deleted", fs.exists(hfilePath));
154+
// Remove everything
155+
for (int i = 0; i < 4; ++i) {
156+
Thread.sleep(TTL * 2);
157+
cleaner.chore();
158+
}
159+
assertFalse("HFile should be deleted",
160+
fs.exists(CommonFSUtils.getTableDir(archiveDir, tableName)));
161+
assertFalse("Link should be deleted",
162+
fs.exists(CommonFSUtils.getTableDir(archiveDir, tableLinkName)));
163+
}
123164

165+
@Test
166+
public void testHFileLinkCleaning() throws Exception {
124167
// Link backref cannot be removed
125168
cleaner.chore();
126169
assertTrue(fs.exists(linkBackRef));
@@ -131,21 +174,28 @@ public void testHFileLinkCleaning() throws Exception {
131174
CommonFSUtils.getTableDir(archiveDir, tableLinkName));
132175
cleaner.chore();
133176
assertFalse("Link should be deleted", fs.exists(linkBackRef));
177+
}
134178

135-
// HFile can be removed
136-
Thread.sleep(ttl * 2);
179+
@Test
180+
public void testHFileLinkByRemovingReference() throws Exception {
181+
// Link backref cannot be removed
137182
cleaner.chore();
138-
assertFalse("HFile should be deleted", fs.exists(hfilePath));
183+
assertTrue(fs.exists(linkBackRef));
184+
assertTrue(fs.exists(hfilePath));
139185

140-
// Remove everything
141-
for (int i = 0; i < 4; ++i) {
142-
Thread.sleep(ttl * 2);
143-
cleaner.chore();
144-
}
145-
assertFalse("HFile should be deleted",
146-
fs.exists(CommonFSUtils.getTableDir(archiveDir, tableName)));
147-
assertFalse("Link should be deleted",
148-
fs.exists(CommonFSUtils.getTableDir(archiveDir, tableLinkName)));
186+
// simulate after removing the reference in data directory, the Link backref can be removed
187+
fs.delete(new Path(familyLinkPath, hfileLinkName), false);
188+
cleaner.chore();
189+
assertFalse("Link should be deleted", fs.exists(linkBackRef));
190+
}
191+
192+
@Test
193+
public void testHFileLinkEmptyBackReferenceDirectory() throws Exception {
194+
// simulate and remove the back reference
195+
fs.delete(linkBackRef, false);
196+
assertTrue("back reference directory still exists", fs.exists(linkBackRefDir));
197+
cleaner.chore();
198+
assertFalse("back reference directory should be deleted", fs.exists(linkBackRefDir));
149199
}
150200

151201
private static Path getFamilyDirPath(final Path rootDir, final TableName table,

0 commit comments

Comments
 (0)