wchevreuil commented on a change in pull request #856: HBASE-21776: Avoid 
duplicate calls to setStoragePolicy which shows up in debug Logging
URL: https://github.com/apache/hbase/pull/856#discussion_r351271834
 
 

 ##########
 File path: 
hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/HRegionFileSystem.java
 ##########
 @@ -189,8 +189,14 @@ Path createStoreDir(final String familyName) throws 
IOException {
    * @param familyName The name of column family.
    * @param policyName The name of the storage policy
    */
-  public void setStoragePolicy(String familyName, String policyName) {
-    FSUtils.setStoragePolicy(this.fs, getStoreDir(familyName), policyName);
+  public void setStoragePolicy(String familyName, String policyName) throws 
IOException {
+    if (this.fs instanceof HFileSystem) {
 
 Review comment:
   > it's not just the duplicate traces but to avoid the duplicate useless 
function calls.
   
   Well, it's a tradeoff indeed. I would still favour code isolation from 
specific impl logic, over the single additional call. Here we are making the 
code a bit tighter, with one more place to be changed in the event of a change 
at _HFileSystem_, for example.  If we agree to get this merged though, can we 
at least make sure we have tests for the two _if_ conditions now?

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