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
> > > >
> > >
> >
>
