Apache9 commented on a change in pull request #2738:
URL: https://github.com/apache/hbase/pull/2738#discussion_r537548207
##########
File path:
hbase-server/src/main/java/org/apache/hadoop/hbase/master/cleaner/HFileLinkCleaner.java
##########
@@ -44,71 +45,79 @@
private static final Logger LOG =
LoggerFactory.getLogger(HFileLinkCleaner.class);
private FileSystem fs = null;
+ private ReentrantReadWriteLock lock = new ReentrantReadWriteLock();
@Override
- public synchronized boolean isFileDeletable(FileStatus fStat) {
- if (this.fs == null) return false;
- Path filePath = fStat.getPath();
- // HFile Link is always deletable
- if (HFileLink.isHFileLink(filePath)) return true;
+ public boolean isFileDeletable(FileStatus fStat) {
+ try {
+ lock.readLock().lock();
+ if (this.fs == null) return false;
+ Path filePath = fStat.getPath();
+ // HFile Link is always deletable
+ if (HFileLink.isHFileLink(filePath)) return true;
Review comment:
Ditto.
##########
File path:
hbase-server/src/main/java/org/apache/hadoop/hbase/master/cleaner/HFileLinkCleaner.java
##########
@@ -44,71 +45,79 @@
private static final Logger LOG =
LoggerFactory.getLogger(HFileLinkCleaner.class);
private FileSystem fs = null;
+ private ReentrantReadWriteLock lock = new ReentrantReadWriteLock();
@Override
- public synchronized boolean isFileDeletable(FileStatus fStat) {
- if (this.fs == null) return false;
- Path filePath = fStat.getPath();
- // HFile Link is always deletable
- if (HFileLink.isHFileLink(filePath)) return true;
+ public boolean isFileDeletable(FileStatus fStat) {
+ try {
+ lock.readLock().lock();
+ if (this.fs == null) return false;
+ Path filePath = fStat.getPath();
+ // HFile Link is always deletable
+ if (HFileLink.isHFileLink(filePath)) return true;
- // If the file is inside a link references directory, means that it is a
back ref link.
- // The back ref can be deleted only if the referenced file doesn't exists.
- Path parentDir = filePath.getParent();
- if (HFileLink.isBackReferencesDir(parentDir)) {
- Path hfilePath = null;
- try {
- // Also check if the HFile is in the HBASE_TEMP_DIRECTORY; this is
where the referenced
- // file gets created when cloning a snapshot.
- hfilePath = HFileLink.getHFileFromBackReference(
- new Path(CommonFSUtils.getRootDir(getConf()),
HConstants.HBASE_TEMP_DIRECTORY), filePath);
- if (fs.exists(hfilePath)) {
- return false;
- }
- // check whether the HFileLink still exists in mob dir.
- hfilePath =
HFileLink.getHFileFromBackReference(MobUtils.getMobHome(getConf()), filePath);
- if (fs.exists(hfilePath)) {
+ // If the file is inside a link references directory, means that it is a
back ref link.
+ // The back ref can be deleted only if the referenced file doesn't
exists.
+ Path parentDir = filePath.getParent();
+ if (HFileLink.isBackReferencesDir(parentDir)) {
+ Path hfilePath = null;
+ try {
+ // Also check if the HFile is in the HBASE_TEMP_DIRECTORY; this is
where the referenced
+ // file gets created when cloning a snapshot.
+ hfilePath = HFileLink.getHFileFromBackReference(new
Path(CommonFSUtils.getRootDir(getConf()), HConstants.HBASE_TEMP_DIRECTORY),
+ filePath);
+ if (fs.exists(hfilePath)) {
+ return false;
+ }
+ // check whether the HFileLink still exists in mob dir.
+ hfilePath =
HFileLink.getHFileFromBackReference(MobUtils.getMobHome(getConf()), filePath);
+ if (fs.exists(hfilePath)) {
+ return false;
+ }
+ hfilePath =
HFileLink.getHFileFromBackReference(CommonFSUtils.getRootDir(getConf()),
filePath);
+ return !fs.exists(hfilePath);
+ } catch (IOException e) {
+ if (LOG.isDebugEnabled()) {
+ LOG.debug("Couldn't verify if the referenced file still exists,
keep it just in case: "
+ + hfilePath);
+ }
return false;
}
- hfilePath =
-
HFileLink.getHFileFromBackReference(CommonFSUtils.getRootDir(getConf()),
filePath);
- return !fs.exists(hfilePath);
+ }
+
+ // HFile is deletable only if has no links
+ Path backRefDir = null;
+ try {
+ backRefDir = HFileLink.getBackReferencesDir(parentDir,
filePath.getName());
+ return CommonFSUtils.listStatus(fs, backRefDir) == null;
} catch (IOException e) {
if (LOG.isDebugEnabled()) {
- LOG.debug("Couldn't verify if the referenced file still exists, keep
it just in case: " +
- hfilePath);
+ LOG.debug(
+ "Couldn't get the references, not deleting file, just in case.
filePath=" + filePath + ", backRefDir=" + backRefDir);
}
return false;
}
- }
-
- // HFile is deletable only if has no links
- Path backRefDir = null;
- try {
- backRefDir = HFileLink.getBackReferencesDir(parentDir,
filePath.getName());
- return CommonFSUtils.listStatus(fs, backRefDir) == null;
- } catch (IOException e) {
- if (LOG.isDebugEnabled()) {
- LOG.debug("Couldn't get the references, not deleting file, just in
case. filePath="
- + filePath + ", backRefDir=" + backRefDir);
- }
- return false;
+ } finally {
+ lock.readLock().unlock();
}
}
@Override
- public synchronized void setConf(Configuration conf) {
+ public void setConf(Configuration conf) {
super.setConf(conf);
// setup filesystem
try {
+ lock.writeLock().lock();
Review comment:
Better move this line before the try block.
##########
File path:
hbase-server/src/main/java/org/apache/hadoop/hbase/master/cleaner/HFileLinkCleaner.java
##########
@@ -44,71 +45,79 @@
private static final Logger LOG =
LoggerFactory.getLogger(HFileLinkCleaner.class);
private FileSystem fs = null;
+ private ReentrantReadWriteLock lock = new ReentrantReadWriteLock();
@Override
- public synchronized boolean isFileDeletable(FileStatus fStat) {
- if (this.fs == null) return false;
- Path filePath = fStat.getPath();
- // HFile Link is always deletable
- if (HFileLink.isHFileLink(filePath)) return true;
+ public boolean isFileDeletable(FileStatus fStat) {
+ try {
+ lock.readLock().lock();
+ if (this.fs == null) return false;
Review comment:
nits: add '{}' to hold the 'return false;' block to fix a checkstyle
warning.
##########
File path:
hbase-server/src/main/java/org/apache/hadoop/hbase/master/cleaner/HFileLinkCleaner.java
##########
@@ -44,71 +45,79 @@
private static final Logger LOG =
LoggerFactory.getLogger(HFileLinkCleaner.class);
private FileSystem fs = null;
+ private ReentrantReadWriteLock lock = new ReentrantReadWriteLock();
@Override
- public synchronized boolean isFileDeletable(FileStatus fStat) {
- if (this.fs == null) return false;
- Path filePath = fStat.getPath();
- // HFile Link is always deletable
- if (HFileLink.isHFileLink(filePath)) return true;
+ public boolean isFileDeletable(FileStatus fStat) {
+ try {
+ lock.readLock().lock();
Review comment:
Usually we will put this line before the try block.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
[email protected]
