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
