Enhance check functionality to support checking starting from a pageId #661
Conversation
| } | ||
| case p.IsLeafPage(): | ||
| for i := range p.LeafPageElements() { | ||
| elem := p.LeafPageElement(uint16(i)) |
There was a problem hiding this comment.
IMO, we should try to get key/value here because the ksize/vsize might be wrong here
There was a problem hiding this comment.
It's a new checking, let's take care of it in a separate PR. Also it shouldn't be included in this method (each function/method should do only one thing); instead we can add a separate invariant checker in checkInvariantProperties, or gets it included in the existing recursivelyCheckPageKeyOrder (needs to change the method name if we follow this approach).
tx_check.go
Outdated
| // Close the channel to signal completion. | ||
| defer close(ch) |
There was a problem hiding this comment.
IMO, closing of channel should be the responsibility of caller.
There was a problem hiding this comment.
sort of agreeing on this, but not a big deal.
Let's take care of in a followup PR with change in the method Check something like below,
go func() {
defer close(ch)
tx.check(chkConfig, ch)
}()
| case p.IsBranchPage(): | ||
| for i := range p.BranchPageElements() { | ||
| elem := p.BranchPageElement(uint16(i)) | ||
| tx.recursivelyCheckBucketInPage(elem.Pgid(), reachable, freed, kvStringer, ch) |
There was a problem hiding this comment.
I'm just trying to understand:
Can you explain me the relationship b/w page and buckets or you can point out to me some article. Thanks
There was a problem hiding this comment.
AFAIU, pages holds the bucket.
Can't it be possible that 1 page(Page size: 4KB) unable to hold whole bucket as bucket size is bigger than 4KB
There was a problem hiding this comment.
Conceptually, a bucket is similar a table in RDBMS, a bucket can contain key/value pairs. But from implementation perspective, a bucket is actually a key/value pair, in which the key is the bucket name, and the value is the,
- for nested bucket (which contains very few data), it contains a nested page, in which all data of that bucket are contained;
- for normal bucket, the value just contains a pageId, which points to the page in which all data are included.
I may consider to write some document to make it clearer.
Signed-off-by: Benjamin Wang <[email protected]>
ahrtr
force-pushed
the
enhance_check_20231231
branch
from
d772fee to
37ad8fa
Compare
|
Superseded by #659 |
Link to #580
Only support specifying pageId in this PR. The reason of giving up supporting elementIdx is
@ishan16696 @Elbehery @fuweid Please let me know if you have any comments on this PR.
@Elbehery Please feel free to cherry-pick the commit in this PR and add test cases in your PR.