github-actions[bot] commented on code in PR #69298:
URL: https://github.com/apache/doris/pull/69298#discussion_r4321746619
##########
fe/fe-connector/fe-connector-iceberg/src/main/java/org/apache/doris/connector/iceberg/IcebergScanPlanProvider.java:
##########
@@ -632,11 +620,16 @@ public void close() {
* streaming source's {@code hasNext}. Cache disabled -> the SDK path,
byte-unchanged.
*/
private CloseableIterable<FileScanTask> streamingFileScanTasks(TableScan
scan, ConnectorSession session,
- Table table, Optional<ConnectorExpression> filter, long sliceSize)
{
+ Table table, Optional<ConnectorExpression> filter, long sliceSize,
+ ManifestReadControl readControl, IcebergTableHandle readHandle) {
if (isManifestCacheEnabled()) {
try {
return TableScanUtil.splitFiles(
- cacheBackedFileScanTasks(scan, session, table, filter,
null), sliceSize);
+ cacheBackedFileScanTasks(scan, session, table, filter,
null, readControl, readHandle),
+ sliceSize);
+ } catch (CancellationException e) {
Review Comment:
[P2] Prevent SDK fallback when scheduler acquisition is cancelled
With the manifest cache enabled, KILL/timeout can close the query groups
after a producer enters `streamSplits` but before `cacheBackedFileScanTasks`
obtains its metadata scheduler. `QueryTaskGroups.get()` then throws
`RejectedExecutionException`. That bypasses this `CancellationException` catch,
reaches the generic catch, and invokes `scan.planFiles()` at line 639. For a v2
table with cold position-delete manifests, SDK planning [builds the delete
index
eagerly](https://github.com/apache/iceberg/blob/apache-iceberg-1.12.0/core/src/main/java/org/apache/iceberg/ManifestGroup.java#L177-L191),
so cancellation starts new remote reads outside the query budget and keeps the
split worker occupied; before coordinator publication there is no coordinator
to stop that producer. Check the captured query lifetime before choosing the
fallback and propagate cancellation for a closed scheduler, including the
materialized/count variants. Add a regression that closes the groups before
scheduler acq
uisition; the existing Iceberg tests only close an already-captured scheduler.
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
