apache
Not able to delete existing table & schema from Pinot UI
https://github.com/apache/pinot/pull/15473/commits/7cff2a4b105cfacbdce8da6934bdadeb3e17d789
Issue in PR #15473
What the PR Did (The Good Part)
The PR added a new deleteBatch() method to delete multiple files at once instead of one by one. This is much faster, especially for cloud storage like S3.
Example: Instead of deleting 100 files one at a time (100 API calls), you can delete them all together (1 API call).
The Problems We Found (What Was Missing)
Problem 1: Hadoop Filesystem Was Left Out
- The PR added optimized
deleteBatch()for S3 (Amazon's storage) - But Hadoop filesystem (HDFS) was forgotten - it still deletes files one by one
- This means Hadoop users don't get the performance improvement
Analogy: It's like upgrading all cars to electric except the delivery trucks - they still run on old technology.
Problem 2: No Safety Check for Missing Files
- The default
deleteBatch()inBasePinotFStries to delete files without checking if they exist first - If a file is already deleted or doesn't exist, it throws an error and stops
- This can break the deletion process
Codecov Report
:white_check_mark: All modified and coverable lines are covered by tests.
:white_check_mark: Project coverage is 63.26%. Comparing base (82b7d2a) to head (2fb8bcc).
:warning: Report is 43 commits behind head on master.
Additional details and impacted files
@@ Coverage Diff @@
## master #17294 +/- ##
==========================================
Coverage 63.25% 63.26%
- Complexity 1475 1476 +1
==========================================
Files 3162 3167 +5
Lines 188668 189196 +528
Branches 28869 28952 +83
==========================================
+ Hits 119348 119690 +342
- Misses 60074 60215 +141
- Partials 9246 9291 +45
| Flag | Coverage Δ | |
|---|---|---|
| custom-integration1 | 100.00% <ø> (ø) |
|
| integration | 100.00% <ø> (ø) |
|
| integration1 | 100.00% <ø> (ø) |
|
| integration2 | 0.00% <ø> (ø) |
|
| java-11 | 63.24% <100.00%> (+0.01%) |
:arrow_up: |
| java-21 | 63.18% <100.00%> (-0.05%) |
:arrow_down: |
| temurin | 63.26% <100.00%> (+<0.01%) |
:arrow_up: |
| unittests | 63.25% <100.00%> (+<0.01%) |
:arrow_up: |
| unittests1 | 55.56% <0.00%> (-0.04%) |
:arrow_down: |
| unittests2 | 34.02% <100.00%> (+0.02%) |
:arrow_up: |
Flags with carried forward coverage won't be shown. Click here to find out more.
:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.
:rocket: New features to boost your workflow:
- :package: JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.
Sure
On Thu, 4 Dec 2025 at 2:41 PM, Abhishek Bafna @.***> wrote:
@.**** commented on this pull request.
In pinot-plugins/pinot-file-system/pinot-hdfs/src/main/java/org/apache/pinot/plugin/filesystem/HadoopPinotFS.java https://github.com/apache/pinot/pull/17294#discussion_r2588177361:
@@ -88,6 +88,83 @@ public boolean delete(URI segmentUri, boolean forceDelete) return _hadoopFS.delete(new Path(segmentUri), true); }
- @Override
Please add test cases for all the public methods.
— Reply to this email directly, view it on GitHub https://github.com/apache/pinot/pull/17294#pullrequestreview-3538863664, or unsubscribe https://github.com/notifications/unsubscribe-auth/AVL2AZQBR74LQDTVUDGCBSL3773F5AVCNFSM6AAAAACNVIM3JCVHI2DSMVQWIX3LMV43YUDVNRWFEZLROVSXG5CSMV3GSZLXHMZTKMZYHA3DGNRWGQ . You are receiving this because you authored the thread.Message ID: @.***>
@abhishekbafna @Jackie-Jiang @xiangfu0 please review and any comments
Sorry for the late response. LGTM otherwise
Let's follow the contract proposed by @abhishekbafna
no worries just help to review and merge.
