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..726595c 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 ctx, + public void postBulkLoadHFile(ObserverContext ctx, List> stagingFamilyPaths, Map> finalPaths, boolean hasLoaded) throws IOException { Configuration cfg = ctx.getEnvironment().getConfiguration(); if (!hasLoaded) { // there is no need to record state - return hasLoaded; + return; } if (finalPaths == null || !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,13 @@ 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; + return; } catch (IOException ioe) { LOG.error("Failed to get tables which have been fully backed up", ioe); - return false; + return; } } @Override 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 7ac0a7e..1bea3d4 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 @@ -958,13 +958,11 @@ 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 + * @param hasLoaded whether the bulkLoad was successful. bulkload is done by the time this hook is called. */ - default boolean postBulkLoadHFile(ObserverContext ctx, + default void postBulkLoadHFile(ObserverContext ctx, List> stagingFamilyPaths, Map> finalPaths, boolean hasLoaded) throws IOException { - return hasLoaded; } /** 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..fc80710 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 @@ -2233,34 +2233,34 @@ public class RSRpcServices implements HBaseRPCErrorHandler, } } + List> familyPaths = null; if (!request.hasBulkToken()) { // Old style bulk load. This will not be supported in future releases - List> familyPaths = new ArrayList<>(request.getFamilyPathCount()); + 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 { + } + if (region.getCoprocessorHost() != null) { + region.getCoprocessorHost().preBulkLoadHFile(familyPaths); + } + try { + if (!request.hasBulkToken()) { 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); - } + } else { + // secure bulk load + map = regionServer.secureBulkLoadManager.secureBulkLoadHFiles(region, request); + } + if (map != null) { + loaded = true; + } + } finally { + if (region.getCoprocessorHost() != null) { + region.getCoprocessorHost().postBulkLoadHFile(familyPaths, map, loaded); } - } else { - // secure bulk load - map = regionServer.secureBulkLoadManager.secureBulkLoadHFiles(region, request); } BulkLoadHFileResponse.Builder builder = BulkLoadHFileResponse.newBuilder(); - if (map != null) { - loaded = true; - } builder.setLoaded(loaded); return builder.build(); } catch (IOException ie) { 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..af70aba 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 @@ -1628,16 +1628,16 @@ public class RegionCoprocessorHost * @return the possibly modified value of hasLoaded * @throws IOException */ - public boolean postBulkLoadHFile(final List> familyPaths, + public void postBulkLoadHFile(final List> familyPaths, Map> map, boolean result) throws IOException { if (this.coprocEnvironments.isEmpty()) { - return result; + return; } - return execOperationWithResult( - new ObserverOperationWithResult(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, result); } }); } diff --git a/hbase-server/src/main/java/org/apache/hadoop/hbase/security/access/AccessController.java b/hbase-server/src/main/java/org/apache/hadoop/hbase/security/access/AccessController.java index 3ed2ee3..8b73389 100644 --- a/hbase-server/src/main/java/org/apache/hadoop/hbase/security/access/AccessController.java +++ b/hbase-server/src/main/java/org/apache/hadoop/hbase/security/access/AccessController.java @@ -2159,6 +2159,7 @@ public class AccessController implements MasterCoprocessor, RegionCoprocessor, @Override public void preBulkLoadHFile(ObserverContext ctx, List> familyPaths) throws IOException { + if (familyPaths == null) return; User user = getActiveUser(ctx); for(Pair el : familyPaths) { requirePermission(user, "preBulkLoadHFile", 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..7d54700 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,7 +567,7 @@ public class SimpleRegionObserver implements RegionCoprocessor, RegionObserver { } @Override - public boolean postBulkLoadHFile(ObserverContext ctx, + public void postBulkLoadHFile(ObserverContext ctx, List> familyPaths, Map> map, boolean hasLoaded) throws IOException { RegionCoprocessorEnvironment e = ctx.getEnvironment(); @@ -583,7 +583,6 @@ public class SimpleRegionObserver implements RegionCoprocessor, RegionObserver { assertEquals(familyPath.substring(familyPath.length()-familyName.length()-1),"/"+familyName); } ctPostBulkLoadHFile.incrementAndGet(); - return hasLoaded; } @Override