Repository: hbase Updated Branches: refs/heads/branch-2 57df73ac6 -> 497902731
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/49790273 Tree: http://git-wip-us.apache.org/repos/asf/hbase/tree/49790273 Diff: http://git-wip-us.apache.org/repos/asf/hbase/diff/49790273 Branch: refs/heads/branch-2 Commit: 497902731a5364b1f209b4f8ebba15604bcf1b3a Parents: 57df73a Author: tedyu <[email protected]> Authored: Wed Dec 6 14:23:22 2017 -0800 Committer: tedyu <[email protected]> Committed: Wed Dec 6 14:23:22 2017 -0800 ---------------------------------------------------------------------- .../hbase/coprocessor/RegionObserver.java | 11 +++++------ .../hbase/regionserver/RSRpcServices.java | 20 ++++++-------------- .../regionserver/RegionCoprocessorHost.java | 16 +++++++--------- .../regionserver/SecureBulkLoadManager.java | 6 +----- .../hbase/coprocessor/SimpleRegionObserver.java | 5 ++--- 5 files changed, 21 insertions(+), 37 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/hbase/blob/49790273/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/49790273/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/49790273/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/49790273/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/49790273/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
