I've started an adjacent VOTE thread in an attempt at clarity of how-to-go-forward here. Thanks, S
On Tue, Nov 17, 2020 at 7:56 AM Andrew Purtell <[email protected]> wrote: > Hi Duo, > > Just to be clear: You are saying go ahead with the merge, but then also go > back and start this discussion fresh, to see if anything was missed and > more can be done? > > > On Nov 16, 2020, at 11:25 PM, 张铎 <[email protected]> wrote: > > > > Oh, this is my fault. I mean the old behavior IS to go to primary > replica > > first, which is what we want to change here. > > > > And what I commented on jira, is to say that we do not need to get a > > performance improvement before merging, it is not the goal of this issue. > > And I suggested that if we want to show our advantage, we need to get the > > primary replica fucked up. I do not know why then the discussion went to > > the HedgeRead and I could not poll it back. I do not think this should > > block the merging but even though it was still very hard to communicate, > so > > I assumed this means we still have a big gap on what we want to solve > here, > > thus I voted a -1 here. > > > > I think we need to go back to the beginning, to reach an agreement on the > > goal here. Let’s review the design doc again to see if we missed > something > > which lead us to this situation. > > > > And I need to say that, I do not want to block the issue to be merged. I > > tried my best to speed up the process. I suggested to land the changes at > > client side to master directly but was refused. I helped to add scan on > > specific replica feature soon on branch-2 to let the port to branch-2 can > > be landed cleanly. > > > > On a mobile device so can not review the code or PR. Very busy these > days. > > And the health examination this morning told me that I had a high blood > > pressure. Not a good birthday present. Will get back to the issue when > > possible. > > > > Thanks. > > > > Stack <[email protected]>于2020年11月17日 周二06:34写道： > > > >>> On Sun, Nov 15, 2020 at 11:20 PM 张铎(Duo Zhang) <[email protected]> > >>> wrote: > >>> > >>> So what is your purpose of distributing the request of region location > >>> lookup? It is just because you want to 'distribute the request of > region > >>> location lookup'? > >>> > >>> Then I'm -1 on merging. We should reach an agreement on what we want to > >>> solve before merging at least. > >>> > >>> > >> HERE.1 > >> > >> > >>> I've helped this issue from the design doc step. For me, the purpose > for > >>> this issue is clear. We want to prevent the hotspot of meta, so the > >>> solution is simple, enable meta replica, and then just modify the > client > >> to > >>> not always go to primary replica first(this is the old behavior even > with > >>> meta replica feature on). > >>> And this will introduce another problem that, there is no meta region > >>> replication implementation for meta read replicas, which means the > >> latency > >>> will be large as we can only sync the data between replicas through > >> region > >>> flush, so we implement meta region replication. > >>> > >>> So I think it is very important to verify that we have truly > distributed > >>> the request of region location lookup, and also make sure that we could > >>> support more requests of region location lookup. Otherwise this feature > >> is > >>> useless. > >>> > >>> And I agree with Andrew that, since the feature is default off on > >> branch-2 > >>> and has no regression, it is OK to merge for now. Theoretically our > >>> approach here should work, so even it does not work for now, I think we > >>> could fix the problems to make it work. > >>> > >>> > >> HERE.2 > >> > >> I agree with all of the above between HERE.1 and HERE.2 (except the > >> suggestion that the old behavior of read replicas is that they went to > the > >> replica first; they go to the primary first -- see [1], [2]). > >> > >> Lets work with any misalignment of understanding/communication offline > and > >> not in the way of merge. > >> > >> Thanks, > >> S > >> > >> 1. http://hbase.apache.org/book.html#_timeline_consistency "In case a > read > >> is performed with Consistency.TIMELINE, then the read RPC will be sent > to > >> the primary region server first." > >> 2. > >> > >> > https://github.com/apache/hbase/blob/branch-2/hbase-client/src/main/java/org/apache/hadoop/hbase/client/ScannerCallableWithReplicas.java#L195 > >> > >> > >> > >>> But your reply above made me wonder whether we are talking about the > same > >>> thing. That's why I'm -1 here. I'm not going to force you to do the > test > >>> suggested by me, as I said it could be done after merging, just want to > >>> reach an agreement on the goal of this feature. > >>> > >>> Thanks. > >>> > >>> Stack <[email protected]> 于2020年11月16日周一 下午12:35写道： > >>> > >>>> On Sun, Nov 15, 2020 at 9:16 AM Andrew Purtell < > >> [email protected] > >>>> > >>>> wrote: > >>>> > >>>>> I agree with Duo’s comment that a performance gain is unlikely but > >>> would > >>>>> be orthogonal anyway; > >>>> > >>>> > >>>> Perf observation is just an aside in the issue. Perf is orthogonal as > >> you > >>>> say above (as long as no regression). > >>>> > >>>> > >>>> > >>>>> it’s an availability gain that is the goal. We can assume it based on > >>>>> theory of operation and unit test results but the gain should be > >> tested > >>>> and > >>>>> measured on a cluster too. > >>>>> > >>>> > >>>> > >>>> The feature is about distributing load on hbase:meta to alleviate > >>>> hotspotting; it makes read replicas more live so replicas are more > >> likely > >>>> to satisfy location lookups making read replicas more effective. That > >>> read > >>>> replicas improve HA is presumed -- it was the original justification > >> for > >>>> this years old commit -- but HA is not the focus of this addition; > >> hence > >>> no > >>>> reports on effectiveness in this area. > >>>> > >>>> I have no problem working on such tests/reports but suggest that they > >> are > >>>> done post merge. > >>>> > >>>> > >>>> > >>>>> That said, the results of the testing thus far indicate no > >> regression, > >>>>> which gives me confidence to support a merge. Specifically, a merge > >> to > >>>>> “unblock” 2.4 (we aren’t really blocked, we are waiting), provided > >> the > >>>>> default there is the feature is configured off. But please indicate > >> in > >>>>> documentation and release notes that the feature is not widely tested > >>>> yet - > >>>>> as is customarily done for new functionality like this. > >>>>> > >>>>> > >>>> No problem w/ flagging the feature as new. > >>>> > >>>> Thanks, > >>>> S > >>>> > >>>> > >>>> > >>>>> > >>>>>> On Nov 15, 2020, at 5:20 AM, 张铎 <[email protected]> wrote: > >>>>>> > >>>>>> Replied on jira, I think we missed an important scenario when > >>> testing. > >>>>>> > >>>>>> Thanks. > >>>>>> > >>>>>> Stack <[email protected]> 于2020年11月15日周日 上午2:30写道： > >>>>>> > >>>>>>> HBASE-18070 makes it so hbase:meta read replicas can run closer to > >>> the > >>>>>>> primary, (< second lags rather than minutes). It adds Async WAL > >>>>>>> Replication[1] on the hbase:meta table; i.e. edits are sprayed > >>> across > >>>>>>> replicas as they arrive at the primary's WAL. Before this work, > >>> Async > >>>>> WAL > >>>>>>> Replication was only available on user-space tables and the only > >>>> option > >>>>> for > >>>>>>> hbase:meta read replicas was reloading the primaries hfiles on a > >>>> period > >>>>>>> (minutes). HBASE-18070 also adds an optional client-side > >>> 'LoadBalance' > >>>>>>> policy that favors read replicas ahead of primary reads falling > >> back > >>>> to > >>>>> the > >>>>>>> primary on fault. Together, these additions allow distributing > >>>>> hbase:meta > >>>>>>> read load across primary and replicas alleviating 'hotspotting'. > >>>>>>> > >>>>>>> I would like to merge the feature to master branch Monday evening > >> if > >>>>> there > >>>>>>> is no objection (Soon after I'll merge to branch-2 so this feature > >>> can > >>>>>>> hopefully be included in the upcoming 2.4.0RC). > >>>>>>> > >>>>>>> * For the design, see [2]. > >>>>>>> * For an amalgamated PR of the 5 or 6 reviewed PRs that comprise > >>> this > >>>>>>> feature, see [3]. > >>>>>>> * For a PE report that compared performance before and after, see > >>>>>>> HBASE-25127 (no regression). > >>>>>>> * A report on ITBLL runs is pending to be attached to HBASE-18070 > >>> but > >>>>> runs > >>>>>>> so far show no regression with the feature enabled (ITBLL runs > >> were > >>>> done > >>>>>>> against a backport of this feature to branch-2 as the ITBLL state > >> of > >>>>> master > >>>>>>> is currently an unknown). > >>>>>>> > >>>>>>> Testing continues mainly looking for further improvement and to > >>> better > >>>>>>> understand this feature in operation. Documentation is included > >> but > >>> in > >>>>> need > >>>>>>> of polish (working on it). > >>>>>>> > >>>>>>> Dump any questions here and I'll be happy to respond. If you need > >>> more > >>>>> time > >>>>>>> to review, just shout. > >>>>>>> > >>>>>>> Thanks and thanks to all who contributed to this feature; the > >>>> reviewers > >>>>> and > >>>>>>> the testers in particular. > >>>>>>> > >>>>>>> S > >>>>>>> > >>>>>>> 1. http://hbase.apache.org/book.html#_asnyc_wal_replication > >>>>>>> 2. > >>>>>>> > >>>>>>> > >>>>> > >>>> > >>> > >> > https://docs.google.com/document/d/1jJWVc-idHhhgL4KDRpjMsQJKCl_NRaCLGiH3Wqwd3O8/edit# > >>>>>>> This patch is currently missing HBASE-25280, a bug found in > >> testing. > >>>>>>> 3. https://github.com/apache/hbase/pull/2643 > >>>>>>> > >>>>> > >>>> > >>> > >> >
