[FLINK-28971][table-planner][doc] Adds user documentation for the new LOOKUP hint #20577

lincoln-lil · 2022-08-15T10:15:44Z

What is the purpose of the change

This is the user documentation work for the new LOOKUP hint, includes Chinese version

Brief change log

add user documentation for the new LOOKUP hint in hints.md page

Does this pull request potentially affect one of the following parts:

Dependencies (does it add or upgrade a dependency): (no)
The public API, i.e., is any changed class annotated with @public(Evolving): (no)
The serializers: (no )
The runtime per-record code paths (performance sensitive): (no)
Anything that affects deployment or recovery: JobManager (and its components), Checkpointing, Kubernetes/Yarn, ZooKeeper: (no)
The S3 file system connector: (no)

Documentation

Does this pull request introduce a new feature? (no)

… LOOKUP hint

flinkbot · 2022-08-15T10:27:53Z

CI report:

ec0d77b Azure: SUCCESS

Bot commands

The @flinkbot bot supports the following commands:

@flinkbot run azure re-run the last Azure build

lincoln-lil · 2022-08-19T01:31:37Z

@flinkbot run azure

godfreyhe

Thanks for the contribution @lincoln-lil , I left some comments, and please add some doc to explain the relation of retry behavior with/without full cache/partial cache

godfreyhe · 2022-08-22T07:25:03Z

docs/content/docs/dev/table/sql/queries/hints.md

+<tr>
+	<th rowspan="1">table</th>
+	<th>table</th>
+	<th>N</th>
+	<th>string</th>
+	<th>N/A</th>
+	<th>the table name of the lookup source</th>


<th> is used for table header, while <td> is used for table body.

godfreyhe · 2022-08-22T07:26:04Z

docs/content/docs/dev/table/sql/queries/hints.md

+	<th>value can be 'true' or 'false' to suggest the planner choose the corresponding lookup function.
+        If the backend lookup source does not support the suggested lookup mode, it will take no effect.</th>


the text should align left

godfreyhe · 2022-08-22T07:27:01Z

docs/content/docs/dev/table/sql/queries/hints.md

+	<th>N</th>
+	<th>string</th>
+	<th>N/A</th>
+	<th>the table name of the lookup source</th>


is full path of table name required ?

no, this is consistent with from clause

godfreyhe · 2022-08-22T07:29:57Z

docs/content/docs/dev/table/sql/queries/hints.md

+<tr>
+	<th>option type</th>
+	<th>option name</th>
+	<th>optional</th>


use required ?

godfreyhe · 2022-08-22T07:35:00Z

docs/content/docs/dev/table/sql/queries/hints.md

