Fix table deletion atomicity bug and optimize segment deletion perfor… - #19254
Open
guruguha wants to merge 1 commit into
Open
Fix table deletion atomicity bug and optimize segment deletion perfor…#19254guruguha wants to merge 1 commit into
guruguha wants to merge 1 commit into
Conversation
…mance
Fixed critical bug where DELETE /tables/{tableName} was not atomic, allowing
concurrent deletions to run in parallel with no mutual exclusion. This could
lead to race conditions, metadata corruption, and inconsistent cluster state.
Key changes:
- Added deletion marker znode-based distributed locking mechanism to prevent
concurrent table deletions across controllers
- Enhanced addTable() to check for deletion markers and prevent table recreation
during deletion operations
- Implemented 24-hour expiry on deletion markers to prevent permanent blocking
- Optimized SegmentDeletionManager to batch segment move operations in the
retention>0 path, reducing deletion time from hours to minutes for large tables
- Added logic to honour table config's deletedSegmentsRetentionPeriod setting
- Implemented post-deletion validation to ensure all table-related znodes are
successfully removed before marking deletion complete
- Added comprehensive unit tests for deletion marker functionality
The deletion marker approach provides mutual exclusion for table deletions while
allowing for controller failover scenarios through the expiry mechanism. The
validation step ensures data consistency by detecting partial deletions.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description: Fixed critical bug where DELETE /tables/{tableName} was not atomic, allowing concurrent deletions to run in parallel with no mutual exclusion. This could lead to race conditions, metadata corruption, and inconsistent cluster state.
Key Changes
• Deletion marker znode: Added distributed locking mechanism to prevent concurrent table deletions across controllers
• Enhanced addTable(): Checks for deletion markers and prevents table recreation during deletion operations
• 24-hour expiry: Implemented automatic expiry on deletion markers to prevent permanent blocking
• Performance optimization: Batched segment move operations in retention>0 path, reducing deletion time from hours to minutes
• Configuration honor: Added logic to respect table config's deletedSegmentsRetentionPeriod setting
• Post-deletion validation: Ensures all table-related znodes are successfully removed before marking deletion complete
• Comprehensive testing: Added unit tests for deletion marker functionality
Bug Claims Addressed
✅ No mutual exclusion between concurrent deletions
✅ Proceeds regardless of table existence
✅ Long race window during deletion
✅ Slow performance in retention>0 path
✅ Unconditional metadata removals
✅ Insufficient guards in table creation
Testing
• Added comprehensive unit tests for deletion marker functionality
• All pre-commit checks pass (spotless, checkstyle, license)
• Manual verification of all bug claims from original report
Files Modified
• pinot-common/src/main/java/org/apache/pinot/common/metadata/ZKMetadataProvider.java
• pinot-controller/src/main/java/org/apache/pinot/controller/helix/core/PinotHelixResourceManager.java
• pinot-controller/src/main/java/org/apache/pinot/controller/helix/core/SegmentDeletionManager.java
• pinot-common/src/test/java/org/apache/pinot/common/metadata/TableDeletionMarkerTest.java (new)