Hi Jingsong, Thanks for your feedback!
Yes, the existing HBase connector use an option 'lookup.async' to control its lookup source implementations that exposed to the planner, however it's a private option for the HBase connector only, so it will not affect the common API. And as discussed in the mailing thread of FLIP-221[1], we got a consensus that do not make it as a common option. It's better making decisions at the query level when a connector has both capabilities. So if everything goes well, we should discuss it whether to deprecate the 'lookup.async' or not for HBase connector after the hint been done. This will be mentioned in the Compatibility part of this FLIP[2]. WDYT? [1]: https://lists.apache.org/thread/v76g8v1o9sjdho9kbzlgjyv38l2oynox [2]: https://cwiki.apache.org/confluence/display/FLINK/FLIP-234%3A+Support+Retryable+Lookup+Join+To+Solve+Delayed+Updates+Issue+In+External+Systems?src=contextnavpagetreemode Best, Lincoln Lee Jingsong Li <jingsongl...@gmail.com> 于2022年6月1日周三 14:11写道： > Hi Lincoln, > > The unified lookup hint is what I want. > > And I like 'async'='true|false' option. > > But there is a compatibility issue, as I remember if async is currently > controlled by connector, and this may also require some API changes? > > We need to have a clear story for the connector combined with this option: > - only sync connector > - only async connector > - both async and sync connector > > Best, > Jingsong > > On Mon, May 30, 2022 at 3:55 PM Lincoln Lee <lincoln.8...@gmail.com> > wrote: > > > Thanks Jark for your quick response and the consensus! > > > > And I will update the FLIP after Jingsong or other developers confirm > that > > there is no problem. > > > > Best, > > Lincoln Lee > > > > > > Jark Wu <imj...@gmail.com> 于2022年5月30日周一 15:49写道： > > > > > Thanks for the update. > > > > > > The unified lookup hint looks good to me. > > > And thanks for explaining ALLOW_UNORDERED. > > > > > > Best, > > > Jark > > > > > > On Mon, 30 May 2022 at 15:31, Lincoln Lee <lincoln.8...@gmail.com> > > wrote: > > > > > > > Hi Jark & Jingsong, > > > > > > > > Thanks for your feedback! > > > > > > > > 1.) support retry on sync lookup > > > > I also agree with supporting it, this will be useful for connectors > > that > > > > don't have asynchronous lookup implementations and can also solve the > > > ASYNC > > > > non-target problem to some extent(because the retrying is blocking > for > > > sync > > > > lookup, and may accumulate delay, but it maybe acceptable for the > case > > > that > > > > most or all data want to do a delayed lookup). > > > > > > > > For the api perspective, we can do some unification. Let's think of > the > > > > whole user story for lookup join: > > > > > > > > ASYNC_LOOKUP vs SYNC_LOOKUP can share a common one: LOOKUP by > > different > > > > hint option values: 'async'='true|false' > > > > > > > > ASYNC_LOOKUP_MISS_RETRY vs SYNC_LOOKUP_MISS_RETRY can share the > > > > LOOKUP_MISS_RETRY with hint option: 'miss-retry'='true|false' > > > > > > > > we can use one single hint LOOKUP with different hint options > > > > ('async'='true|false', 'miss-retry'='true|false') to cover all > related > > > > functionalities. > > > > Compared to multiple hints with different subsets of functionality, a > > > > single hint may be easier for users to understand and use, and > specific > > > > parameters can be quickly found through documentation > > > > > > > > the support matrix will be: > > > > lookup support async retry > > > > sync w/o retry N N > > > > sync w/ retry N Y > > > > async w/o retry Y N > > > > async w/ retry Y Y > > > > > > > > and the available hint options for each mode: > > > > mode support hint options > > > > async async'='true' > > > > 'output-mode'='ordere|allow-unordered' > > > > 'capacity'='100' > > > > 'timeout'='180s' > > > > retry miss-retry'='true' > > > > 'retry-strategy'='fixed-delay' > > > > 'delay'='10s' > > > > 'max-attempts'='3' > > > > > > > > > > > > 2.) 'allow-unordered' vs 'unordered' for > > > > 'table.exec.async-lookup.output-mode' > > > > Yes, make it align with DataStream Api maybe more intuitive, but > > there's > > > > some difference in table layer that makes the 'allow-unordered' > > > meaningful: > > > > updates in the pipeline need to be processed in order, > ALLOW_UNORDERED > > > > means if users allow unordered result, it will attempt to use > > > > AsyncDataStream.OutputMode.UNORDERED when it does not affect the > > > > correctness of the result, otherwise ORDERED will be still used. > > > > > > > > Another choice is that when the user specifies unordered mode, > planner > > > > throws an error when it finds that it may affect correctness. But > this > > is > > > > not user-friendly and is not consistent with the customary treatment > of > > > > invalid query hints(best effort). > > > > > > > > I opened a pr https://github.com/apache/flink/pull/19759 for the new > > > > option > > > > 'table.exec.async-lookup.output-mode' and also a discussion on > > > FLINK-27625: > > > > add query hint 'ASYNC_LOOKUP' for async lookup join(Since the changes > > > were > > > > relatively minor, no new flip was created) > > > > > > > > If we can reach a consensus on the single unified hint, e.g., LOOKUP, > > > then > > > > FLINK-27625 can be covered. > > > > > > > > WDYT? > > > > > > > > Best, > > > > Lincoln Lee > > > > > > > > > > > > Jark Wu <imj...@gmail.com> 于2022年5月27日周五 21:04写道： > > > > > > > > > Hi Lincoln, > > > > > > > > > > Delayed Dim Join is a frequently requested feature, it's exciting > to > > > see > > > > > this feature is on the road. > > > > > The FLIP looks good to me in general. I only left some minor > > comments. > > > > > > > > > > 1) support retry for sync lookup > > > > > I'm also fine with the idea proposed by Jingsong. But this doesn't > > > > conflict > > > > > with the FLIP and can > > > > > be future work. It would be great if we can determine the APIs > first. > > > > > > > > > > 1) "allow-unordered" => "unordered" > > > > > I would prefer the "unordered" output mode rather than > > > "allow-unordered". > > > > > Because this fully aligns with the DataStream behaviors and avoids > > > > > confusion on the differences. > > > > > I understand the purpose that adding a "allow" prefix here, but I > > think > > > > the > > > > > semantic is fine to just > > > > > use "unordered" here. We didn't see any users confused about > > > > > OutputMode#UNORDERED. > > > > > > > > > > Best, > > > > > Jark > > > > > > > > > > > > > > > > > > > > On Fri, 27 May 2022 at 12:58, Jingsong Li <jingsongl...@gmail.com> > > > > wrote: > > > > > > > > > > > Thanks Lincoln for your proposal. > > > > > > > > > > > > Take a look at `strategy: fixed-delay delay: duration, e.g., 10s > > > > > > max-attempts: integer, e.g., 3`. > > > > > > > > > > > > Are these options only for async? It looks like normal lookups > work > > > > too? > > > > > > > > > > > > One thing is: most of the lookup functions seem to be synchronous > > > now? > > > > > > There are not so many asynchronous ones? > > > > > > > > > > > > Best, > > > > > > Jingsong > > > > > > > > > > > > On Tue, May 24, 2022 at 11:48 AM Lincoln Lee < > > lincoln.8...@gmail.com > > > > > > > > > > wrote: > > > > > > > > > > > > > Hi all, > > > > > > > > > > > > > > Considering the new common table option 'lookup.max-retries' > > > proposed > > > > > in > > > > > > > FLIP-221[1] which is commonly used for exception handling in > > > > connector > > > > > > > implementation, we should clearly distinguish > ASYNC_LOOKUP_RETRY > > > from > > > > > it > > > > > > to > > > > > > > avoid confusing users. > > > > > > > > > > > > > > To do so, the name ASYNC_LOOKUP_RETRY can change to > > > > > > > ASYNC_LOOKUP_MISS_RETRY, and as the name implies, restrict it > to > > > > > support > > > > > > > retries only for lookup misses and no longer include exceptions > > > (for > > > > > sql > > > > > > > connectors, let the connector implementer decide how to handle > > > > > exceptions > > > > > > > since there are various kinds of retryable exceptions and can > not > > > > retry > > > > > > > ones). > > > > > > > > > > > > > > The FLIP[2] has been updated. > > > > > > > > > > > > > > [1] > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > https://cwiki.apache.org/confluence/display/FLINK/FLIP-221%3A+Abstraction+for+lookup+source+cache+and+metric > > > > > > > [2] > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > https://cwiki.apache.org/confluence/display/FLINK/FLIP-234%3A+Support+Retryable+Lookup+Join+To+Solve+Delayed+Updates+Issue+In+External+Systems > > > > > > > > > > > > > > > > > > > > > Best, > > > > > > > Lincoln Lee > > > > > > > > > > > > > > > > > > > > > Lincoln Lee <lincoln.8...@gmail.com> 于2022年5月19日周四 18:24写道： > > > > > > > > > > > > > > > Dear Flink developers, > > > > > > > > > > > > > > > > I would like to open a discussion on FLIP 234 [1] to support > > > > > retryable > > > > > > > > lookup join to solve delayed updates issue, as a pre-work for > > > this > > > > > > > > solution, we proposed FLIP-232[2] which adds a generic retry > > > > support > > > > > > for > > > > > > > > Async I/O. > > > > > > > > We prefer to offer this retry capability via query hints, > > similar > > > > to > > > > > > new > > > > > > > > join hints proposed in FLINK-27625[3] & FLIP-204[4]. > > > > > > > > > > > > > > > > This feature is backwards compatible and transparently to > > > > connectors. > > > > > > For > > > > > > > > existing connectors which implements AsyncTableFunction, can > > > easily > > > > > > > enable > > > > > > > > async retry via the new join hint. > > > > > > > > > > > > > > > > [1] > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > https://cwiki.apache.org/confluence/display/FLINK/FLIP-234%3A+Support+Retryable+Lookup+Join+To+Solve+Delayed+Updates+Issue+In+External+Systems > > > > > > > > [2] > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > https://cwiki.apache.org/confluence/pages/viewpage.action?pageId=211883963 > > > > > > > > [3] > > > > https://lists.apache.org/thread/jm9kg33wk9z2bvo2b0g5bp3n5kfj6qv8 > > > > > > > > [4] > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > https://cwiki.apache.org/confluence/display/FLINK/FLIP-204:+Introduce+Hash+Lookup+Join > > > > > > > > > > > > > > > > Best, > > > > > > > > Lincoln Lee > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > >
