Return-Path: X-Original-To: archive-asf-public-internal@cust-asf2.ponee.io Delivered-To: archive-asf-public-internal@cust-asf2.ponee.io Received: from cust-asf.ponee.io (cust-asf.ponee.io [163.172.22.183]) by cust-asf2.ponee.io (Postfix) with ESMTP id 1F095200BC2 for ; Thu, 3 Nov 2016 00:46:38 +0100 (CET) Received: by cust-asf.ponee.io (Postfix) id 1D992160B0A; Wed, 2 Nov 2016 23:46:38 +0000 (UTC) Delivered-To: archive-asf-public@cust-asf.ponee.io Received: from mail.apache.org (hermes.apache.org [140.211.11.3]) by cust-asf.ponee.io (Postfix) with SMTP id 16636160AFB for ; Thu, 3 Nov 2016 00:46:36 +0100 (CET) Received: (qmail 98647 invoked by uid 500); 2 Nov 2016 23:46:36 -0000 Mailing-List: contact commits-help@hbase.apache.org; run by ezmlm Precedence: bulk List-Help: List-Unsubscribe: List-Post: List-Id: Reply-To: dev@hbase.apache.org Delivered-To: mailing list commits@hbase.apache.org Received: (qmail 98638 invoked by uid 99); 2 Nov 2016 23:46:36 -0000 Received: from git1-us-west.apache.org (HELO git1-us-west.apache.org) (140.211.11.23) by apache.org (qpsmtpd/0.29) with ESMTP; Wed, 02 Nov 2016 23:46:36 +0000 Received: by git1-us-west.apache.org (ASF Mail Server at git1-us-west.apache.org, from userid 33) id 0F3BEE09DE; Wed, 2 Nov 2016 23:46:36 +0000 (UTC) Content-Type: text/plain; charset="us-ascii" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit From: tedyu@apache.org To: commits@hbase.apache.org Message-Id: X-Mailer: ASF-Git Admin Mailer Subject: hbase git commit: HBASE-16978 Disable backup by default - addendum (Vladimir Rodionov) Date: Wed, 2 Nov 2016 23:46:36 +0000 (UTC) archived-at: Wed, 02 Nov 2016 23:46:38 -0000 Repository: hbase Updated Branches: refs/heads/HBASE-7912 cd41083dc -> 2d12fc2d8 HBASE-16978 Disable backup by default - addendum (Vladimir Rodionov) Project: http://git-wip-us.apache.org/repos/asf/hbase/repo Commit: http://git-wip-us.apache.org/repos/asf/hbase/commit/2d12fc2d Tree: http://git-wip-us.apache.org/repos/asf/hbase/tree/2d12fc2d Diff: http://git-wip-us.apache.org/repos/asf/hbase/diff/2d12fc2d Branch: refs/heads/HBASE-7912 Commit: 2d12fc2d8a2af401d9956b8e8dc94b4a5a75456b Parents: cd41083 Author: tedyu Authored: Wed Nov 2 16:46:25 2016 -0700 Committer: tedyu Committed: Wed Nov 2 16:46:25 2016 -0700 ---------------------------------------------------------------------- .../hbase/backup/master/BackupController.java | 13 ++++++++++--- .../hbase/backup/master/BackupLogCleaner.java | 14 ++++++-------- .../master/LogRollMasterProcedureManager.java | 11 +++++++++-- .../LogRollRegionServerProcedureManager.java | 17 +++++++++++++++++ 4 files changed, 42 insertions(+), 13 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/hbase/blob/2d12fc2d/hbase-server/src/main/java/org/apache/hadoop/hbase/backup/master/BackupController.java ---------------------------------------------------------------------- diff --git a/hbase-server/src/main/java/org/apache/hadoop/hbase/backup/master/BackupController.java b/hbase-server/src/main/java/org/apache/hadoop/hbase/backup/master/BackupController.java index f4ae35c..a1426c2 100644 --- a/hbase-server/src/main/java/org/apache/hadoop/hbase/backup/master/BackupController.java +++ b/hbase-server/src/main/java/org/apache/hadoop/hbase/backup/master/BackupController.java @@ -25,6 +25,8 @@ import org.apache.commons.logging.LogFactory; import org.apache.hadoop.hbase.HConstants; import org.apache.hadoop.hbase.HTableDescriptor; import org.apache.hadoop.hbase.TableExistsException; +import org.apache.hadoop.hbase.backup.BackupRestoreConstants; +import org.apache.hadoop.hbase.backup.impl.BackupManager; import org.apache.hadoop.hbase.backup.impl.BackupSystemTable; import org.apache.hadoop.hbase.coprocessor.BaseMasterAndRegionObserver; import org.apache.hadoop.hbase.coprocessor.MasterCoprocessorEnvironment; @@ -32,9 +34,9 @@ import org.apache.hadoop.hbase.coprocessor.ObserverContext; import org.apache.hadoop.hbase.master.MasterServices; /** - * The current implementation checks if the backup system table + * The current implementation checks if the backup system table * (hbase:backup) exists on HBasae Master startup and if it does not - - * it creates it. We need to make sure that backup system table is + * it creates it. We need to make sure that backup system table is * created under HBase user with ADMIN privileges */ public class BackupController extends BaseMasterAndRegionObserver { @@ -45,12 +47,17 @@ public class BackupController extends BaseMasterAndRegionObserver { throws IOException { // Need to create the new system table for backups (if does not exist) MasterServices master = ctx.getEnvironment().getMasterServices(); + if (!BackupManager.isBackupEnabled(master.getConfiguration())) { + LOG.error("Aborted BackupController. Backup is not enabled. Please enable '"+ + BackupRestoreConstants.BACKUP_ENABLE_KEY +"' configuration setting."); + return; + } HTableDescriptor backupHTD = BackupSystemTable.getSystemTableDescriptor(); try{ master.createTable(backupHTD, null, HConstants.NO_NONCE, HConstants.NO_NONCE); LOG.info("Created "+ BackupSystemTable.getTableNameAsString()+" table"); } catch(TableExistsException e) { LOG.info("Table "+ BackupSystemTable.getTableNameAsString() +" already exists"); - } + } } } http://git-wip-us.apache.org/repos/asf/hbase/blob/2d12fc2d/hbase-server/src/main/java/org/apache/hadoop/hbase/backup/master/BackupLogCleaner.java ---------------------------------------------------------------------- diff --git a/hbase-server/src/main/java/org/apache/hadoop/hbase/backup/master/BackupLogCleaner.java b/hbase-server/src/main/java/org/apache/hadoop/hbase/backup/master/BackupLogCleaner.java index bb9765f..dbea917 100644 --- a/hbase-server/src/main/java/org/apache/hadoop/hbase/backup/master/BackupLogCleaner.java +++ b/hbase-server/src/main/java/org/apache/hadoop/hbase/backup/master/BackupLogCleaner.java @@ -28,9 +28,9 @@ import org.apache.commons.logging.LogFactory; import org.apache.hadoop.conf.Configuration; import org.apache.hadoop.fs.FileStatus; import org.apache.hadoop.hbase.HBaseInterfaceAudience; -import org.apache.hadoop.hbase.HConstants; import org.apache.hadoop.hbase.TableNotFoundException; import org.apache.hadoop.hbase.backup.BackupRestoreConstants; +import org.apache.hadoop.hbase.backup.impl.BackupManager; import org.apache.hadoop.hbase.backup.impl.BackupSystemTable; import org.apache.hadoop.hbase.classification.InterfaceAudience; import org.apache.hadoop.hbase.classification.InterfaceStability; @@ -40,8 +40,6 @@ import org.apache.hadoop.hbase.master.HMaster; import org.apache.hadoop.hbase.master.MasterServices; import org.apache.hadoop.hbase.master.cleaner.BaseLogCleanerDelegate; -import com.google.common.annotations.VisibleForTesting; - /** * Implementation of a log cleaner that checks if a log is still scheduled for * incremental backup before deleting it when its TTL is over. @@ -79,10 +77,10 @@ public class BackupLogCleaner extends BaseLogCleanerDelegate { public Iterable getDeletableFiles(Iterable files) { // all members of this class are null if backup is disabled, // so we cannot filter the files - if (this.getConf() == null) { + if (this.getConf() == null || !BackupManager.isBackupEnabled(getConf())) { return files; } - + List list = new ArrayList(); try (final BackupSystemTable table = new BackupSystemTable(conn)) { // If we do not have recorded backup sessions @@ -95,7 +93,7 @@ public class BackupLogCleaner extends BaseLogCleanerDelegate { LOG.warn("hbase:backup is not available" + tnfe.getMessage()); return files; } - + for(FileStatus file: files){ String wal = file.getPath().toString(); boolean logInSystemTable = table.isWALFileDeletable(wal); @@ -108,7 +106,7 @@ public class BackupLogCleaner extends BaseLogCleanerDelegate { } } } - return list; + return list; } catch (IOException e) { LOG.error("Failed to get hbase:backup table, therefore will keep all files", e); // nothing to delete @@ -119,7 +117,7 @@ public class BackupLogCleaner extends BaseLogCleanerDelegate { @Override public void setConf(Configuration config) { // If backup is disabled, keep all members null - if (!config.getBoolean(BackupRestoreConstants.BACKUP_ENABLE_KEY, + if (!config.getBoolean(BackupRestoreConstants.BACKUP_ENABLE_KEY, BackupRestoreConstants.BACKUP_ENABLE_DEFAULT)) { LOG.warn("Backup is disabled - allowing all wals to be deleted"); return; http://git-wip-us.apache.org/repos/asf/hbase/blob/2d12fc2d/hbase-server/src/main/java/org/apache/hadoop/hbase/backup/master/LogRollMasterProcedureManager.java ---------------------------------------------------------------------- diff --git a/hbase-server/src/main/java/org/apache/hadoop/hbase/backup/master/LogRollMasterProcedureManager.java b/hbase-server/src/main/java/org/apache/hadoop/hbase/backup/master/LogRollMasterProcedureManager.java index a5f32bb..46e2faf 100644 --- a/hbase-server/src/main/java/org/apache/hadoop/hbase/backup/master/LogRollMasterProcedureManager.java +++ b/hbase-server/src/main/java/org/apache/hadoop/hbase/backup/master/LogRollMasterProcedureManager.java @@ -27,6 +27,8 @@ import org.apache.commons.logging.Log; import org.apache.commons.logging.LogFactory; import org.apache.hadoop.hbase.CoordinatedStateManagerFactory; import org.apache.hadoop.hbase.ServerName; +import org.apache.hadoop.hbase.backup.BackupRestoreConstants; +import org.apache.hadoop.hbase.backup.impl.BackupManager; import org.apache.hadoop.hbase.coordination.BaseCoordinatedStateManager; import org.apache.hadoop.hbase.errorhandling.ForeignException; import org.apache.hadoop.hbase.errorhandling.ForeignExceptionDispatcher; @@ -87,7 +89,12 @@ public class LogRollMasterProcedureManager extends MasterProcedureManager { @Override public void execProcedure(ProcedureDescription desc) throws IOException { - this.done = false; + + if (!BackupManager.isBackupEnabled(master.getConfiguration())) { + LOG.error("Backup is not enabled. Please enable '"+ + BackupRestoreConstants.BACKUP_ENABLE_KEY +"' configuration setting."); + return; + } // start the process on the RS ForeignExceptionDispatcher monitor = new ForeignExceptionDispatcher(desc.getInstance()); List serverNames = master.getServerManager().getOnlineServersList(); @@ -95,7 +102,7 @@ public class LogRollMasterProcedureManager extends MasterProcedureManager { for (ServerName sn : serverNames) { servers.add(sn.toString()); } - + List conf = desc.getConfigurationList(); byte[] data = new byte[0]; if(conf.size() > 0){ http://git-wip-us.apache.org/repos/asf/hbase/blob/2d12fc2d/hbase-server/src/main/java/org/apache/hadoop/hbase/backup/regionserver/LogRollRegionServerProcedureManager.java ---------------------------------------------------------------------- diff --git a/hbase-server/src/main/java/org/apache/hadoop/hbase/backup/regionserver/LogRollRegionServerProcedureManager.java b/hbase-server/src/main/java/org/apache/hadoop/hbase/backup/regionserver/LogRollRegionServerProcedureManager.java index afdd438..5887d21 100644 --- a/hbase-server/src/main/java/org/apache/hadoop/hbase/backup/regionserver/LogRollRegionServerProcedureManager.java +++ b/hbase-server/src/main/java/org/apache/hadoop/hbase/backup/regionserver/LogRollRegionServerProcedureManager.java @@ -26,6 +26,8 @@ import org.apache.commons.logging.Log; import org.apache.commons.logging.LogFactory; import org.apache.hadoop.conf.Configuration; import org.apache.hadoop.hbase.CoordinatedStateManagerFactory; +import org.apache.hadoop.hbase.backup.BackupRestoreConstants; +import org.apache.hadoop.hbase.backup.impl.BackupManager; import org.apache.hadoop.hbase.backup.master.LogRollMasterProcedureManager; import org.apache.hadoop.hbase.coordination.BaseCoordinatedStateManager; import org.apache.hadoop.hbase.errorhandling.ForeignExceptionDispatcher; @@ -70,6 +72,7 @@ public class LogRollRegionServerProcedureManager extends RegionServerProcedureMa private RegionServerServices rss; private ProcedureMemberRpcs memberRpcs; private ProcedureMember member; + private boolean started = false; /** * Create a default backup procedure manager @@ -82,7 +85,13 @@ public class LogRollRegionServerProcedureManager extends RegionServerProcedureMa */ @Override public void start() { + if (!BackupManager.isBackupEnabled(rss.getConfiguration())) { + LOG.error("Backup is not enabled. Please enable '"+ + BackupRestoreConstants.BACKUP_ENABLE_KEY +"' configuration setting."); + return; + } this.memberRpcs.start(rss.getServerName().toString(), member); + this.started = true; LOG.info("Started region server backup manager."); } @@ -93,6 +102,9 @@ public class LogRollRegionServerProcedureManager extends RegionServerProcedureMa */ @Override public void stop(boolean force) throws IOException { + if (!started) { + return; + } String mode = force ? "abruptly" : "gracefully"; LOG.info("Stopping RegionServerBackupManager " + mode + "."); @@ -143,6 +155,11 @@ public class LogRollRegionServerProcedureManager extends RegionServerProcedureMa @Override public void initialize(RegionServerServices rss) throws KeeperException { this.rss = rss; + if (!BackupManager.isBackupEnabled(rss.getConfiguration())) { + LOG.error("Initialization skipped. Backup is not enabled. Please enable '"+ + BackupRestoreConstants.BACKUP_ENABLE_KEY +"' configuration setting."); + return; + } BaseCoordinatedStateManager coordManager = (BaseCoordinatedStateManager) CoordinatedStateManagerFactory.getCoordinatedStateManager(rss .getConfiguration());