HBASE-27201 Clean up error-prone findings in hbase-backup

hbase-backup/src/main/java/org/apache/hadoop/hbase/backup/BackupInfo.java

@@ -37,7 +37,6 @@
 
 import org.apache.hadoop.hbase.shaded.protobuf.ProtobufUtil;
 import org.apache.hadoop.hbase.shaded.protobuf.generated.BackupProtos;
-import org.apache.hadoop.hbase.shaded.protobuf.generated.BackupProtos.BackupInfo.Builder;
 
 /**
  * An object to encapsulate the information for each backup session
@@ -451,13 +450,13 @@ public byte[] toByteArray() throws IOException {
     return toProtosBackupInfo().toByteArray();
   }
 
-  private void setBackupTableInfoMap(Builder builder) {
+  private void setBackupTableInfoMap(BackupProtos.BackupInfo.Builder builder) {
     for (Entry<TableName, BackupTableInfo> entry : backupTableInfoMap.entrySet()) {
       builder.addBackupTableInfo(entry.getValue().toProto());
     }
   }
 
-  private void setTableSetTimestampMap(Builder builder) {
+  private void setTableSetTimestampMap(BackupProtos.BackupInfo.Builder builder) {
     if (this.getTableSetTimestampMap() != null) {
       for (Entry<TableName, Map<String, Long>> entry : this.getTableSetTimestampMap().entrySet()) {
         builder.putTableSetTimestamp(entry.getKey().getNameAsString(),
@@ -531,10 +530,9 @@ public String getShortDescription() {
     sb.append("Type=" + getType()).append(",");
     sb.append("Tables=" + getTableListAsString()).append(",");
     sb.append("State=" + getState()).append(",");
-    Date date = null;
     Calendar cal = Calendar.getInstance();
     cal.setTimeInMillis(getStartTs());
-    date = cal.getTime();
+    Date date = cal.getTime();
     sb.append("Start time=" + date).append(",");
     if (state == BackupState.FAILED) {
       sb.append("Failed message=" + getFailedMsg()).append(",");
@@ -560,7 +558,7 @@ public String getStatusAndProgressAsString() {
   }
 
   public String getTableListAsString() {
-    StringBuffer sb = new StringBuffer();
+    StringBuilder sb = new StringBuilder();
     sb.append("{");
     sb.append(StringUtils.join(backupTableInfoMap.keySet(), ","));
     sb.append("}");

hbase-backup/src/main/java/org/apache/hadoop/hbase/backup/RestoreDriver.java

@@ -83,7 +83,7 @@ protected void init() {
     Log4jUtils.disableZkAndClientLoggers();
   }
 
-  private int parseAndRun(String[] args) throws IOException {
+  private int parseAndRun() throws IOException {
     // Check if backup is enabled
     if (!BackupManager.isBackupEnabled(getConf())) {
       System.err.println(BackupRestoreConstants.ENABLE_BACKUP);
@@ -146,7 +146,7 @@ private int parseAndRun(String[] args) throws IOException {
       if (cmd.hasOption(OPTION_SET)) {
         String setName = cmd.getOptionValue(OPTION_SET);
         try {
-          tables = getTablesForSet(conn, setName, conf);
+          tables = getTablesForSet(conn, setName);
         } catch (IOException e) {
           System.out.println("ERROR: " + e.getMessage() + " for setName=" + setName);
           printToolUsage();
@@ -182,8 +182,7 @@ private int parseAndRun(String[] args) throws IOException {
     return 0;
   }
 
-  private String getTablesForSet(Connection conn, String name, Configuration conf)
-    throws IOException {
+  private String getTablesForSet(Connection conn, String name) throws IOException {
     try (final BackupSystemTable table = new BackupSystemTable(conn)) {
       List<TableName> tables = table.describeBackupSet(name);
 
@@ -214,7 +213,7 @@ protected void processOptions(CommandLine cmd) {
 
   @Override
   protected int doWork() throws Exception {
-    return parseAndRun(cmd.getArgs());
+    return parseAndRun();
   }
 
   public static void main(String[] args) throws Exception {

hbase-backup/src/main/java/org/apache/hadoop/hbase/backup/impl/BackupCommands.java

@@ -310,7 +310,7 @@ public void execute() throws IOException {
       String setName = null;
       if (cmdline.hasOption(OPTION_SET)) {
         setName = cmdline.getOptionValue(OPTION_SET);
-        tables = getTablesForSet(setName, getConf());
+        tables = getTablesForSet(setName);
 
         if (tables == null) {
           System.out
@@ -371,7 +371,7 @@ private boolean verifyPath(String path) {
       }
     }
 
-    private String getTablesForSet(String name, Configuration conf) throws IOException {
+    private String getTablesForSet(String name) throws IOException {
       try (final BackupSystemTable table = new BackupSystemTable(conn)) {
         List<TableName> tables = table.describeBackupSet(name);
 
@@ -1001,14 +1001,14 @@ public void execute() throws IOException {
           processSetDescribe(args);
           break;
         case SET_LIST:
-          processSetList(args);
+          processSetList();
           break;
         default:
           break;
       }
     }
 
-    private void processSetList(String[] args) throws IOException {
+    private void processSetList() throws IOException {
       super.execute();
 
       // List all backup set names
@@ -1081,13 +1081,13 @@ private TableName[] toTableNames(String[] tables) {
       return arr;
     }
 
+    @SuppressWarnings("StringSplitter")
     private void processSetAdd(String[] args) throws IOException {
       if (args == null || args.length != 4) {
         printUsage();
         throw new IOException(INCORRECT_USAGE);
       }
       super.execute();
-
       String setName = args[2];
       String[] tables = args[3].split(",");
       TableName[] tableNames = new TableName[tables.length];

hbase-backup/src/main/java/org/apache/hadoop/hbase/backup/impl/BackupManifest.java

@@ -366,7 +366,6 @@ private void setIncrTimeRanges(Map<TableName, Map<String, Long>> incrTimeRanges)
   }
 
   // backup image directory
-  private String tableBackupDir = null;
   private BackupImage backupImage;
 
   /**
@@ -385,7 +384,6 @@ public BackupManifest(BackupInfo backup) {
    * @param backup The ongoing backup session info
    */
   public BackupManifest(BackupInfo backup, TableName table) {
-    this.tableBackupDir = backup.getTableBackupDir(table);
     List<TableName> tables = new ArrayList<TableName>();
     tables.add(table);
     BackupImage.Builder builder = BackupImage.newBuilder();
@@ -470,7 +468,7 @@ public List<TableName> getTableList() {
    * TODO: fix it. Persist the manifest file.
    * @throws IOException IOException when storing the manifest file.
    */
-  public void store(Configuration conf) throws BackupException {
+  public void store(Configuration conf) throws IOException {
     byte[] data = backupImage.toProto().toByteArray();
     // write the file, overwrite if already exist
     Path manifestFilePath =
@@ -479,10 +477,7 @@ public void store(Configuration conf) throws BackupException {
     try (FSDataOutputStream out =
       manifestFilePath.getFileSystem(conf).create(manifestFilePath, true)) {
       out.write(data);
-    } catch (IOException e) {
-      throw new BackupException(e.getMessage());
     }
-
     LOG.info("Manifest file stored to " + manifestFilePath);
   }
 
@@ -526,7 +521,7 @@ public ArrayList<BackupImage> getRestoreDependentList(boolean reverse) {
       restoreImages.put(Long.valueOf(image.startTs), image);
     }
     return new ArrayList<>(
-      reverse ? (restoreImages.descendingMap().values()) : (restoreImages.values()));
+      reverse ? restoreImages.descendingMap().values() : restoreImages.values());
   }
 
   /**

hbase-backup/src/main/java/org/apache/hadoop/hbase/backup/impl/BackupSystemTable.java

@@ -19,6 +19,8 @@
 
 import java.io.Closeable;
 import java.io.IOException;
+import java.io.InterruptedIOException;
+import java.nio.charset.StandardCharsets;
 import java.util.ArrayList;
 import java.util.Arrays;
 import java.util.Collections;
@@ -237,6 +239,7 @@ private void waitForSystemTable(Admin admin, TableName tableName) throws IOExcep
       try {
         Thread.sleep(100);
       } catch (InterruptedException e) {
+        throw (IOException) new InterruptedIOException().initCause(e);
       }
       if (EnvironmentEdgeManager.currentTime() - startTime > TIMEOUT) {
         throw new IOException(
@@ -574,7 +577,7 @@ public String readBackupStartCode(String backupRoot) throws IOException {
       if (val.length == 0) {
         return null;
       }
-      return new String(val);
+      return new String(val, StandardCharsets.UTF_8);
     }
   }
 
@@ -1639,7 +1642,7 @@ public String[] getListOfBackupIdsFromDeleteOperation() throws IOException {
       if (val.length == 0) {
         return null;
       }
-      return new String(val).split(",");
+      return new String(val, StandardCharsets.UTF_8).split(",");
     }
   }
 
@@ -1654,7 +1657,7 @@ public boolean isMergeInProgress() throws IOException {
     Get get = new Get(MERGE_OP_ROW);
     try (Table table = connection.getTable(tableName)) {
       Result res = table.get(get);
-      return (!res.isEmpty());
+      return !res.isEmpty();
     }
   }
 
@@ -1720,7 +1723,7 @@ public String[] getListOfBackupIdsFromMergeOperation() throws IOException {
       if (val.length == 0) {
         return null;
       }
-      return new String(val).split(",");
+      return new String(val, StandardCharsets.UTF_8).split(",");
     }
   }
 
@@ -1736,11 +1739,13 @@ static Scan createScanForOrigBulkLoadedFiles(TableName table) {
     return scan;
   }
 
+  @SuppressWarnings("StringSplitter")
   static String getTableNameFromOrigBulkLoadRow(String rowStr) {
     String[] parts = rowStr.split(BLK_LD_DELIM);
     return parts[1];
   }
 
+  @SuppressWarnings("StringSplitter")
   static String getRegionNameFromOrigBulkLoadRow(String rowStr) {
     // format is bulk : namespace : table : region : file
     String[] parts = rowStr.split(BLK_LD_DELIM);

hbase-backup/src/main/java/org/apache/hadoop/hbase/backup/impl/IncrementalTableBackupClient.java

@@ -204,8 +204,7 @@ private void copyBulkLoadedFiles(List<String> activeFiles, List<String> archiveF
       String tgtDest = backupInfo.getBackupRootDir() + Path.SEPARATOR + backupInfo.getBackupId();
       int attempt = 1;
       while (activeFiles.size() > 0) {
-        LOG.info(
-          "Copy " + activeFiles.size() + " active bulk loaded files. Attempt =" + (attempt++));
+        LOG.info("Copy " + activeFiles.size() + " active bulk loaded files. Attempt =" + attempt++);
         String[] toCopy = new String[activeFiles.size()];
         activeFiles.toArray(toCopy);
         // Active file can be archived during copy operation,

hbase-backup/src/main/java/org/apache/hadoop/hbase/backup/impl/RestoreTablesClient.java

@@ -181,7 +181,7 @@ private void restoreImages(BackupImage[] images, TableName sTable, TableName tTa
 
   private List<Path> getFilesRecursively(String fileBackupDir)
     throws IllegalArgumentException, IOException {
-    FileSystem fs = FileSystem.get((new Path(fileBackupDir)).toUri(), new Configuration());
+    FileSystem fs = FileSystem.get(new Path(fileBackupDir).toUri(), new Configuration());
     List<Path> list = new ArrayList<>();
     RemoteIterator<LocatedFileStatus> it = fs.listFiles(new Path(fileBackupDir), true);
     while (it.hasNext()) {

hbase-backup/src/main/java/org/apache/hadoop/hbase/backup/impl/TableBackupClient.java

@@ -322,7 +322,7 @@ protected void addManifest(BackupInfo backupInfo, BackupManager backupManager, B
    * @return meta data dir
    */
   protected String obtainBackupMetaDataStr(BackupInfo backupInfo) {
-    StringBuffer sb = new StringBuffer();
+    StringBuilder sb = new StringBuilder();
     sb.append("type=" + backupInfo.getType() + ",tablelist=");
     for (TableName table : backupInfo.getTables()) {
       sb.append(table + ";");

hbase-backup/src/main/java/org/apache/hadoop/hbase/backup/mapreduce/MapReduceBackupCopyJob.java

@@ -47,7 +47,6 @@
 import org.apache.hadoop.tools.DistCpConstants;
 import org.apache.hadoop.tools.DistCpOptions;
 import org.apache.yetus.audience.InterfaceAudience;
-import org.apache.zookeeper.KeeperException.NoNodeException;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
 
@@ -127,7 +126,6 @@ public BackupInfo getBackupInfo() {
    * @param backupInfo  backup info
    * @param newProgress progress
    * @param bytesCopied bytes copied
-   * @throws NoNodeException exception
    */
   static void updateProgress(BackupInfo backupInfo, BackupManager backupManager, int newProgress,
     long bytesCopied) throws IOException {
@@ -361,7 +359,6 @@ private SequenceFile.Writer getWriter(Path pathToListFile) throws IOException {
    * @param conf     The hadoop configuration
    * @param copyType The backup copy type
    * @param options  Options for customized ExportSnapshot or DistCp
-   * @throws Exception exception
    */
   @Override
   public int copy(BackupInfo context, BackupManager backupManager, Configuration conf,

hbase-backup/src/main/java/org/apache/hadoop/hbase/backup/mapreduce/MapReduceBackupMergeJob.java

@@ -22,11 +22,11 @@
 import java.io.IOException;
 import java.io.InputStream;
 import java.io.OutputStream;
+import java.util.ArrayDeque;
 import java.util.ArrayList;
 import java.util.HashSet;
 import java.util.List;
 import java.util.Set;
-import java.util.Stack;
 import org.apache.commons.io.IOUtils;
 import org.apache.commons.lang3.StringUtils;
 import org.apache.hadoop.conf.Configuration;
@@ -257,7 +257,7 @@ protected void copyFile(FileSystem fs, Path p, Path newPath) throws IOException
    */
   protected Path convertToDest(Path p, Path backupDirPath) {
     String backupId = backupDirPath.getName();
-    Stack<String> stack = new Stack<String>();
+    ArrayDeque<String> stack = new ArrayDeque<String>();
     String name = null;
     while (true) {
       name = p.getName();

hbase-backup/src/main/java/org/apache/hadoop/hbase/backup/regionserver/LogRollBackupSubprocedure.java

@@ -17,6 +17,7 @@
  */
 package org.apache.hadoop.hbase.backup.regionserver;
 
+import java.nio.charset.StandardCharsets;
 import java.util.HashMap;
 import java.util.List;
 import java.util.concurrent.Callable;
@@ -56,7 +57,7 @@ public LogRollBackupSubprocedure(RegionServerServices rss, ProcedureMember membe
     this.rss = rss;
     this.taskManager = taskManager;
     if (data != null) {
-      backupRoot = new String(data);
+      backupRoot = new String(data, StandardCharsets.UTF_8);
     }
   }
 

hbase-backup/src/main/java/org/apache/hadoop/hbase/backup/util/BackupUtils.java

@@ -68,7 +68,7 @@
  */
 @InterfaceAudience.Private
 public final class BackupUtils {
-  protected static final Logger LOG = LoggerFactory.getLogger(BackupUtils.class);
+  private static final Logger LOG = LoggerFactory.getLogger(BackupUtils.class);
   public static final String LOGNAME_SEPARATOR = ".";
   public static final int MILLISEC_IN_HOUR = 3600000;
 
@@ -136,9 +136,10 @@ public static void copyTableRegionInfo(Connection conn, BackupInfo backupInfo, C
         // write a copy of descriptor to the target directory
         Path target = new Path(backupInfo.getTableBackupDir(table));
         FileSystem targetFs = target.getFileSystem(conf);
-        FSTableDescriptors descriptors =
-          new FSTableDescriptors(targetFs, CommonFSUtils.getRootDir(conf));
-        descriptors.createTableDescriptorForTableDirectory(target, orig, false);
+        try (FSTableDescriptors descriptors =
+          new FSTableDescriptors(targetFs, CommonFSUtils.getRootDir(conf))) {
+          descriptors.createTableDescriptorForTableDirectory(target, orig, false);
+        }
         LOG.debug("Attempting to copy table info for:" + table + " target: " + target
           + " descriptor: " + orig);
         LOG.debug("Finished copying tableinfo.");
@@ -275,12 +276,12 @@ public static List<String> getWALFilesOlderThan(final Configuration c,
     return logFiles;
   }
 
+  @SuppressWarnings("StringSplitter")
   public static TableName[] parseTableNames(String tables) {
     if (tables == null) {
       return null;
     }
     String[] tableArray = tables.split(BackupRestoreConstants.TABLENAME_DELIMITER_IN_COMMAND);
-
     TableName[] ret = new TableName[tableArray.length];
     for (int i = 0; i < tableArray.length; i++) {
       ret[i] = TableName.valueOf(tableArray[i]);
@@ -593,6 +594,7 @@ public int compare(BackupInfo o1, BackupInfo o2) {
         return ts1 < ts2 ? 1 : -1;
       }
 
+      @SuppressWarnings("StringSplitter")
       private long getTimestamp(String backupId) {
         String[] split = backupId.split("_");
         return Long.parseLong(split[1]);
@@ -731,6 +733,7 @@ public static BulkLoadHFiles createLoader(Configuration config) {
     return BulkLoadHFiles.create(conf);
   }
 
+  @SuppressWarnings("StringSplitter")
   public static String findMostRecentBackupId(String[] backupIds) {
     long recentTimestamp = Long.MIN_VALUE;
     for (String backupId : backupIds) {