WT-12178: bug fix, __wt_block_extlist_merge has potential risks #10015
Conversation
This file contains 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
Record how many pages we want and how many pages we have queued so far in a tree, then resume the walk next iteration. This avoids a single tree with a target larger than the queue size being walked completely before the eviction server moves on to the next tree. (cherry picked from commit fca6a8d)
…s. (wiredtiger#3826) (cherry picked from commit 2f489ea)
There's trickiness in the page-pinned check. By definition a remove operation leaves a cursor positioned if it's initially positioned. However, if every item on the page is deleted and we unpin the page, eviction might delete the page and our search will re-instantiate an empty page for us. Cursor remove returns not-found whether or not that eviction/deletion happens and it's OK unless cursor-overwrite is configured (which means we return success even if there's no item to delete). In that case, we'll fail when we try to point the cursor at the key on the page to satisfy the positioned requirement. It's arguably safe to simply leave the key initialized in the cursor (as that's all a positioned cursor implies), but it's probably safer to avoid page eviction entirely in the positioned case. (cherry picked from commit 2ac616e)
…dtiger#4017) This reverts commit 565496e.
(cherry picked from commit 9ba3e90)
|
Hi @y123456yz, thank you for your submission! |
y123456yz
changed the title
|
@y123456yz I don't think this is broken, risky, or worth fixing sorry. Merging extent lists is an operation that we expect to alter the extent lists that we pass in. Even if we wanted to make this change, I don't think it's useful to mark fields of a structure with |
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.
__wt_block_extlist_merge has potential risks.
in function __wt_block_extlist_merge, We switched a and b，this may leads to the following potential risks:
in PR: #9896
https://github.com/y123456yz/wiredtiger/blob/develop_WT_EXTLIST_last_bug_fix/src/block/block_ext.c#L955-L956
Because of this problem, some unit test run failed, it took me a lot of time to locate. So it's worth adding some comments and WT_UNUSED here.