WT-13912 Create a new checkpoint struct dedicated to handles information #11347
Conversation
|
Thanks for creating a pull request! Please answer the questions below by editing this comment. Type of change made in this PR
What makes this change safe?Answering this question helps the reviewers understand where they should focus their attention. Please consider these prompts:
References: Checklist before requesting a review
|
etienneptl
changed the title
src/checkpoint/checkpoint.h
Outdated
| wt_shared wt_off_t logsize; /* Checkpoint log size period */ | ||
| bool signalled; /* Checkpoint signalled */ | ||
|
|
||
| WT_CKPT_HANDLE handle; |
There was a problem hiding this comment.
Should I make this plural?
| WT_CKPT_HANDLE handle; | |
| WT_CKPT_HANDLES handles; /* Checkpoint handles */ |
There was a problem hiding this comment.
The content of the struct doesn't look like there are many handles.
And it doesn't seem to be a handle. I'd rather call it "stats" or something like that.
There was a problem hiding this comment.
I am not sure if I understand correctly your comment or I misunderstood the fields. AFAIK, those fields reflect stats (indeed) about all the dhandles the checkpoint is currently dealing with. It could be one or thousands.
There was a problem hiding this comment.
Sorry, after seeing @sueloverso's comment and looking at other structures we have in the code, your comment makes sense @ershov, sorry.
@sueloverso, it seems that we use *_info much less than *_stats, would you be ok with WT_CKPT_HANDLE_STATS?
There was a problem hiding this comment.
Renamed in 6f129f9
|
Test coverage is ok, please refer to the Code change/coverage report links below and try to improve it if feasible.
|
src/checkpoint/checkpoint.h
Outdated
| struct __wt_ckpt_handle_stats { | ||
| uint64_t apply; /* handles applied */ | ||
| uint64_t apply_time; /* applied handles gather time */ | ||
| uint64_t drop; /* handles drop */ |
There was a problem hiding this comment.
Some of these are past tense and others are present, could we change them to be consistent. I don't mind which way.
There was a problem hiding this comment.
Updated in be24b26
etienneptl
force-pushed
the
WT-13912-create-ckpt-struct-handles
branch
from
9ec3e7e to
d4d1908
Compare
The changes introduce a new struct called
WT_CKPT_HANDLE_STATSthat contains all the information related to handles needed by the checkpoint. The existingWT_CKPT_CONNECTIONstruct has been updated to use the new struct.