laserninja opened a new pull request, #12194:
URL: https://github.com/apache/gravitino/pull/12194
### What changes were proposed in this pull request?
Implement `fetchScanTasks` (`POST
/v1/{prefix}/namespaces/{namespace}/tables/{table}/tasks`) for the Iceberg REST
server, wired through the full dispatcher chain, and advertise it in
`/v1/config`.
Scan planning in Gravitino is synchronous: `POST .../plan` always returns
status `COMPLETED` with the full set of `file-scan-tasks` inline and never
issues `plan-task` tokens. Any token presented to `/tasks` therefore did not
originate from this server, which per the Iceberg REST spec is an unknown plan
task, so the endpoint returns `404` with `NoSuchPlanTaskException`. The table
is resolved first, so a bad table reports a table-level 404 rather than masking
it as an unknown plan task.
**Files changed:**
- **`IcebergTableOperations`**: New JAX-RS endpoint with metrics,
authorization, and logging
- **`IcebergTableOperationDispatcher`**: Interface method for
`fetchScanTasks`
- **`IcebergTableOperationExecutor`**: Delegates to `CatalogWrapperForREST`
- **`IcebergTableEventDispatcher`**: Pre/post/failure event dispatch
- **`IcebergTableHookDispatcher`**: Pass-through delegation (read-only, no
hooks needed)
- **`CatalogWrapperForREST`**: Resolves the table, then throws
`NoSuchPlanTaskException`
- **`IcebergExceptionMapper`**: Maps `NoSuchPlanTaskException` -> 404
- **`IcebergConfigOperations`**: Advertises
`Endpoint.V1_FETCH_TABLE_SCAN_PLAN_TASKS`
- **`OperationType`** / **`AuditLog`** / **`CompatibilityUtils`**: Add
`FETCH_SCAN_TASKS`
- **3 new event classes**: Pre/post/failure events
- **`docs/iceberg-rest-service.md`**: Document the synchronous-planning
behavior and why `/tasks` exists
### Why are the changes needed?
Server-side scan planning is a two-step protocol in the Iceberg REST spec:
`POST .../plan` to submit, `POST .../tasks` to collect batched results.
Gravitino implemented only the first step, so `/tasks` returned 404 purely
because the route did not exist.
Clients gate on the advertised endpoint set rather than probing. pyiceberg
refuses to use server-side scan planning at all unless `/tasks` appears in
`/v1/config`, raising `NotImplementedError` even though Gravitino returns every
task inline and never actually needs the second call. Implementing and
advertising the endpoint completes the protocol surface, unblocks those
clients, and leaves a clear place for batched planning to land if `/plan` later
starts emitting `plan-tasks`.
Fix: #11284
> **Note on overlap with #11635:** these are different endpoints for
different issues. #11635 adds `GET`/`DELETE .../plan/{planId}` (the async plan
lifecycle); this PR adds `POST .../tasks` (the batched-results step). No
behavioral overlap, but they touch several of the same files additively, so
whichever merges second will need a trivial rebase.
### Does this PR introduce _any_ user-facing change?
Yes:
- **New REST endpoint**: `POST
/v1/{prefix}/namespaces/{namespace}/tables/{table}/tasks` - returns 404
`NoSuchPlanTaskException` for any plan-task token; 404 `NoSuchTableException`
if the table does not exist.
- **`/v1/config`** now advertises `POST
/v1/{prefix}/namespaces/{namespace}/tables/{table}/tasks`.
- **New audit operation** `FETCH_SCAN_TASKS`, and three new `@DeveloperApi`
event classes for event listeners.
- **Incidental fix to an existing endpoint**: `POST .../plan` with a missing
or empty request body previously returned **500** (unmapped
`NullPointerException`); it now returns **400**. This also makes the empty-body
case consistent with `{}` and `{"plan-task": null}`, which Jackson already
rejected with 400.
### How was this patch tested?
New tests:
- `TestIcebergTableOperations.testFetchScanTasksUnknownPlanTask` - asserts
404 **and** that the error payload type is `NoSuchPlanTaskException`, so the
test cannot pass on a bare "route not registered" 404; also asserts `PreEvent`
+ `FailureEvent` dispatch.
- `TestIcebergTableOperations.testFetchScanTasksTableNotFound` - asserts 404
with a `NoSuchTableException` payload.
-
`TestIcebergTableOperations.testScanPlanningEndpointsRejectMissingRequestBody`
- covers the empty-body 400 for both `/plan` and `/tasks`.
- `TestIcebergConfig.testConfigAdvertisesFetchScanTasksEndpoint` - verifies
`/v1/config` advertises the `/tasks` path, which is the actual reason the
endpoint needs to exist.
```bash
./gradlew :iceberg:iceberg-rest-server:test :core:test -PskipITs
```
All existing tests continue to pass; the new tests were re-run with
`--rerun` to check for order-dependence.
--
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]
