Skip to content

release/v20.07 - Fix(cleanup): Avoid truncating in vlog.Open on error (#1465)#1489

Merged
jarifibrahim merged 1 commit intorelease/v2.2007from
ibrahim/r2.2007-vlog-woffset-fix
Aug 27, 2020
Merged

release/v20.07 - Fix(cleanup): Avoid truncating in vlog.Open on error (#1465)#1489
jarifibrahim merged 1 commit intorelease/v2.2007from
ibrahim/r2.2007-vlog-woffset-fix

Conversation

@jarifibrahim
Copy link
Contributor

@jarifibrahim jarifibrahim commented Aug 27, 2020

The vlog.Open() function can return an error denoting that the open was
unsuccessful but we have db.cleanup which will be called to stop all
the running go routines in case badger couldn't start. The db.cleanup
function calls vlog.Close() which will truncate the maxFid vlog file
based on the vlog.writableLogOffset. The vlog.writableLogOffset was not
being updated in case open failed. As a result of this, we will end up
truncating the vlog file to lenght 0 if open fails.

This PR fixes this by using vlog.stopDiscardStatFlush() instead of vlog.close.

(cherry picked from commit ed3b219)


This change is Reviewable

* Fix(cleanup): Avoid truncating in value.Open on error

The vlog.Open() function can return an error denoting that the open was
unsuccessful but we have `db.cleanup` which will be called to stop all
the running go routines in case badger couldn't start. The db.cleanup
function calls vlog.Close() which will truncate the maxFid vlog file
based on the vlog.writableLogOffset. The vlog.writableLogOffset was not
being updated in case open failed. As a result of this, we will end up
truncating the vlog file to lenght 0 if open fails.

This PR fixes this by using vlog.stopDiscardStatFlush() instead of vlog.close.

(cherry picked from commit ed3b219)
Copy link
Contributor

@ashish-goswami ashish-goswami left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reviewed 2 of 2 files at r1.
Reviewable status: :shipit: complete! all files reviewed, all discussions resolved (waiting on @manishrjain)

Copy link
Contributor

@ashish-goswami ashish-goswami left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

:lgtm:

Reviewable status: :shipit: complete! all files reviewed, all discussions resolved (waiting on @manishrjain)

@jarifibrahim jarifibrahim changed the title Fix(cleanup): Avoid truncating in value.Open on error (#1465) release/v20.07 - Fix(cleanup): Avoid truncating in value.Open on error (#1465) Aug 27, 2020
@jarifibrahim jarifibrahim changed the title release/v20.07 - Fix(cleanup): Avoid truncating in value.Open on error (#1465) release/v20.07 - Fix(cleanup): Avoid truncating in vlog.Open on error (#1465) Aug 27, 2020
@jarifibrahim jarifibrahim merged commit b41e77a into release/v2.2007 Aug 27, 2020
@jarifibrahim jarifibrahim deleted the ibrahim/r2.2007-vlog-woffset-fix branch August 27, 2020 13:02
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Development

Successfully merging this pull request may close these issues.

2 participants