[Fix](merge-on-write) Should update pending delete bitmap KVs in MS when no need to calc delete bitmaps in publish phase #46039
Conversation
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
|
Thank you for your contribution to Apache Doris. Please clearly describe your PR:
|
|
run buildall |
|
we need to add an additional filed to identify the owner of the pending delete bitmap |
TPC-H: Total hot run time: 32447 ms |
|
TeamCity be ut coverage result: |
TPC-DS: Total hot run time: 190524 ms |
zhannngchen
reviewed
Dec 26, 2024
ClickBench: Total hot run time: 31.56 s |
2bb6847 to
c55511c
Compare
|
run buildall |
|
TeamCity be ut coverage result: |
TPC-H: Total hot run time: 32709 ms |
TPC-DS: Total hot run time: 190186 ms |
ClickBench: Total hot run time: 31.58 s |
|
run buildall |
|
PR approved by at least one committer and no changes requested. |
|
PR approved by anyone and no changes requested. |
TPC-H: Total hot run time: 32722 ms |
TPC-DS: Total hot run time: 190734 ms |
ClickBench: Total hot run time: 31.31 s |
|
TeamCity be ut coverage result: |
bobhan1
added a commit
to bobhan1/doris
that referenced
this pull request
Dec 30, 2024
…hen no need to calc delete bitmaps in publish phase (apache#46039) consider the following situation: 1. Txn A acquires the lock, obtains version X to publish, calculates the delete bitmap, writes the pending delete bitmap KVs to the MS, but fails for some reason before committing the transaction in the MS. 2. Txn B acquires the lock, obtains version X to publish, **cleans up the pending delete bitmap KV written by Txn A**, calculates the delete bitmap, **writes its pending delete bitmap KV to the MS**, but also fails for some reason before committing the transaction in the MS. 3. Txn A then reacquires the lock, obtains version X to publish, and notices that neither the version nor the compaction counts have changed. It will skip the process of calculating the delete bitmap and writing the pending delete bitmap KV to the MS apache#39018 and eventually succeeds in committing the transaction in the MS. In this case, Txn A will save the wrong delete bitmaps(generated by Txn B) in MS and causing correctness problem. To solve the problem, we should still update delete bitmap KVs in MS when we skip the calculation of delete bitmap on BE in publish phase. Also add a defensive check: record `lock_id` when writing pending delete bitmap keys and check if the `lock_id` is correct when commit txn in MS.
bobhan1
added a commit
to bobhan1/doris
that referenced
this pull request
Dec 30, 2024
…hen no need to calc delete bitmaps in publish phase (apache#46039) consider the following situation: 1. Txn A acquires the lock, obtains version X to publish, calculates the delete bitmap, writes the pending delete bitmap KVs to the MS, but fails for some reason before committing the transaction in the MS. 2. Txn B acquires the lock, obtains version X to publish, **cleans up the pending delete bitmap KV written by Txn A**, calculates the delete bitmap, **writes its pending delete bitmap KV to the MS**, but also fails for some reason before committing the transaction in the MS. 3. Txn A then reacquires the lock, obtains version X to publish, and notices that neither the version nor the compaction counts have changed. It will skip the process of calculating the delete bitmap and writing the pending delete bitmap KV to the MS apache#39018 and eventually succeeds in committing the transaction in the MS. In this case, Txn A will save the wrong delete bitmaps(generated by Txn B) in MS and causing correctness problem. To solve the problem, we should still update delete bitmap KVs in MS when we skip the calculation of delete bitmap on BE in publish phase. Also add a defensive check: record `lock_id` when writing pending delete bitmap keys and check if the `lock_id` is correct when commit txn in MS.
16 tasks
zhannngchen
pushed a commit
that referenced
this pull request
Jan 13, 2025
…it txn in MS (#46841) ### What problem does this PR solve? Related PR: #46039 Problem Summary: #46039 add a defensive check when commit_txn in MS to check whether the `lock_id` of pending delete bitmaps on tablets involved in the txn is the current txn's `lock_id`. But this may report a false negative in the following circumstance: 1. heavy schema change begins and add shadow index to table. 2. txn A load data to base index and shadow index. 3. txn A write its pending delete bitmaps on MS. This includes tablets of base index and shadow index. 4. txn A failed to remove its pending delete bitmaps for some reson(e.g. `commit_txn()` failed due to too large value) 5. txn B load data to base index and shadow index. 6. schema change failed for some reason and **remove shadow index on table.** 7. txn B send delete bitmap calculation task to BE. **Note that this will not involved tablets under shadow index because these tablets have been dropped.** **So these tablets' pending delete bitmaps will still be txn A's**. 8. txn B commit txn on MS and find that pending delete bitmaps' `lock_id` on tablets under shadow index not match. And txn B will failed. We can see that the checks on these dropped tablets are useless so we remove the mandatory check to avoid this false negative and print a warning log instead to help locate problems.
bobhan1
added a commit
to bobhan1/doris
that referenced
this pull request
Jan 13, 2025
…it txn in MS (apache#46841) Related PR: apache#46039 Problem Summary: apache#46039 add a defensive check when commit_txn in MS to check whether the `lock_id` of pending delete bitmaps on tablets involved in the txn is the current txn's `lock_id`. But this may report a false negative in the following circumstance: 1. heavy schema change begins and add shadow index to table. 2. txn A load data to base index and shadow index. 3. txn A write its pending delete bitmaps on MS. This includes tablets of base index and shadow index. 4. txn A failed to remove its pending delete bitmaps for some reson(e.g. `commit_txn()` failed due to too large value) 5. txn B load data to base index and shadow index. 6. schema change failed for some reason and **remove shadow index on table.** 7. txn B send delete bitmap calculation task to BE. **Note that this will not involved tablets under shadow index because these tablets have been dropped.** **So these tablets' pending delete bitmaps will still be txn A's**. 8. txn B commit txn on MS and find that pending delete bitmaps' `lock_id` on tablets under shadow index not match. And txn B will failed. We can see that the checks on these dropped tablets are useless so we remove the mandatory check to avoid this false negative and print a warning log instead to help locate problems.
16 tasks
bobhan1
added a commit
to bobhan1/doris
that referenced
this pull request
Jan 21, 2025
…_txn()` (apache#47136) Related PR: apache#46039 Problem Summary: apache#46039 introduce an defensive check when `commit_txn()`, but this may influence the commit process. This PR remove this check totally to eliminate this overhead.
bobhan1
added a commit
to bobhan1/doris
that referenced
this pull request
Jan 22, 2025
…_txn()` (apache#47136) Related PR: apache#46039 Problem Summary: apache#46039 introduce an defensive check when `commit_txn()`, but this may influence the commit process. This PR remove this check totally to eliminate this overhead.
BiteTheDDDDt
pushed a commit
to BiteTheDDDDt/incubator-doris
that referenced
this pull request
Feb 7, 2025
…hen no need to calc delete bitmaps in publish phase apache#46039 (apache#3722)
lzyy2024
pushed a commit
to lzyy2024/doris
that referenced
this pull request
Feb 21, 2025
…_txn()` (apache#47136) ### What problem does this PR solve? Related PR: apache#46039 Problem Summary: apache#46039 introduce an defensive check when `commit_txn()`, but this may influence the commit process. This PR remove this check totally to eliminate this overhead.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
What problem does this PR solve?
consider the following situation:
In this case, Txn A will save the wrong delete bitmaps(generated by Txn B) in MS and causing correctness problem.
To solve the problem, we should still update delete bitmap KVs in MS when we skip the calculation of delete bitmap on BE in publish phase.
Also add a defensive check: record
lock_idwhen writing pending delete bitmap keys and check if thelock_idis correct when commit txn in MS.Release note
None
Check List (For Author)
Test
Behavior changed:
Does this need documentation?
Check List (For Reviewer who merge this PR)