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

leveldb: add a Sync method #310

Closed
wants to merge 1 commit into from
Closed

Conversation

Stebalien
Copy link

Put + Sync is the equivalent of calling Put with WriteOptions.Sync.

We'd like this method for go-ipfs as we're adding a Sync(keyPrefix string) method to our datastore abstraction. We considered adding a Sync option to the Put method as leveldb does but:

  • It's not always possible to know when a write is the "final" write before actually performing the write.
  • We'd like to be able to "sync" different sub-datastores independently (i.e., sync everything under keyPrefix).

Put + Sync is the equivalent of calling Put with WriteOptions.Sync. The
difference is that Sync can be called after all writes are complete to persist
previous writes without requiring a new write.
@lrita
Copy link
Contributor

lrita commented Dec 2, 2019

The Sync method should acquire writing-lock(writeLockC) to keep thread safe.

@Stebalien
Copy link
Author

That depends on the guarantees of the Syncer interface. The current implementations (os.File, memWriter, fileWrap, writer) are all internally thread-safe and interleaving syncs with other writes shouldn't cause any problems.

I'm happy to take the lock anyways but I'd rather not stop the world unnecessarily.

@lrita
Copy link
Contributor

lrita commented Dec 2, 2019

Your patch will cause panic by getting nil of db.journalWriter without acquire writing-lock(writeLockC), it is easy the find.

@Stebalien
Copy link
Author

Stebalien commented Dec 2, 2019 via email

@lrita
Copy link
Contributor

lrita commented Dec 3, 2019

goleveldb will auto rotate journalWriter internally.

If you want implement the sync barrier during a lot of async writing, you will do a lot modification, you need find all the un-compaction journal files, and sync each of them.

@Stebalien
Copy link
Author

Yeah. This is going to need some discussion.

@Stebalien Stebalien closed this Dec 3, 2019
@Stebalien Stebalien deleted the feat/sync branch December 3, 2019 14:26
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants