WT-12693 Use bounded cursor for history store #10508
Conversation
quchenhao
requested review from
luke-pearson,
kommiharibabu,
bvpvamsikrishna and
agorrod
|
This is a POC of using bounded cursor for history store. We can optimize based on this initial implementation. |
| @@ -283,7 +283,7 @@ __curfile_reset(WT_CURSOR *cursor) | |||
| * guarding the call to cursor bound reset with the API_USER_ENTRY macro. Doing so prevents | |||
| * internal API calls from resetting cursor bounds unintentionally, e.g. cursor->remove. | |||
| */ | |||
| if (API_USER_ENTRY(session)) | |||
| if (WT_IS_HS(session->dhandle) || API_USER_ENTRY(session)) | |||
There was a problem hiding this comment.
I assume this is done to allow for the history store cursor's reset logic to still reset the bounds
There was a problem hiding this comment.
Yes, that's correct.
src/cursor/cur_hs.c
Outdated
| } | ||
| } | ||
| } | ||
| WT_RET(cursor->bound(cursor, "bound=lower,inclusive=true")); |
There was a problem hiding this comment.
In order for this to be performant, we need to use precompiled config. See bounded_cursor_perf.cpp for an example.
src/cursor/cur_hs.c
Outdated
| } | ||
| WT_RET(cursor->bound(cursor, "bound=lower,inclusive=true")); | ||
| WT_RET(cursor->next(cursor)); | ||
| WT_RET(cursor->bound(cursor, "action=clear")); |
There was a problem hiding this comment.
The clear config should be compiled too.
| @@ -630,6 +631,13 @@ __cursor_row_prev( | |||
| key->data = WT_INSERT_KEY(ins); | |||
| key->size = WT_INSERT_KEY_SIZE(ins); | |||
|
|
|||
| if (WT_IS_HS(session->dhandle) && F_ISSET(&cbt->iface, WT_CURSTD_BOUND_UPPER)) { | |||
There was a problem hiding this comment.
I understand why this is needed but I can't say I'm a fan of it. I think we'll need to think of a more generalizable solution.
There was a problem hiding this comment.
I'm not either. Do you have any better idea?
There was a problem hiding this comment.
At this stage my only thought is wondering if we can go a layer down and write a __cursor_row_search_before / after that intentionally places the cursor at the lowest level of WiredTiger. That sounds tedious and not like what we'd want right now. At the very least we should move this into its own function, I'll keep thinking, and have looped in Jie.
| @@ -449,6 +450,13 @@ __cursor_row_next( | |||
| key->data = WT_INSERT_KEY(ins); | |||
| key->size = WT_INSERT_KEY_SIZE(ins); | |||
|
|
|||
| if (WT_IS_HS(session->dhandle) && F_ISSET(&cbt->iface, WT_CURSTD_BOUND_LOWER)) { | |||
There was a problem hiding this comment.
There is already a function to check the bounds below. __wt_btcur_bounds_early_exit, If that function doesn't satisfy the requirement for HS, how about changing it to support HS also instead of adding separate code here?
| @@ -501,6 +509,13 @@ __cursor_row_next( | |||
| */ | |||
| WT_RET(__cursor_row_slot_key_return(cbt, rip, &kpack)); | |||
|
|
|||
| if (WT_IS_HS(session->dhandle) && F_ISSET(&cbt->iface, WT_CURSTD_BOUND_LOWER)) { | |||
|
Jie and I have discussed this ticket and we both would like to explore alternate solutions to read uncommitted isolation support. We will create a ticket to track that work separately then revisit this. Is that okay @quchenhao? |
SGTM! |
No description provided.