havefunatcode opened a new pull request, #20160:
URL: https://github.com/apache/pinot/pull/20160
Fixes #19384
SegmentUriPush job only picks files ending with a segment archive extension
(`.tar.gz`, `.tar.lz4`, `.tar.zst`) in
`SegmentPushUtils.getSegmentUriToTarPathMap()`. Segments in the deep store are
stored without extension (for example committed REALTIME segments like
`myTable__0__0__20260601T1557Z`), so they are silently skipped and the job
pushes nothing. The single segment URI upload API works for these files, only
the job is blocked.
Changes:
- Add an overload `getSegmentUriToTarPathMap(outputDirURI, pushSpec, files,
allowSegmentsWithoutExtension)`. When the flag is true and
`pushFileNamePattern` is set, files matched by the pattern are selected even
without archive extension. The old 3-arg method keeps the current behavior.
- Use it only in the URI push runners: standalone `SegmentUriPushJobRunner`
(through a new protected `getSegmentUriToTarPathMap()` in
`BaseSegmentPushJobRunner`), `HadoopSegmentUriPushJobRunner` and
`BaseSparkSegmentUriPushJobRunner`. Tar push and metadata push are not changed.
- `.metadata.tar.gz` files are still skipped.
- `BaseSegmentPushJobRunner.getSegmentsToReplace()` now uses
`SegmentPushUtils.getSegmentName()` (made public, it already handles deep store
names without extension), so consistent data push also works with these
segments. For tar and metadata push the map only has archive files, so the
result is the same.
I required `pushFileNamePattern` for files without extension on purpose.
Without a pattern, a plain "take every file" rule could push things like
`_SUCCESS`, `.crc` files or segment `.tmp.*` files left in the output dir. With
the pattern the user says which files are segments. The pattern should be
strict enough to skip `<segmentName>.tmp.<uuid>` files left by realtime
commits, e.g.:
```yaml
jobType: SegmentUriPush
outputDirURI: 'adl2://deepstore/myTable/'
pushJobSpec:
pushFileNamePattern: 'glob:**/myTable__*Z'
```
Behavior change: for an existing URI push job that already sets
`pushFileNamePattern`, files (and directories, since the recursive listing
includes them) matched by the pattern without archive extension are now pushed
too. Patterns ending with `.tar.gz` like the samples in `PushJobSpec` are not
affected. I also added one line to the `PushJobSpec` javadoc about this.
If you prefer a different rule (for example allow it always for URI push, or
a new explicit flag in `PushJobSpec`), I am happy to change it.
Testing:
- New
`SegmentPushUtilsTest#testGetSegmentUriToTarPathMapWithoutArchiveExtension`
- New `SegmentUriPushJobRunnerTest` in the standalone module (local FS,
checks the selected files and the segment names used for consistent push)
- `./mvnw -pl
pinot-plugins/pinot-batch-ingestion/pinot-batch-ingestion-standalone -am
-Dtest=SegmentPushUtilsTest,SegmentUriPushJobRunnerTest
-Dsurefire.failIfNoSpecifiedTests=false install` passed
- `test-compile` for the Hadoop, spark-base and spark-3 batch ingestion
modules passed
- spotless, checkstyle and license checks passed on the touched modules
Not tested with a real ADLS deep store, only local file system.
Label: bugfix
--
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]
