Apache9 commented on PR #4863: URL: https://github.com/apache/hbase/pull/4863#issuecomment-1324474438
> > So the fix here is to create the new replication source after terminating the old replication source. > > First, how could this fix the problem? Second, I guess why we create the replication source outside the lock is to reduce the locking time, but anyway, IIRC the start up of a replication source is asynchronous, so probably it is OK to move it into the lock protection. > > Thanks for reviewing Duo @Apache9 > > The problem arises like this > > 1. Imagine we have a replication source A whose sizeOfLogQueue is x, and the global sizeOfLogQueue is y. > 2. When disable the peer, we will create a new replication source A', and enqueue the log queue, that makes the sizeOfLogQueue of A' also grows to x. > https://github.com/apache/hbase/blob/master/hbase-server/src/main/java/org/apache/hadoop/hbase/replication/regionserver/ReplicationSourceManager.java#L473 > 3. To avoid the metric being cleared, we introduce a flag to keep the old metric. See [HBASE-23231](https://issues.apache.org/jira/browse/HBASE-23231) for more details. > As a result, the global sizeOfLogQueue is x+y, however actually it should still be y. > 4. In the first commit, I only decrease the global sizeOfLogQueue when terminate the old replication source, but @shahrs87 point out that maybe there are metrics should also be reset, for example the sizeOfHFileRefsQueue. > > So I think maybe we can change the order, terminate old replication first and then create the new one. That will make the problem less complicated. OK, so if we terminate the replication source first, we do not need to keep the old metrics, just let it decrease and after we create the new replication source, the metrics will be restored? -- 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]
