saintstack commented on a change in pull request #1737: URL: https://github.com/apache/hbase/pull/1737#discussion_r445315094
########## File path: hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/LogRoller.java ########## @@ -58,8 +60,8 @@ protected void scheduleFlush(String encodedRegionName) { encodedRegionName, r); return; } - // force flushing all stores to clean old logs - requester.requestFlush(r, true, FlushLifeCycleTracker.DUMMY); + // force flushing specified stores to clean old logs + requester.requestFlush(r, false, families, FlushLifeCycleTracker.DUMMY); Review comment: The flag is set when we are in a critical condition -- the WAL count is in excess of our WAL limit. The flag's intent IIRC is that we flush all stores regardless of what determination is made at flush time as to which stores are in need of flush or not; the old edit may actually be hanging out in a store that is small and not in need of flush normally or in accordance w/ some flush policy. The flag says 'force' the flush. My understanding is that this is a FlushRequest usually but the flag changes the request to a demand. Here you are passing a set of stores instead where the stores chosen are the ones that will free up WALs. So, this flag is now redundant? We should purge it. Now it will confuse since the 'force' instead is a list of families to flush -- else its null if old behavior. ---------------------------------------------------------------- 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: us...@infra.apache.org