-
Notifications
You must be signed in to change notification settings - Fork 0
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
review #6
review #6
Conversation
compact.go
Outdated
} else if startBlock >= 0 { | ||
break | ||
var overlappingTime bool | ||
if d.meta.MinTime < maxtGlobal && d.meta.MaxTime > mintGlobal { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Same comment as populateBlock
ac2c412
to
159cbe3
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
These reviews are for my tracking (just nits). I will fix them myself after merging.
if len(ds) < 2 { | ||
return nil | ||
} | ||
startBlock, endBlock := -1, -1 | ||
prevMaxTime := ds[0].meta.MaxTime | ||
var overlappingMetas []string |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
overlappingMetas
-> overlappingDirs
for i, b := range blocks { | ||
if !overlapping { | ||
if b.MinTime() < lastMaxT { | ||
if i > 0 && b.MinTime() < globalMaxt { | ||
fmt.Println(b.MinTime(), globalMaxt, b.MinTime() < globalMaxt) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
fmt.Println(b.MinTime(), globalMaxt, b.MinTime() < globalMaxt) |
if b.MinTime() < lastMaxT { | ||
if i > 0 && b.MinTime() < globalMaxt { | ||
fmt.Println(b.MinTime(), globalMaxt, b.MinTime() < globalMaxt) | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
db, err := Open(tmpdir, nil, nil, nil) | ||
testutil.Ok(t, err) | ||
db.DisableCompactions() | ||
defer func() { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Have a separate defer
for the tempdir
above (just after creating). Else if creating block or opening db
is failed, we cannot do os.RemoveAll(tmpdir)
.
Signed-off-by: Krasi Georgiev kgeorgie@redhat.com