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:
[email protected]


With regards,
Apache Git Services

Reply via email to