guluo2016 commented on PR #5303:
URL: https://github.com/apache/hbase/pull/5303#issuecomment-1605879025

   > Maybe at the caller place, we have already checked whether compaction is 
enabled at table level?
   
   Yes, we indeed check checked whether compaction is enabled, for example [The 
code](https://github.com/apache/hbase/blob/master/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/HRegion.java#L7433-L7438)
 :
   ```
   if (this.rsServices != null && store.needsCompaction()) {
       this.rsServices.getCompactionRequestor().requestSystemCompaction(this, 
store,
                 "bulkload hfiles request compaction", true);
        LOG.info("Request compaction for region {} family {} after bulk load",
              this.getRegionInfo().getEncodedName(), 
store.getColumnFamilyName());
   }
   ```
   We will check whether compaction is enabled by calling ` 
this.rsServices.getCompactionRequestor().requestSystemCompaction` after calling 
`store.needsCompaction`
   
   > Where do we call this method?  
   
   And this method would be called in these place.
   
![2023-06-25_131837](https://github.com/apache/hbase/assets/19660320/a1cfb144-56c8-42f1-9672-72d010865f23)
   
   
   
   In here, I mean, since the main function of  `needsCompaction()` is  to 
check whether to need compaction, so even if we would check whether compaction 
after calling this method, I still think it is not right to return true for a 
disabled comapction table.
   
   


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