[GitHub] [hbase] busbey commented on a change in pull request #910: HBASE-23378: Clean Up FSUtil setClusterId

[GitBox <gi...@apache.org>]

busbey commented on a change in pull request #910: HBASE-23378: Clean Up FSUtil setClusterId
URL: https://github.com/apache/hbase/pull/910#discussion_r357854662
 
 

 ##########
 File path: hbase-server/src/main/java/org/apache/hadoop/hbase/util/FSUtils.java
 ##########
 @@ -635,53 +636,55 @@ private static void rewriteAsPb(final FileSystem fs, final Path rootdir, final P
   }
 
   /**
-   * Writes a new unique identifier for this cluster to the "hbase.id" file
-   * in the HBase root directory
+   * Writes a new unique identifier for this cluster to the "hbase.id" file in the HBase root
+   * directory.
    * @param fs the root directory FileSystem
    * @param rootdir the path to the HBase root directory
    * @param clusterId the unique identifier to store
    * @param wait how long (in milliseconds) to wait between retries
    * @throws IOException if writing to the FileSystem fails and no wait value
    */
-  public static void setClusterId(FileSystem fs, Path rootdir, ClusterId clusterId,
-      int wait) throws IOException {
+  public static void setClusterId(final FileSystem fs, final Path rootdir,
+      final ClusterId clusterId, final long wait) throws IOException {
+
+    final Path idFile = new Path(rootdir, HConstants.CLUSTER_ID_FILE_NAME);
+    final Path tempDir = new Path(rootdir, HConstants.HBASE_TEMP_DIRECTORY);
+    final Path tempIdFile = new Path(tempDir, HConstants.CLUSTER_ID_FILE_NAME);
+
+    LOG.debug("Create cluster ID file [{}] with ID: {}", idFile, clusterId);
+
     while (true) {
-      try {
-        Path idFile = new Path(rootdir, HConstants.CLUSTER_ID_FILE_NAME);
-        Path tempIdFile = new Path(rootdir, HConstants.HBASE_TEMP_DIRECTORY +
-          Path.SEPARATOR + HConstants.CLUSTER_ID_FILE_NAME);
-        // Write the id file to a temporary location
-        FSDataOutputStream s = fs.create(tempIdFile);
-        try {
-          s.write(clusterId.toByteArray());
-          s.close();
-          s = null;
-          // Move the temporary file to its normal location. Throw an IOE if
-          // the rename failed
-          if (!fs.rename(tempIdFile, idFile)) {
-            throw new IOException("Unable to move temp version file to " + idFile);
-          }
-        } finally {
-          // Attempt to close the stream if still open on the way out
-          try {
-            if (s != null) s.close();
-          } catch (IOException ignore) { }
-        }
-        if (LOG.isDebugEnabled()) {
-          LOG.debug("Created cluster ID file at " + idFile.toString() + " with ID: " + clusterId);
+      Optional<IOException> failure = Optional.empty();
+
+      LOG.debug("Write the cluster ID file to a temporary location: {}", tempIdFile);
+      try (FSDataOutputStream s = fs.create(tempIdFile)) {
+        s.write(clusterId.toByteArray());
+
+        LOG.debug("Move the temporary cluster ID file to its target location [{}]:[{}]", tempIdFile,
+          idFile);
+        if (!fs.rename(tempIdFile, idFile)) {
 
 Review comment:
   you can't do this rename before closing `s`

----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services