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

Add option to compact leveldb to repo fsck #3927

Open
kevina opened this issue May 17, 2017 · 14 comments
Open

Add option to compact leveldb to repo fsck #3927

kevina opened this issue May 17, 2017 · 14 comments
Labels
help wanted Seeking public contribution on this issue

Comments

@kevina
Copy link
Contributor

kevina commented May 17, 2017

See #3763 for context.

@kevina kevina self-assigned this May 17, 2017
@jefft0
Copy link
Contributor

jefft0 commented May 18, 2017

Will it be possible to run fsck to compact while the daemon is running (online) in another terminal? (In my steady-state use case, the daemon is running in one terminal to serve webcam files, and another terminal adds new files every hour, which can occasionally call fsck to compact since the online daemon doesn't seem to do it.)

@Kubuxu
Copy link
Member

Kubuxu commented May 18, 2017

@jefft0 when daemon is running the leveldb is autocompacted (low priority background routine of leveldb), in theory.

As you have shown in #3763 adding a lot of files with no-copy without daemon running means problems with compaction.

@jefft0
Copy link
Contributor

jefft0 commented May 18, 2017

In my experience, the low priority compaction didn't happen and my add script failed. I need to remove the uncertainty of "maybe will compact." That's why I asked, can another script do fsck compact while the daemon is running online?

@Kubuxu
Copy link
Member

Kubuxu commented May 18, 2017

That depends on how @kevina implements it. If it isn't much of an issue, I think it should be possible to do it online.

@kevina
Copy link
Contributor Author

kevina commented May 18, 2017

It should be possible to call CompactRange with other go route are accessing the database, although it might block them until it finishes (it seams to get a write lock). https://github.com/syndtr/goleveldb/blob/master/leveldb/db_write.go#L388

If it should be implemented with the daemon running then perhaps adding as part of repo fsck is not the best option. @whyrusleeping

In any case the main problem is exposing the underlying LevelDB to do this cleanly go-ds-leveldb will likely need to be modified.

@kevina
Copy link
Contributor Author

kevina commented May 18, 2017

Related to the original problem but not this issue, I am concerned on why auto compaction was not triggered. Is these really something that runs in the background and needs the process to run for a while in order to trigger, or is something about our usage causing it to not trigger, there are a bunch of parameters controlling compaction and perhaps they need to be adjusted for our use case. Or perhaps the problem is fixed in the latest version of leveldb (we haven't upgraded in a while). @whyrusleeping thoughts.

@whyrusleeping
Copy link
Member

Really not sure the exact circumstances that led to the compaction not being run, It was likely compounding failures of some sort. ipfs add was probably without a daemon at first, generating a large number of leveldb files (with no compaction), then the daemon maybe ran, but there were too many files to compact due to the file descriptor limit issue, so that failed.

@kevina
Copy link
Contributor Author

kevina commented May 19, 2017

@whyrusleeping you seam to know a little more than I do, does the ipfs daemon some how trigger compaction? Is it a background thread in leveldb that does the compaction and won't run if you only use leveldb for short periods of time?

@whyrusleeping
Copy link
Member

@kevina its a background thread in leveldb that triggers the compaction normally

@kevina
Copy link
Contributor Author

kevina commented Jun 12, 2017

@whyrusleeping for the fsck option, I take it it should require the daemon be offline. I think of you compact the full range it will get a write lock on the database so nothing will be able to happen anyway while the compaction is running.

@whyrusleeping
Copy link
Member

@kevina Yeah, thats probably the right way to do it.

@whyrusleeping whyrusleeping added the help wanted Seeking public contribution on this issue label Sep 2, 2017
@whyrusleeping
Copy link
Member

@kevina still planning on working on this?

@kevina
Copy link
Contributor Author

kevina commented Sep 2, 2017

Yeah. I kind of forget about these little tasks.

@kevina
Copy link
Contributor Author

kevina commented Sep 4, 2017

I looked into it and implementing this will be non-trivial. The first problem will be to get to the underlying datastore, this is possible, but only after calling NewDatastore as go-ds-levelds exports the DB field (by accident) but not the type returned:

type datastore struct {
	DB *leveldb.DB
}

The second problem would be to discover the datastores that need compacting.

ref: https://github.com/ipfs/go-ds-leveldb/blob/4f27fe0c470cb3762d7a242bcf2f8d151e3da7dd/datastore.go

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
help wanted Seeking public contribution on this issue
Projects
None yet
Development

No branches or pull requests

4 participants