+```
+Note: the optimizer prefers async lookup if no 'async' option is specified, it will always use sync lookup when:
+1. the connector only implements the sync lookup
+2. user enables 'TRY_RESOLVE' mode of 'table.optimizer.non-deterministic-update.strategy' and the 


is there any doc to explain them more? if existing, please add a link to refer it

yes, there should be a configuration link

godfreyhe · 2022-08-22T07:37:52Z

docs/content/docs/dev/table/sql/queries/hints.md

+Note: the async options are consistent with the async options in [job level Execution Options]({{< ref "docs/dev/table/config" >}}#execution-options),
+will use job level configuration if not set. Another difference is that the scope of the LOOKUP hint
+is smaller, limited to the table name corresponding to the hint option set in the current lookup
+operation (other lookup operations will not be affected by the LOOKUP hint).


"Another difference is that the scope of the LOOKUP hint
is smaller, limited to the table name corresponding to the hint option set in the current lookup
operation (other lookup operations will not be affected by the LOOKUP hint)."

It's better we should introduce select scope concept to explain it, which is more specific and precise

Agree with you that select scope is necessary for query hints, and I suggested @swuferhong introducing this concept in the begining of query hints in #20513
so this part will not add it again, but after #20351 merged, we can add a link here

godfreyhe · 2022-08-22T07:39:46Z

docs/content/docs/dev/table/sql/queries/hints.md

+##### 3. Enable Delayed Retry Strategy For Lookup
+Delayed retry for lookup join is intended to solve the problem of delayed updates in external system
+which cause unexpected enrichment with stream data. The hint option 'retry-predicate'='lookup_miss'
+can enable retry on both sync and async lookup, only support fixed delay retry strategy currently.


only fixed delay retry strategy is supported currently.

…Caching On Retries'

lincoln-lil

@godfreyhe thanks for your comments here! I've addressed these comments and add a section about 'Effect Of Enabling Caching On Retries'

lincoln-lil · 2022-08-22T09:02:38Z

docs/content/docs/dev/table/sql/queries/hints.md

+<tr>
+	<th>option type</th>
+	<th>option name</th>
+	<th>optional</th>


lincoln-lil · 2022-08-22T09:04:50Z

docs/content/docs/dev/table/sql/queries/hints.md

+	<th>N</th>
+	<th>string</th>
+	<th>N/A</th>
+	<th>the table name of the lookup source</th>


no, this is consistent with from clause

lincoln-lil · 2022-08-22T09:11:31Z

docs/content/docs/dev/table/sql/queries/hints.md

+	<th>value can be 'true' or 'false' to suggest the planner choose the corresponding lookup function.
+        If the backend lookup source does not support the suggested lookup mode, it will take no effect.</th>


lincoln-lil · 2022-08-22T09:15:00Z

docs/content/docs/dev/table/sql/queries/hints.md

+Note: the async options are consistent with the async options in [job level Execution Options]({{< ref "docs/dev/table/config" >}}#execution-options),
+will use job level configuration if not set. Another difference is that the scope of the LOOKUP hint
+is smaller, limited to the table name corresponding to the hint option set in the current lookup
+operation (other lookup operations will not be affected by the LOOKUP hint).


Agree with you that select scope is necessary for query hints, and I suggested @swuferhong introducing this concept in the begining of query hints in #20513
so this part will not add it again, but after #20351 merged, we can add a link here

xuyangzhong · 2022-08-23T03:14:11Z

docs/content/docs/dev/table/sql/queries/hints.md

+##### Further Notes
+
+###### Effect Of Enabling Caching On Retries
+[FLIP-221](https://cwiki.apache.org/confluence/display/FLINK/FLIP-229%3A+Introduces+Join+Hint+for+Flink+SQL+Batch+Job) adds caching support for lookup source,


It seems that this url is wrong.

godfreyhe · 2022-08-23T03:50:18Z

docs/content/docs/dev/table/sql/queries/hints.md

+When PARTIAL caching is enabled, it will lookup from local cache first for a coming record and will
+do an external lookup via backend connector if cache miss(if cache hit, then return the record immediately),
+and this will trigger a retry when lookup result is empty(same with caching disabled), the final lookup
+result is determined when retry done(in PARTIAL caching mode, it will also update local cache).


when the retry is complete ?

changed to "the final lookup result is determined when retry completed"

godfreyhe · 2022-08-23T03:55:54Z

docs/content/docs/dev/table/sql/queries/hints.md

+```sql
+SELECT /*+ LOOKUP(key=value[, key=value]*) */
+
+key:
+    stringLiteral
+
+value:
+    stringLiteral
+```


add a section ### Syntax

godfreyhe · 2022-08-23T03:56:28Z

docs/content/docs/dev/table/sql/queries/hints.md

+- async options are all optional, will use default value if not configured.
+- there is no default value for retry options, all retry options should be set to valid values when need to enable retry.
+
+##### 1. Use Sync And Async Lookup Function


the font is too small, it's better we can change the title levels.

Yes, 5th level is too small, seems we should not follow the sub levels strictly, I'll update this.

godfreyhe · 2022-08-23T03:57:12Z

docs/content/docs/dev/table/sql/queries/hints.md

+    stringLiteral
+```
+
+The available hint options:


add a section ### Lookuup Hint Options

lincoln-lil

@godfreyhe I've addressed all your comments and updated the pr, thanks again!

lincoln-lil · 2022-08-23T06:09:10Z

