Skip to content
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

Use WAL instead of Vlog for crash recovery. #1535

Closed
wants to merge 25 commits into from
Closed

Conversation

NamanJain8
Copy link
Contributor

@NamanJain8 NamanJain8 commented Sep 21, 2020

Fixes DGRAPH-2177

This PR separates smaller values from vlog and instead stores it in WAL.
This leads to minimization of disk usage as WAL files can be easily cleaned.
Smaller values ( <ValueThreshold ): WAL -> LSM
Bigger values : WAL -> Vlog -> LSM


This change is Reviewable

@NamanJain8 NamanJain8 marked this pull request as ready for review September 29, 2020 07:56
Copy link

@codelingo codelingo bot left a comment

Choose a reason for hiding this comment

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

LGTM. 1 rules errored during the review.

Copy link
Contributor

@jarifibrahim jarifibrahim left a comment

Choose a reason for hiding this comment

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

Delete all wal files on successful db close.

require.NoError(t, y.Munmap(db.vlog.filesMap[db.vlog.maxFid].fmap))
for _, f := range db.vlog.filesMap {
require.NoError(t, y.Munmap(db.vlog.vlog.filesMap[db.vlog.vlog.maxFid].fmap))
for _, f := range db.vlog.vlog.filesMap {
Copy link
Contributor

Choose a reason for hiding this comment

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

Call vlog as logManager

db2_test.go Show resolved Hide resolved
@@ -861,7 +949,7 @@ func (db *DB) writeRequests(reqs []*request) error {
return errors.Wrap(err, "writeRequests")
}
db.Lock()
db.updateHead(b.Ptrs)
db.updateHead([]valuePointer{b.head})
Copy link
Contributor

Choose a reason for hiding this comment

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

Add a comment.

db.go Outdated
@@ -137,7 +217,12 @@ func (db *DB) replayFunction() func(Entry, valuePointer) error {
nv = make([]byte, len(e.Value))
copy(nv, e.Value)
} else {
nv = vp.Encode()
// Write to vlog and get the value pointer to vlog
vlogP, err := toVlog(&e)
Copy link
Contributor

Choose a reason for hiding this comment

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

We'll need a test for this.

db_test.go Show resolved Hide resolved
value.go Show resolved Hide resolved
value_test.go Outdated Show resolved Hide resolved
value_test.go Show resolved Hide resolved
value_test.go Outdated Show resolved Hide resolved
value.go Outdated Show resolved Hide resolved
Copy link

@codelingo codelingo bot left a comment

Choose a reason for hiding this comment

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

LGTM. 1 rules errored during the review.

Copy link

@codelingo codelingo bot left a comment

Choose a reason for hiding this comment

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

LGTM. 1 rules errored during the review.

@stale
Copy link

stale bot commented Oct 31, 2020

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

@stale stale bot added the status/stale The issue hasn't had activity for a while and it's marked for closing. label Oct 31, 2020
@NamanJain8
Copy link
Contributor Author

Closing this as this is not needed now. #1555 was merged which fixes the issue.

@NamanJain8 NamanJain8 closed this Nov 4, 2020
@NamanJain8 NamanJain8 deleted the naman/wal branch November 4, 2020 12:12
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
status/stale The issue hasn't had activity for a while and it's marked for closing.
Development

Successfully merging this pull request may close these issues.

2 participants