GeorryHuang commented on a change in pull request #4174:
URL: https://github.com/apache/hbase/pull/4174#discussion_r824448585



##########
File path: 
hbase-server/src/main/java/org/apache/hadoop/hbase/wal/FSHLogProvider.java
##########
@@ -90,13 +90,6 @@ public static Writer createWriter(final Configuration conf, 
final FileSystem fs,
       } else {
         LOG.debug("Error instantiating log writer.", e);
       }
-      if (writer != null) {

Review comment:
       Must we remove close logic here? I do agree @Apache9 's suggestion that 
we should close the writer inside initialization logic, but IMHO we should keep 
this close logic since the Writer is an interface. Maybe developers would 
implement a new XXXXWriter in the future and miss the close logic.
   
   what about this:
   
   in the specific writer:
   ```
      if (this.output != null) {
         try {
           this.output.close();
              this.output=null
         } catch (IOException e) {
           LOG.warn("Close output failed", e);
         }
       }
   ```
   
   then the real close() do nothing because `this.output` is null.
   
   I'm not very familiar with code here, please correct me if I'm wrong.
   
   
   




-- 
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.

To unsubscribe, e-mail: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]


Reply via email to