Skip to content

[#11284] feat(iceberg-rest): Add the fetch scan tasks endpoint and its plumbing - #12412

Open
laserninja wants to merge 1 commit into
apache:mainfrom
laserninja:feat/11284-2-fetch-scan-tasks-endpoint
Open

[#11284] feat(iceberg-rest): Add the fetch scan tasks endpoint and its plumbing#12412
laserninja wants to merge 1 commit into
apache:mainfrom
laserninja:feat/11284-2-fetch-scan-tasks-endpoint

Conversation

@laserninja

Copy link
Copy Markdown
Collaborator

What changes were proposed in this pull request?

This is PR 2 of the stack that splits #12194, as agreed with @nevzheng (plan). PR 1 is #12411.

It wires up POST /v1/{prefix}/namespaces/{namespace}/tables/{table}/tasks, the second step of the Iceberg REST two-step scan planning protocol, without yet implementing what it hands back:

  • IcebergTableOperations.fetchScanTasks - the JAX-RS resource, with the same authorization expression as POST .../plan since it is the same read of the same table.
  • fetchScanTasks through the table dispatcher chain: IcebergTableOperationDispatcher (interface), IcebergTableEventDispatcher (events), IcebergTableHookDispatcher (pass-through, the operation is read-only), IcebergTableOperationExecutor (delegation to the catalog wrapper).
  • OperationType.FETCH_SCAN_TASKS and AuditLog.Operation.FETCH_SCAN_TASKS, with IcebergFetchScanTasksPreEvent, IcebergFetchScanTasksEvent and IcebergFetchScanTasksFailureEvent.
  • NoSuchPlanTaskException mapped to 404 in IcebergExceptionMapper.

CatalogWrapperForREST.planTableScan still returns every file scan task inline and hands out no plan-tasks, so no plan task presented to this endpoint was issued by this server, and CatalogWrapperForREST.fetchScanTasks rejects every one of them as unknown. Two deliberate consequences:

  • The endpoint is not advertised in /v1/config. Clients gate on the advertised endpoint set rather than probing - pyiceberg enables server-side scan planning when it sees POST .../tasks there, and fails the scan if the endpoint then serves nothing. Advertising it before it can return tasks would be worse than not having it. From a client's point of view this PR leaves behaviour exactly as it is today.
  • The success path of the endpoint arrives with batching, in the last PR of the stack, together with TestIcebergFetchScanTasksEndpoint which exercises it end to end. I checked that FetchScanTasksResponse is already covered by RESTSerializers.registerAll, so the response serialization this PR leaves untested is not an open question.

One change here is not strictly about /tasks: both scan planning endpoints now report 400 instead of 500 when called with no request body. Jersey passes the resource method a null entity, which became an NPE downstream. /tasks needed the guard, and applying the same three lines to /plan seemed better than leaving the neighbouring endpoint wrong. Happy to drop it into its own PR if you would rather keep this one single-purpose.

Why are the changes needed?

#11284 asks for the fetch scan tasks endpoint. Reviewers on #12194 asked for that work in reviewable pieces; this is the plumbing piece, separated from the batching logic that gives it something to return.

Fix: #11284

Does this PR introduce any user-facing change?

A new REST route exists but is not advertised, and answers 404 NoSuchPlanTaskException for any plan task. No new configuration properties. The only behaviour change a user can observe is that POST .../plan and POST .../tasks with an empty body now return 400 instead of 500.

How was this patch tested?

./gradlew :iceberg:iceberg-rest-server:check -PskipITs and ./gradlew :core:test --tests "*TestCompatibilityUtils*", both green.

Three tests added to TestIcebergTableOperations, each run against both a flat and a nested namespace:

  • testFetchScanTasksUnknownPlanTask - a plan task this server never issued is a 404. Asserts on the error payload (type is NoSuchPlanTaskException, message names the rejected plan task), not just the status, so it cannot pass against an unregistered route. Asserts the pre event and the failure event are dispatched.
  • testFetchScanTasksTableNotFound - a missing table reports NoSuchTableException, not a masked unknown plan task.
  • testScanPlanningEndpointsRejectMissingRequestBody - an empty body on /plan and on /tasks is a 400.

…and its plumbing

Wire `POST /v1/{prefix}/namespaces/{namespace}/tables/{table}/tasks`, the
second step of the Iceberg REST two-step scan planning protocol, through the
Iceberg REST server: the JAX-RS resource, the table dispatcher chain, the
`FETCH_SCAN_TASKS` audit operation with its three listener events, and
`NoSuchPlanTaskException` mapped to 404.

Scan planning still returns every file scan task inline and hands out no
`plan-tasks`, so no plan task presented to this endpoint was issued by this
server and every request is rejected as unknown. The endpoint is deliberately
not advertised in `/v1/config` while that is the case, so a spec-compliant
client never reaches it. Batching a plan into plan tasks, and redeeming them
here, follows in a later change.

Also report a 400 rather than a 500 when either scan planning endpoint is
called without a request body: Jersey passes a null entity, which would
otherwise surface as a downstream NPE.
@github-actions

Copy link
Copy Markdown

Code Coverage Report

Overall Project 68.35% +0.37% 🟢
Files changed 90.43% 🟢

Module Coverage
aliyun 1.72% 🔴
api 49.37% 🟢
authorization-common 85.96% 🟢
aws 42.04% 🟢
azure 2.47% 🔴
catalog-common 9.92% 🔴
catalog-fileset 79.74% 🟢
catalog-glue 68.95% 🟢
catalog-hive 79.4% 🟢
catalog-jdbc-common 45.7% 🟢
catalog-jdbc-doris 81.8% 🟢
catalog-jdbc-mysql 79.33% 🟢
catalog-jdbc-postgresql 83.39% 🟢
catalog-jdbc-starrocks 79.16% 🟢
catalog-kafka 77.01% 🟢
catalog-lakehouse-generic 59.18% 🟢
catalog-lakehouse-hudi 79.1% 🟢
catalog-lakehouse-iceberg 85.86% 🟢
catalog-lakehouse-paimon 84.23% 🟢
catalog-model 77.72% 🟢
cli 44.48% 🟢
client-java 78.27% 🟢
common 52.57% 🟢
core 83.29% +0.34% 🟢
filesystem-hadoop3 77.28% 🟢
flink 0.0% 🔴
flink-common 48.68% 🟢
flink-runtime 0.0% 🔴
gcp 14.12% 🔴
hadoop-auth 68.0% 🟢
hadoop-common 12.7% 🔴
hive-metastore-common 53.4% 🟢
iceberg-aliyun-bundle 0.0% 🔴
iceberg-common 64.75% 🟢
iceberg-rest-server 75.07% +2.66% 🟢
idp-basic 86.02% 🟢
integration-test-common 0.0% 🔴
jobs 62.92% 🟢
lance-common 31.75% 🔴
lance-rest-server 63.47% 🟢
lineage 53.02% 🟢
optimizer 83.17% 🟢
optimizer-api 21.95% 🔴
server 85.76% 🟢
server-common 76.47% 🟢
spark 28.57% 🔴
spark-common 45.89% 🟢
tencent 69.84% 🟢
trino-connector 40.29% 🟢
Files
Module File Coverage
core OperationType.java 100.0% 🟢
CompatibilityUtils.java 99.41% 🟢
AuditLog.java 97.66% 🟢
iceberg-rest-server IcebergTableOperationExecutor.java 98.91% 🟢
IcebergExceptionMapper.java 98.18% 🟢
IcebergTableEventDispatcher.java 92.7% 🟢
IcebergTableOperations.java 81.67% 🟢
IcebergTableHookDispatcher.java 80.41% 🟢
CatalogWrapperForREST.java 79.07% 🟢
IcebergFetchScanTasksFailureEvent.java 66.67% 🟢
IcebergFetchScanTasksPreEvent.java 66.67% 🟢
IcebergTableOperationDispatcher.java 0.0% 🔴
IcebergFetchScanTasksEvent.java 0.0% 🔴

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Improvement] Support POST /v1/{prefix}/namespaces/{namespace}/tables/{table}/tasks endpoint for server-side scan planning

1 participant