HIVE-29587: Cleanup acid table dir after async drop#6459
HIVE-29587: Cleanup acid table dir after async drop#6459VenuReddy2103 wants to merge 1 commit intoapache:masterfrom
Conversation
asf-ci-hive
added
tests pending
tests passed
and removed
tests unstable
tests pending
labels
VenuReddy2103
changed the title
|
thanks for the refactor @VenuReddy2103 , that looks much cleaner |
| isReplicated = isDbReplicationTarget(db); | ||
|
|
||
| EnvironmentContext context = request.getEnvContext(); | ||
| if (!request.isDeleteData() && context != null && ReplChangeManager.isSourceOfReplication(db)) { |
There was a problem hiding this comment.
what does request.isDeleteData() do? do we need it
There was a problem hiding this comment.
Since it is enough to set only when request.isDeleteData() is false(i.e., soft delete), have added this check. But there is no harm setting it in context irrespective of it since we do not use it. Do you suggest to remove it?
| if (isSoftDelete) { | ||
| context = new EnvironmentContext(); | ||
| context.putToProperties(hive_metastoreConstants.TXN_ID, String.valueOf(request.getTxnId())); | ||
| if (ReplChangeManager.isSourceOfReplication(db)) { |
There was a problem hiding this comment.
Yes it is redundant to set here since we set in DropTableHandler. And it need to present in DropTableHandler. When drop soft table is followed by drop database cascade, we won't preseve this info in CQ_TBLPROPERTIES if we do not set in DropTableHandler.
|
with not in scope for this PR |
|
3 participants
What changes were proposed in this pull request?
Added a new compaction type(
DEFERRED_CLEANUP). It is used while adding a row to compaction queue table when table is soft deleted.Why are the changes needed?
When drop acid table is followed by drop database cascade operation, the acid table directory itself is never deleted. Hence managed database directory is also never deleted. Orphaned table and database directories remaining indefinitely in the filesystem, leading to potential storage bloat and inconsistencies between HMS metadata and filesystem state
Does this PR introduce any user-facing change?
No
How was this patch tested?
Tested manually