Skip to content

Remove vlog file if bootstrap, syncDir or mmap fails #1434

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

Merged
merged 2 commits into from
Jul 17, 2020

Conversation

jarifibrahim
Copy link
Contributor

@jarifibrahim jarifibrahim commented Jul 16, 2020

The createVlogFile function creates a new vlog file. The function apart
from creating a new file, bootstraps it, syncs it to the disk, and mmaps it.

If any of the three operations fail, we will end up with a file on the
disk but the vlog.maxFid will not be updated.

On the next attempt to create the vlog files, we will get a "File
already exists" error. This commit fixes this issue by removing the file
if createVlogFile encounters any error.

Fixes DGRAPH-1930


This change is Reviewable

The createVlogFile function creates a new vlog file. The function apart
from creating a new file, bootstraps it, syncs it to the disk and mmaps
it.

If any of the three operations fail, we will end up with a file on the
disk but the vlog.maxFid will not be updated.

On the next attempt to create the vlog files, we will get a "File
already exists" error. This commit fixes this issue by removing the file
if createVlogFile encounters any error.
@@ -1000,14 +1000,26 @@ func (vlog *valueLog) createVlogFile(fid uint32) (*logFile, error) {
return nil, errFile(err, lf.path, "Create value log file")
}

removeFile := func() {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Add test for this.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I tried to add a test by setting ulimit but this will be difficult for two reasons

  1. ulimit is set per process. If I set the ulimit for a test, it might affect other tests too.
  2. It's hard to set the correct ulimit and the test wouldn't fail even if the limit is off by 1.

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: 0 of 1 files reviewed, 1 unresolved discussion (waiting on @ashish-goswami and @manishrjain)

Copy link
Contributor

@NamanJain8 NamanJain8 left a comment

Choose a reason for hiding this comment

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

Looks good!!

@jarifibrahim jarifibrahim merged commit dfcca75 into master Jul 17, 2020
@jarifibrahim jarifibrahim deleted the ibrahim/vlog-file-exists branch July 17, 2020 10:50
jarifibrahim pushed a commit that referenced this pull request Jul 17, 2020
The createVlogFile function creates a new vlog file. The function apart
from creating a new file, bootstraps it, syncs it to the disk, and mmaps it.

If any of the three operations fail, we will end up with a file on the
disk but the vlog.maxFid will not be updated.

On the next attempt to create the vlog files, we will get a "File
already exists" error. This commit fixes this issue by removing the file
if createVlogFile encounters any error.

Fixes DGRAPH-1930

(cherry picked from commit dfcca75)
jarifibrahim pushed a commit that referenced this pull request Jul 29, 2020
The createVlogFile function creates a new vlog file. The function apart
from creating a new file, bootstraps it, syncs it to the disk, and mmaps it.

If any of the three operations fail, we will end up with a file on the
disk but the vlog.maxFid will not be updated.

On the next attempt to create the vlog files, we will get a "File
already exists" error. This commit fixes this issue by removing the file
if createVlogFile encounters any error.

Fixes DGRAPH-1930
jarifibrahim pushed a commit that referenced this pull request Aug 17, 2020
The createVlogFile function creates a new vlog file. The function apart
from creating a new file, bootstraps it, syncs it to the disk, and mmaps it.

If any of the three operations fail, we will end up with a file on the
disk but the vlog.maxFid will not be updated.

On the next attempt to create the vlog files, we will get a "File
already exists" error. This commit fixes this issue by removing the file
if createVlogFile encounters any error.

Fixes DGRAPH-1930

(cherry picked from commit dfcca75)
jarifibrahim pushed a commit that referenced this pull request Aug 18, 2020
The createVlogFile function creates a new vlog file. The function apart
from creating a new file, bootstraps it, syncs it to the disk, and mmaps it.

If any of the three operations fail, we will end up with a file on the
disk but the vlog.maxFid will not be updated.

On the next attempt to create the vlog files, we will get a "File
already exists" error. This commit fixes this issue by removing the file
if createVlogFile encounters any error.

Fixes DGRAPH-1930

(cherry picked from commit dfcca75)
jarifibrahim pushed a commit that referenced this pull request Sep 9, 2020
The createVlogFile function creates a new vlog file. The function apart
from creating a new file, bootstraps it, syncs it to the disk, and mmaps it.

If any of the three operations fail, we will end up with a file on the
disk but the vlog.maxFid will not be updated.

On the next attempt to create the vlog files, we will get a "File
already exists" error. This commit fixes this issue by removing the file
if createVlogFile encounters any error.

Fixes DGRAPH-1930

(cherry picked from commit dfcca75)
jarifibrahim pushed a commit that referenced this pull request Sep 10, 2020
The createVlogFile function creates a new vlog file. The function apart
from creating a new file, bootstraps it, syncs it to the disk, and mmaps it.

If any of the three operations fail, we will end up with a file on the
disk but the vlog.maxFid will not be updated.

On the next attempt to create the vlog files, we will get a "File
already exists" error. This commit fixes this issue by removing the file
if createVlogFile encounters any error.

Fixes DGRAPH-1930

(cherry picked from commit dfcca75)
jarifibrahim pushed a commit that referenced this pull request Oct 2, 2020
The createVlogFile function creates a new vlog file. The function apart
from creating a new file, bootstraps it, syncs it to the disk, and mmaps it.

If any of the three operations fail, we will end up with a file on the
disk but the vlog.maxFid will not be updated.

On the next attempt to create the vlog files, we will get a "File
already exists" error. This commit fixes this issue by removing the file
if createVlogFile encounters any error.

Fixes DGRAPH-1930
mYmNeo pushed a commit to mYmNeo/badger that referenced this pull request Jan 16, 2023
…) (hypermodeinc#1515)

The createVlogFile function creates a new vlog file. The function apart
from creating a new file, bootstraps it, syncs it to the disk, and mmaps it.

If any of the three operations fail, we will end up with a file on the
disk but the vlog.maxFid will not be updated.

On the next attempt to create the vlog files, we will get a "File
already exists" error. This commit fixes this issue by removing the file
if createVlogFile encounters any error.

Fixes DGRAPH-1930

(cherry picked from commit dfcca75)
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.

3 participants