HIVE-29210: Minor compaction produces duplicates conditionally in cas… #6101
Conversation
…e of HMS instance running initiator crash
| return 1; | ||
| } | ||
| } | ||
| else if (visibilityTxnId != parsedDelta.visibilityTxnId) { |
There was a problem hiding this comment.
else if (visibilityTxnId != parsedDelta.visibilityTxnId) {
return visibilityTxnId < parsedDelta.visibilityTxnId ? 1 : -1;
}
There was a problem hiding this comment.
we shouldn't allow concurrent compaction with above properties
asf-ci-hive
added
tests pending
tests passed
and removed
tests passed
tests pending
labels
|
Hi @tanishq-chugh , |
…ction job concurrently
asf-ci-hive
added
tests unstable
tests pending
and removed
tests pending
tests unstable
labels
|
kuczoram
left a comment
kuczoram
left a comment
There was a problem hiding this comment.
Thanks a lot @tanishq-chugh! It looks good to me.
|
hi @tanishq-chugh, how could we get into the situation with concurrent minor compactions? If that is related to compaction running on HMS I already mentioned to you, that we should just drop the support (it's already deprecated) |
| && next.minWriteId == prev.minWriteId | ||
| && next.statementId == prev.statementId) { | ||
| && next.statementId == prev.statementId | ||
| && (next.isDeleteDelta || prev.isDeleteDelta)) { |
|
Hi @deniskuzZ |
|
|
||
| // Overlapping compacted deltas with different visibilityTxnIDs simulating concurrent compaction from two workers | ||
| addDeltaFile(t, null, 22L, 23L, 2, 24); | ||
| addDeltaFile(t, null, 22L, 23L, 2, 25); |
There was a problem hiding this comment.
I don't think we should allow this to happen in first place
@tanishq-chugh HIVE-29210 mentions HMS local workers |
|
@deniskuzZ It mentions initiator in HMS but worker threads within HS2. |
|
@tanishq-chugh, revokeFromLocalWorkers was only needed when HMS had local workers. As we remove support it should be just dropped, instead of adding workarounds in a code |
5 participants
…e of HMS instance running initiator crash
What changes were proposed in this pull request?
Compactor cleaner fix to address duplicate directories created from multiple jobs running same compaction
Why are the changes needed?
To address the race condition where multiple jobs running same compaction leads to duplicate data
Does this PR introduce any user-facing change?
No
How was this patch tested?
Manual Testing after reproducing the issue on a deployed cluster