laserninja opened a new pull request, #12411:
URL: https://github.com/apache/gravitino/pull/12411
### What changes were proposed in this pull request?
Sort the file scan tasks a scan plan produces, so that an identical scan of
an identical snapshot produces an identical `POST .../tables/{table}/plan`
response.
Tasks are ordered by:
```
(data file location, start, length,
data sequence number, file sequence number,
manifest location, manifest entry position)
```
The first three fields are the obvious key, and they are not enough. One
data file path can be referenced by several manifest entries - the same file
appended in two snapshots, for instance - and those entries tie on location,
offset and length while carrying different sequence numbers and delete files:
```
location=…/dup.parquet start=0 length=10 dataSeq=2 manifest=…-m0.avro pos=0
location=…/dup.parquet start=0 length=10 dataSeq=1 manifest=…-m0.avro pos=0
```
A manifest entry is unique within a snapshot, identified by its manifest and
its position in that manifest, so comparing those makes the order total for any
two tasks that are not interchangeable. Sequence numbers are compared before
the manifest fields because they survive a rewrite into new manifests, and they
remain the discriminator if a reader leaves the manifest fields unset.
This is the first PR of the stack that implements
[#11284](https://github.com/apache/gravitino/issues/11284); see the plan in
[#12194](https://github.com/apache/gravitino/pull/12194).
### Why are the changes needed?
Iceberg plans manifests in parallel, so `planFiles()` does not return tasks
in a reproducible order: planning one snapshot twice can return the same tasks
in a different order, and so can two Gravitino replicas planning it at the same
time.
That matters on its own - an identical request should give an identical
response, which makes the scan plan cache and any client-side comparison of two
plans behave predictably - and it is a prerequisite for the rest of #11284.
Server-side scan planning hands a client one batch of a plan at a time and
identifies a batch by position, so positions have to mean the same thing every
time a plan is computed.
Fix: #11284
### Does this PR introduce _any_ user-facing change?
The `file-scan-tasks` array in a scan planning response is now ordered
rather than arbitrary. No field is added, removed or changed, and the set of
tasks returned is the same as before; a client that treats the array as an
unordered collection, as the Iceberg REST specification allows, sees no
difference.
### How was this patch tested?
New `TestScanPlanTaskOrdering`:
- tasks are ordered by data file location, for files appended in a different
order;
- two manifest entries of the same data file path are ordered by their
sequence numbers - this fails without the sequence number and manifest entry
comparisons, since sorting is stable and the two tasks otherwise keep the order
Iceberg planned them in;
- planning the same snapshot twice returns the same order.
```bash
./gradlew :iceberg:iceberg-rest-server:check -PskipITs
```
--
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]
