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

Reply via email to