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.

Sign up for GitHub

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

Jump to bottom

[BUG]: *DB.Sync should sync memtable in addition to value log #1847

Closed
mcfog opened this issue Jan 18, 2023 · 1 comment · Fixed by #1953
Closed

[BUG]: *DB.Sync should sync memtable in addition to value log #1847

mcfog opened this issue Jan 18, 2023 · 1 comment · Fixed by #1953
Labels
kind/bug Something is broken.

Comments

@mcfog
Copy link

mcfog commented Jan 18, 2023

What version of Badger are you using?

github.com/dgraph-io/badger/v3 v3.2103.5

What version of Go are you using?

No response

Have you tried reproducing the issue with the latest release?

Yes

What is the hardware spec (RAM, CPU, OS)?

What steps will reproduce the bug?

call *DB.Sync()

Expected behavior and actual result.

Both memtable('s wal file) and value log should be persisted by Sync call.

Additional information

No response
@mcfog mcfog added the kind/bug Something is broken. label Jan 18, 2023
@mangalaman93
Copy link
Contributor

This seems reasonable to me. If you could raise a PR with a test that proves the issue, that'd be awesome. Otherwise, we will try to get to it as soon as we can. Thanks

SarthakMakhija added a commit to SarthakMakhija/badger that referenced this issue May 22, 2023
@SarthakMakhija
Sync active memtable and value log on Db.Sync (dgraph-io#1847)
9b6fc27
@SarthakMakhija SarthakMakhija mentioned this issue May 22, 2023
Merged
SarthakMakhija added a commit to SarthakMakhija/badger that referenced this issue May 22, 2023
@SarthakMakhija
Add lock during DB.Sync operation and apply linting feedback from the…
dc82e95 
… CI (dgraph-io#1847)
SarthakMakhija added a commit to SarthakMakhija/badger that referenced this issue May 22, 2023
@SarthakMakhija
Correct the logic to combine memtable and vlog sync errors and add te…
ad59b77 
…sts for the same (dgraph-io#1847)
SarthakMakhija added a commit to SarthakMakhija/badger that referenced this issue May 22, 2023
@SarthakMakhija
Changed the format specifier in CombineErrors, internal error details…
9f81ece 
… are not revealed (dgraph-io#1847)
SarthakMakhija added a commit to SarthakMakhija/badger that referenced this issue May 22, 2023
@SarthakMakhija
Sarthak | Changed the ordering of the import packages (dgraph-io#1847)
70ffdb0
mangalaman93 pushed a commit that referenced this issue Jun 3, 2023
@SarthakMakhija
Sync active memtable and value log on Db.Sync (#1847) (#1953)
4b0d919 
Fixes #1847

Currently, `DB.Sync()` only syncs the value log but not the WAL of the
active memtable, however recovery happens from the WAL of the active
memtable as a part of the `DB.Open()` method.

This change attempts to sync both the logs, however there can be an
issue in syncing one of the logs. This change lists all of the possible
cases that can arise during the sync operations. Some of these issues
will be taken separately. For example,
the issue #1954 can arise and
shall be handled separately.

This change adds a few tests to ensure that the Sync behavior is not
broken. This change also adds a learning test `(db2_test.go ->
TestAssertValueLogIsNotWrittenToOnStartup)` to ensure that value log is
only read from (and not written to) during startup. This learning shall
be used in the next issue (the one described above (*))
@requilence requilence mentioned this issue Nov 20, 2023
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
kind/bug Something is broken.
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

Sync active memtable and value log on Db.Sync (#1847) SarthakMakhija/badger
2 participants
@mcfog @mangalaman93