You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@hbase.apache.org by te...@apache.org on 2017/12/06 15:06:36 UTC
hbase git commit: HBASE-19417 Remove boolean return value from
postBulkLoadHFile hook
Repository: hbase
Updated Branches:
refs/heads/master ebd8841e0 -> 27ed4d8ad
HBASE-19417 Remove boolean return value from postBulkLoadHFile hook
Project: http://git-wip-us.apache.org/repos/asf/hbase/repo
Commit: http://git-wip-us.apache.org/repos/asf/hbase/commit/27ed4d8a
Tree: http://git-wip-us.apache.org/repos/asf/hbase/tree/27ed4d8a
Diff: http://git-wip-us.apache.org/repos/asf/hbase/diff/27ed4d8a
Branch: refs/heads/master
Commit: 27ed4d8add20c5424943eee54acff266567e765a
Parents: ebd8841
Author: tedyu <yu...@gmail.com>
Authored: Wed Dec 6 07:06:28 2017 -0800
Committer: tedyu <yu...@gmail.com>
Committed: Wed Dec 6 07:06:28 2017 -0800
----------------------------------------------------------------------
.../hadoop/hbase/backup/BackupObserver.java | 18 ++++++++----------
.../hbase/coprocessor/RegionObserver.java | 11 +++++------
.../hbase/regionserver/RSRpcServices.java | 20 ++++++--------------
.../regionserver/RegionCoprocessorHost.java | 16 +++++++---------
.../regionserver/SecureBulkLoadManager.java | 6 +-----
.../hbase/coprocessor/SimpleRegionObserver.java | 5 ++---
6 files changed, 29 insertions(+), 47 deletions(-)
----------------------------------------------------------------------
http://git-wip-us.apache.org/repos/asf/hbase/blob/27ed4d8a/hbase-backup/src/main/java/org/apache/hadoop/hbase/backup/BackupObserver.java
----------------------------------------------------------------------
diff --git a/hbase-backup/src/main/java/org/apache/hadoop/hbase/backup/BackupObserver.java b/hbase-backup/src/main/java/org/apache/hadoop/hbase/backup/BackupObserver.java
index e2b27ff..1d8f780 100644
--- a/hbase-backup/src/main/java/org/apache/hadoop/hbase/backup/BackupObserver.java
+++ b/hbase-backup/src/main/java/org/apache/hadoop/hbase/backup/BackupObserver.java
@@ -54,17 +54,17 @@ public class BackupObserver implements RegionCoprocessor, RegionObserver {
}
@Override
- public boolean postBulkLoadHFile(ObserverContext<RegionCoprocessorEnvironment> ctx,
- List<Pair<byte[], String>> stagingFamilyPaths, Map<byte[], List<Path>> finalPaths,
- boolean hasLoaded) throws IOException {
+ public void postBulkLoadHFile(ObserverContext<RegionCoprocessorEnvironment> ctx,
+ List<Pair<byte[], String>> stagingFamilyPaths, Map<byte[], List<Path>> finalPaths)
+ throws IOException {
Configuration cfg = ctx.getEnvironment().getConfiguration();
- if (!hasLoaded) {
+ if (finalPaths == null) {
// there is no need to record state
- return hasLoaded;
+ return;
}
- if (finalPaths == null || !BackupManager.isBackupEnabled(cfg)) {
+ if (!BackupManager.isBackupEnabled(cfg)) {
LOG.debug("skipping recording bulk load in postBulkLoadHFile since backup is disabled");
- return hasLoaded;
+ return;
}
try (Connection connection = ConnectionFactory.createConnection(cfg);
BackupSystemTable tbl = new BackupSystemTable(connection)) {
@@ -75,13 +75,11 @@ public class BackupObserver implements RegionCoprocessor, RegionObserver {
if (LOG.isTraceEnabled()) {
LOG.trace(tableName + " has not gone thru full backup");
}
- return hasLoaded;
+ return;
}
tbl.writePathsPostBulkLoad(tableName, info.getEncodedNameAsBytes(), finalPaths);
- return hasLoaded;
} catch (IOException ioe) {
LOG.error("Failed to get tables which have been fully backed up", ioe);
- return false;
}
}
@Override
http://git-wip-us.apache.org/repos/asf/hbase/blob/27ed4d8a/hbase-server/src/main/java/org/apache/hadoop/hbase/coprocessor/RegionObserver.java
----------------------------------------------------------------------
diff --git a/hbase-server/src/main/java/org/apache/hadoop/hbase/coprocessor/RegionObserver.java b/hbase-server/src/main/java/org/apache/hadoop/hbase/coprocessor/RegionObserver.java
index e036441..6b5527b 100644
--- a/hbase-server/src/main/java/org/apache/hadoop/hbase/coprocessor/RegionObserver.java
+++ b/hbase-server/src/main/java/org/apache/hadoop/hbase/coprocessor/RegionObserver.java
@@ -974,13 +974,12 @@ public interface RegionObserver {
* @param ctx the environment provided by the region server
* @param stagingFamilyPaths pairs of { CF, HFile path } submitted for bulk load
* @param finalPaths Map of CF to List of file paths for the loaded files
- * @param hasLoaded whether the bulkLoad was successful
- * @return the new value of hasLoaded
+ * if the Map is not null, the bulkLoad was successful. Otherwise the bulk load failed.
+ * bulkload is done by the time this hook is called.
*/
- default boolean postBulkLoadHFile(ObserverContext<RegionCoprocessorEnvironment> ctx,
- List<Pair<byte[], String>> stagingFamilyPaths, Map<byte[], List<Path>> finalPaths,
- boolean hasLoaded) throws IOException {
- return hasLoaded;
+ default void postBulkLoadHFile(ObserverContext<RegionCoprocessorEnvironment> ctx,
+ List<Pair<byte[], String>> stagingFamilyPaths, Map<byte[], List<Path>> finalPaths)
+ throws IOException {
}
/**
http://git-wip-us.apache.org/repos/asf/hbase/blob/27ed4d8a/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/RSRpcServices.java
----------------------------------------------------------------------
diff --git a/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/RSRpcServices.java b/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/RSRpcServices.java
index 58e2970..f5a35a4 100644
--- a/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/RSRpcServices.java
+++ b/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/RSRpcServices.java
@@ -2214,7 +2214,6 @@ public class RSRpcServices implements HBaseRPCErrorHandler,
checkOpen();
requestCount.increment();
HRegion region = getRegion(request.getRegion());
- boolean loaded = false;
Map<byte[], List<Path>> map = null;
// Check to see if this bulk load would exceed the space quota for this table
@@ -2233,24 +2232,20 @@ public class RSRpcServices implements HBaseRPCErrorHandler,
}
}
+ List<Pair<byte[], String>> familyPaths = new ArrayList<>(request.getFamilyPathCount());
+ for (FamilyPath familyPath : request.getFamilyPathList()) {
+ familyPaths.add(new Pair<>(familyPath.getFamily().toByteArray(), familyPath.getPath()));
+ }
if (!request.hasBulkToken()) {
- // Old style bulk load. This will not be supported in future releases
- List<Pair<byte[], String>> familyPaths = new ArrayList<>(request.getFamilyPathCount());
- for (FamilyPath familyPath : request.getFamilyPathList()) {
- familyPaths.add(new Pair<>(familyPath.getFamily().toByteArray(), familyPath.getPath()));
- }
if (region.getCoprocessorHost() != null) {
region.getCoprocessorHost().preBulkLoadHFile(familyPaths);
}
try {
map = region.bulkLoadHFiles(familyPaths, request.getAssignSeqNum(), null,
request.getCopyFile());
- if (map != null) {
- loaded = true;
- }
} finally {
if (region.getCoprocessorHost() != null) {
- loaded = region.getCoprocessorHost().postBulkLoadHFile(familyPaths, map, loaded);
+ region.getCoprocessorHost().postBulkLoadHFile(familyPaths, map);
}
}
} else {
@@ -2258,10 +2253,7 @@ public class RSRpcServices implements HBaseRPCErrorHandler,
map = regionServer.secureBulkLoadManager.secureBulkLoadHFiles(region, request);
}
BulkLoadHFileResponse.Builder builder = BulkLoadHFileResponse.newBuilder();
- if (map != null) {
- loaded = true;
- }
- builder.setLoaded(loaded);
+ builder.setLoaded(map != null);
return builder.build();
} catch (IOException ie) {
throw new ServiceException(ie);
http://git-wip-us.apache.org/repos/asf/hbase/blob/27ed4d8a/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/RegionCoprocessorHost.java
----------------------------------------------------------------------
diff --git a/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/RegionCoprocessorHost.java b/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/RegionCoprocessorHost.java
index 2188300..0448451 100644
--- a/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/RegionCoprocessorHost.java
+++ b/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/RegionCoprocessorHost.java
@@ -1624,20 +1624,18 @@ public class RegionCoprocessorHost
/**
* @param familyPaths pairs of { CF, file path } submitted for bulk load
* @param map Map of CF to List of file paths for the final loaded files
- * @param result whether load was successful or not
- * @return the possibly modified value of hasLoaded
* @throws IOException
*/
- public boolean postBulkLoadHFile(final List<Pair<byte[], String>> familyPaths,
- Map<byte[], List<Path>> map, boolean result) throws IOException {
+ public void postBulkLoadHFile(final List<Pair<byte[], String>> familyPaths,
+ Map<byte[], List<Path>> map) throws IOException {
if (this.coprocEnvironments.isEmpty()) {
- return result;
+ return;
}
- return execOperationWithResult(
- new ObserverOperationWithResult<RegionObserver, Boolean>(regionObserverGetter, result) {
+ execOperation(coprocEnvironments.isEmpty()? null:
+ new RegionObserverOperationWithoutResult() {
@Override
- public Boolean call(RegionObserver observer) throws IOException {
- return observer.postBulkLoadHFile(this, familyPaths, map, getResult());
+ public void call(RegionObserver observer) throws IOException {
+ observer.postBulkLoadHFile(this, familyPaths, map);
}
});
}
http://git-wip-us.apache.org/repos/asf/hbase/blob/27ed4d8a/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/SecureBulkLoadManager.java
----------------------------------------------------------------------
diff --git a/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/SecureBulkLoadManager.java b/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/SecureBulkLoadManager.java
index 6ce44fe..89847f9 100644
--- a/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/SecureBulkLoadManager.java
+++ b/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/SecureBulkLoadManager.java
@@ -195,7 +195,6 @@ public class SecureBulkLoadManager {
if (region.getCoprocessorHost() != null) {
region.getCoprocessorHost().preBulkLoadHFile(familyPaths);
}
- boolean loaded = false;
Map<byte[], List<Path>> map = null;
try {
@@ -238,12 +237,9 @@ public class SecureBulkLoadManager {
return null;
}
});
- if (map != null) {
- loaded = true;
- }
} finally {
if (region.getCoprocessorHost() != null) {
- region.getCoprocessorHost().postBulkLoadHFile(familyPaths, map, loaded);
+ region.getCoprocessorHost().postBulkLoadHFile(familyPaths, map);
}
}
return map;
http://git-wip-us.apache.org/repos/asf/hbase/blob/27ed4d8a/hbase-server/src/test/java/org/apache/hadoop/hbase/coprocessor/SimpleRegionObserver.java
----------------------------------------------------------------------
diff --git a/hbase-server/src/test/java/org/apache/hadoop/hbase/coprocessor/SimpleRegionObserver.java b/hbase-server/src/test/java/org/apache/hadoop/hbase/coprocessor/SimpleRegionObserver.java
index 47113d8..1394dbd 100644
--- a/hbase-server/src/test/java/org/apache/hadoop/hbase/coprocessor/SimpleRegionObserver.java
+++ b/hbase-server/src/test/java/org/apache/hadoop/hbase/coprocessor/SimpleRegionObserver.java
@@ -567,8 +567,8 @@ public class SimpleRegionObserver implements RegionCoprocessor, RegionObserver {
}
@Override
- public boolean postBulkLoadHFile(ObserverContext<RegionCoprocessorEnvironment> ctx,
- List<Pair<byte[], String>> familyPaths, Map<byte[], List<Path>> map, boolean hasLoaded)
+ public void postBulkLoadHFile(ObserverContext<RegionCoprocessorEnvironment> ctx,
+ List<Pair<byte[], String>> familyPaths, Map<byte[], List<Path>> map)
throws IOException {
RegionCoprocessorEnvironment e = ctx.getEnvironment();
assertNotNull(e);
@@ -583,7 +583,6 @@ public class SimpleRegionObserver implements RegionCoprocessor, RegionObserver {
assertEquals(familyPath.substring(familyPath.length()-familyName.length()-1),"/"+familyName);
}
ctPostBulkLoadHFile.incrementAndGet();
- return hasLoaded;
}
@Override