stevenzwu commented on code in PR #7571:
URL: https://github.com/apache/iceberg/pull/7571#discussion_r1190615363
##########
flink/v1.17/flink/src/main/java/org/apache/iceberg/flink/FlinkReadOptions.java:
##########
@@ -105,4 +105,8 @@ private FlinkReadOptions() {}
public static final String LIMIT = "limit";
public static final ConfigOption<Long> LIMIT_OPTION =
ConfigOptions.key(PREFIX + LIMIT).longType().defaultValue(-1L);
+
+ public static final String PLAN_RETRY_NUM = "plan-retry-num";
Review Comment:
suggested a diff name i the doc. applies to other variable or method
##########
flink/v1.17/flink/src/main/java/org/apache/iceberg/flink/FlinkReadOptions.java:
##########
@@ -105,4 +105,8 @@ private FlinkReadOptions() {}
public static final String LIMIT = "limit";
public static final ConfigOption<Long> LIMIT_OPTION =
ConfigOptions.key(PREFIX + LIMIT).longType().defaultValue(-1L);
+
+ public static final String PLAN_RETRY_NUM = "plan-retry-num";
Review Comment:
suggested a diff name i the doc
##########
flink/v1.17/flink/src/main/java/org/apache/iceberg/flink/source/enumerator/ContinuousIcebergEnumerator.java:
##########
@@ -115,7 +116,12 @@ private ContinuousEnumerationResult discoverSplits() {
return new ContinuousEnumerationResult(
Collections.emptyList(), enumeratorPosition.get(), enumeratorPosition.get());
} else {
- return splitPlanner.planSplits(enumeratorPosition.get());
+ AtomicReference<ContinuousEnumerationResult> result = new AtomicReference<>();
+ Tasks.foreach(enumeratorPosition.get())
Review Comment:
I was thinking about more like max allowed consecutive planning failures (not retries), because planning is scheduled periodical already
##########
docs/flink-configuration.md:
##########
@@ -110,27 +110,28 @@ env.getConfig()
`Read option` has the highest priority, followed by `Flink configuration` and then `Table property`.
-| Read option | Flink configuration | Table property | Default | Description |
-| --------------------------- | --------------------------------------------- | ---------------------------- | -------------------------------- | ------------------------------------------------------------ |
-| snapshot-id | N/A | N/A | null | For time travel in batch mode. Read data from the specified snapshot-id. |
-| case-sensitive | connector.iceberg.case-sensitive | N/A | false | If true, match column name in a case sensitive way. |
-| as-of-timestamp | N/A | N/A | null | For time travel in batch mode. Read data from the most recent snapshot as of the given time in milliseconds. |
+| Read option | Flink configuration | Table property | Default | Description
|
+|-----------------------------|-----------------------------------------------|------------------------------|----------------------------------|------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------
----------------|
+| snapshot-id | N/A | N/A | null | For time travel in batch mode. Read data from the specified snapshot-id.
|
+| case-sensitive | connector.iceberg.case-sensitive | N/A | false | If true, match column name in a case sensitive way.
|
+| as-of-timestamp | N/A | N/A | null | For time travel in batch mode. Read data from the most recent snapshot as of the given time in milliseconds.
|
| starting-strategy | connector.iceberg.starting-strategy | N/A | INCREMENTAL_FROM_LATEST_SNAPSHOT | Starting strategy for streaming execution. TABLE_SCAN_THEN_INCREMENTAL: Do a regular table scan then switch to the incremental mode. The incremental mode starts from the current snapshot exclusive. INCREMENTAL_FROM_LATEST_SNAPSHOT: Start incremental mode from the latest snapshot inclusive. If it is an empty map, all future append snapshots should be discovered. INCREMENTAL_FROM_EARLIEST_SNAPSHOT: Start incremental mode from the earliest snapshot inclusive. If it is an empty map, all future append snapshots should be discovered. INCREMENTAL_FROM_SNAPSHOT_ID: Start incremental mode from a snapshot with a specific id inclusive. INCREMENTAL_FROM_SNAPSHOT_TIMESTAMP: Start incremental mode from a snapshot with a specific timestamp inclusive. If the timestamp is between two snapshots, it should start from the snapshot after the timestamp. Just fo
r FIP27 Source. |
-| start-snapshot-timestamp | N/A | N/A | null | Start to read data from the most recent snapshot as of the given time in milliseconds. |
-| start-snapshot-id | N/A | N/A | null | Start to read data from the specified snapshot-id. |
-| end-snapshot-id | N/A | N/A | The latest snapshot id | Specifies the end snapshot.
-| branch | N/A | N/A | main | Specifies the branch to read from in batch mode
-| tag | N/A | N/A | null | Specifies the tag to read from in batch mode
-| start-tag | N/A | N/A | null | Specifies the starting tag to read from for incremental reads
-| end-tag | N/A | N/A | null | Specifies the ending tag to to read from for incremental reads |
-| split-size | connector.iceberg.split-size | read.split.target-size | 128 MB | Target size when combining input splits. |
-| split-lookback | connector.iceberg.split-file-open-cost | read.split.planning-lookback | 10 | Number of bins to consider when combining input splits. |
-| split-file-open-cost | connector.iceberg.split-file-open-cost | read.split.open-file-cost | 4MB | The estimated cost to open a file, used as a minimum weight when combining splits. |
-| streaming | connector.iceberg.streaming | N/A | false | Sets whether the current task runs in streaming or batch mode. |
-| monitor-interval | connector.iceberg.monitor-interval | N/A | 60s | Monitor interval to discover splits from new snapshots. Applicable only for streaming read. |
-| include-column-stats | connector.iceberg.include-column-stats | N/A | false | Create a new scan from this that loads the column stats with each data file. Column stats include: value count, null value count, lower bounds, and upper bounds. |
-| max-planning-snapshot-count | connector.iceberg.max-planning-snapshot-count | N/A | Integer.MAX_VALUE | Max number of snapshots limited per split enumeration. Applicable only to streaming read. |
-| limit | connector.iceberg.limit | N/A | -1 | Limited output number of rows. |
+| start-snapshot-timestamp | N/A | N/A | null | Start to read data from the most recent snapshot as of the given time in milliseconds.
|
+| start-snapshot-id | N/A | N/A | null | Start to read data from the specified snapshot-id.
|
+| end-snapshot-id | N/A | N/A | The latest snapshot id | Specifies the end snapshot.
|
+| branch | N/A | N/A | main | Specifies the branch to read from in batch mode
|
+| tag | N/A | N/A | null | Specifies the tag to read from in batch mode
|
+| start-tag | N/A | N/A | null | Specifies the starting tag to read from for incremental reads
|
+| end-tag | N/A | N/A | null | Specifies the ending tag to to read from for incremental reads
|
+| split-size | connector.iceberg.split-size | read.split.target-size | 128 MB | Target size when combining input splits.
|
+| split-lookback | connector.iceberg.split-file-open-cost | read.split.planning-lookback | 10 | Number of bins to consider when combining input splits.
|
+| split-file-open-cost | connector.iceberg.split-file-open-cost | read.split.open-file-cost | 4MB | The estimated cost to open a file, used as a minimum weight when combining splits.
|
+| streaming | connector.iceberg.streaming | N/A | false | Sets whether the current task runs in streaming or batch mode.
|
+| monitor-interval | connector.iceberg.monitor-interval | N/A | 60s | Monitor interval to discover splits from new snapshots. Applicable only for streaming read.
|
+| include-column-stats | connector.iceberg.include-column-stats | N/A | false | Create a new scan from this that loads the column stats with each data file. Column stats include: value count, null value count, lower bounds, and upper bounds.
|
+| max-planning-snapshot-count | connector.iceberg.max-planning-snapshot-count | N/A | Integer.MAX_VALUE | Max number of snapshots limited per split enumeration. Applicable only to streaming read.
|
+| limit | connector.iceberg.limit | N/A | -1 | Limited output number of rows.
|
+| plan-retry-num | connector.iceberg.plan-retry-num | N/A | 3 | Retries before job failure when there is an error during continuous planning. Set to -1 to not fail on error.
|
Review Comment:
`max-allowed-consecutive-planning-failures`? I know it is a bit long, but probably more clear
Regarding the description, `retries` is not accurate. how about the following?
```
Max allowed consecutive failures for scan planning before failing the job. Set to -1 for never failing the job for scan planing failure.
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org