docs/content/docs/dev/table/sql/queries/hints.md

+When PARTIAL caching is enabled, it will lookup from local cache first for a coming record and will
+do an external lookup via backend connector if cache miss(if cache hit, then return the record immediately),
+and this will trigger a retry when lookup result is empty(same with caching disabled), the final lookup
+result is determined when retry done(in PARTIAL caching mode, it will also update local cache).


changed to "the final lookup result is determined when retry completed"

lincoln-lil · 2022-08-23T06:17:05Z

docs/content/docs/dev/table/sql/queries/hints.md

+    stringLiteral
+```
+
+The available hint options:


lincoln-lil · 2022-08-23T06:19:31Z

docs/content/docs/dev/table/sql/queries/hints.md

+- async options are all optional, will use default value if not configured.
+- there is no default value for retry options, all retry options should be set to valid values when need to enable retry.
+
+##### 1. Use Sync And Async Lookup Function


Yes, 5th level is too small, seems we should not follow the sub levels strictly, I'll update this.

lincoln-lil · 2022-08-23T10:39:10Z

@flinkbot run azure

lincoln-lil · 2022-08-24T01:30:16Z

@flinkbot run azure

godfreyhe

Thanks for the update

godfreyhe · 2022-08-24T09:03:47Z

docs/content/docs/dev/table/sql/queries/hints.md

+2. configure the async parameters
+3. enable delayed retry strategy for lookup
+
+##### Syntax


### Syntax

change to level4 "#### Syntax" ?
because LOOKUP hint is level4 and not displayed on index

godfreyhe · 2022-08-24T09:04:27Z

docs/content/docs/dev/table/sql/queries/hints.md

+### Join Hints
+
+#### LOOKUP Hint


how about merge them into ### LOOKUP Join Hints

the "### Join Hints" here is reserved for merge new join hint doc, to keep consistent with other join hints, change "#### LOOKUP Hint" -> "#### LOOKUP"

godfreyhe · 2022-08-24T09:06:01Z

docs/content/docs/dev/table/sql/queries/hints.md

+```
+Note: the optimizer prefers async lookup if no 'async' option is specified, it will always use sync lookup when:
+1. the connector only implements the sync lookup
+2. user enables 'TRY_RESOLVE' mode of ['table.optimizer.non-deterministic-update.strategy']({{ < ref "docs/dev/table/config" > }}#table-optimizer-non-deterministic-update-strategy) and the 


wrong display result:

godfreyhe · 2022-08-24T09:12:08Z

docs/content.zh/docs/dev/table/sql/queries/hints.md

+LOOKUP('table'='Customers', 'async'='true', 'output-mode'='allow_unordered', 'capacity'='100', 'timeout'='180s')
+```
+注意：联接提示上的异步查找参数和[作业级别配置参数]({{< ref "docs/dev/table/config" >}}#execution-options)的
+含义是一致的，没有设置的参数值由默认值生效，另一个区别是联接提示作用的范围更小，仅限于当前联接操作中对应联接提示选项设置的表名（未被联接提示作用的其他联接查询不受影响）。


no need new line here

godfreyhe · 2022-08-24T09:24:35Z

docs/content.zh/docs/dev/table/sql/queries/hints.md

+缓存策略有部分缓存、全部缓存两种，当开启全部缓存时（'lookup.cache'='FULL'），重试无法起作用（因为查找表被完整缓存，重试查找没有任何实际意义）；当开启部分缓存时，当一条数据开始查找处理时，
+先在本地缓存中查找，如果没找到则通过连接器进行外部查找（如果存在，则立即返回），此时查不到的记录和不开启缓存时一样，会触发重试查找，重试结束时的结果即为最终的查找结果（在部分缓存模式下，更新本地缓存）。 


当开启全部缓存时 -> 开启全部缓存时
当开启部分缓存时 -> 开启部分缓存时 otherwise, there are two 当s

godfreyhe · 2022-08-24T09:35:21Z

docs/content.zh/docs/dev/table/sql/queries/hints.md

+
+#### 关于查找键及 'retry-predicate'='lookup_miss' 重试条件的说明
+对不同的连接器，提供的索引查找能力可能是不同的，例如内置的 HBase 连接器，默认仅提供了基于 `rowkey` 的索引查找能力（未
+启用二级索引），而对于内置的 JDBC 连接器，默认情况下任何字段都可以被用作索引查找，这是物理存储的特性不同所决定的。


lincoln-lil

@godfreyhe thanks for your detailed comments! I've updated the pr.

lincoln-lil · 2022-08-25T01:41:57Z

docs/content/docs/dev/table/sql/queries/hints.md

+### Join Hints
+
+#### LOOKUP Hint


the "### Join Hints" here is reserved for merge new join hint doc, to keep consistent with other join hints, change "#### LOOKUP Hint" -> "#### LOOKUP"

lincoln-lil · 2022-08-25T01:59:19Z

docs/content/docs/dev/table/sql/queries/hints.md

+2. configure the async parameters
+3. enable delayed retry strategy for lookup
+
+##### Syntax


change to level4 "#### Syntax" ?
because LOOKUP hint is level4 and not displayed on index

lincoln-lil · 2022-08-26T01:24:19Z

@flinkbot run azure

godfreyhe

LGTM

This closes apache#20577

[FLINK-28971][table-planner][doc] Adds user documentation for the new…

cc59fad

… LOOKUP hint

flinkbot added the component=<none> label Aug 15, 2022

fix doc ref

edb588c

lincoln-lil mentioned this pull request Aug 16, 2022

[FLINK-28858][docs] Add document to describe join hints for batch sql #20513

Closed

godfreyhe reviewed Aug 22, 2022

View reviewed changes

address review comments and add a section of the 'Effect Of Enabling …

a7d2efd

…Caching On Retries'

lincoln-lil commented Aug 22, 2022

View reviewed changes

fix flip ref

57509bc

xuyangzhong reviewed Aug 23, 2022

View reviewed changes

godfreyhe reviewed Aug 23, 2022

View reviewed changes

address comments

87ecee8

lincoln-lil commented Aug 23, 2022

View reviewed changes

godfreyhe reviewed Aug 24, 2022

View reviewed changes

address comments

b9e80b7

lincoln-lil commented Aug 25, 2022

View reviewed changes

remove unnecessary newlines

ec0d77b

godfreyhe approved these changes Sep 2, 2022

View reviewed changes

godfreyhe closed this in bbe9690 Sep 2, 2022

huangxiaofeng10047 pushed a commit to huangxiaofeng10047/flink that referenced this pull request Nov 3, 2022

[FLINK-28971][docs] Adds user documentation for the new LOOKUP hint

6525045

This closes apache#20577

		<th>value can be 'true' or 'false' to suggest the planner choose the corresponding lookup function.
		If the backend lookup source does not support the suggested lookup mode, it will take no effect.</th>

		缓存策略有部分缓存、全部缓存两种，当开启全部缓存时（'lookup.cache'='FULL'），重试无法起作用（因为查找表被完整缓存，重试查找没有任何实际意义）；当开启部分缓存时，当一条数据开始查找处理时，
		先在本地缓存中查找，如果没找到则通过连接器进行外部查找（如果存在，则立即返回），此时查不到的记录和不开启缓存时一样，会触发重试查找，重试结束时的结果即为最终的查找结果（在部分缓存模式下，更新本地缓存）。

[FLINK-28971][table-planner][doc] Adds user documentation for the new LOOKUP hint #20577

[FLINK-28971][table-planner][doc] Adds user documentation for the new LOOKUP hint #20577

Uh oh!

Conversation

lincoln-lil commented Aug 15, 2022

What is the purpose of the change

Brief change log

Does this pull request potentially affect one of the following parts:

Documentation

Uh oh!

flinkbot commented Aug 15, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

CI report:

Uh oh!

lincoln-lil commented Aug 19, 2022

Uh oh!

godfreyhe left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

lincoln-lil left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

lincoln-lil left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

flinkbot commented Aug 15, 2022 •

edited

Loading