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
pkg/sorted/leveldb: writing blobpacked index on samba mount fails #1185
Comments
@dcramer thanks.
and run perkeepd with -recovery=2 and -reindex=true ? |
In any case, I think I've seen someone on IRC try and fail with samba mounts as well, so I suspect we're missing something to support writing there. |
Same-ish error:
|
@dcramer could you try without samba? with your blobs and index configured to be on a local disk? |
@mpl it definitely worked before I moved the paths to the samba mount Aside here's the way I have it mounted:
|
oh, ok then. |
@dcramer btw, one thing you could try in the meantime, is to store the blobs on the samba share, but let the index be stored on localdisk. |
The person on irc ran into a different error; specifically they got a 'fsync: invalid argument' iirc (which looked like this leveldb issue: google/leveldb#281). Either way, it looks like upstream leveldb doesn't support putting it on cifs, so if that's what's causing the issue I don't think it's a perkeep bug. |
@dcramer, can you run perkeep under strace, like:
And then post the possibly-redacted (might have passwords/keys) relevant section of I'm wondering what system calls the goleveldb code is trying to do and what the Linux CIFS/SMB implementation is actually doing. |
@euank, we don't use google/leveldb, so I doubt that bug is relevant. We use github.com/syndtr/goleveldb |
Ah, thanks for clarifying, I should have looked before assuming. |
fyi changing leveldb to be local disk and leaving blobs on samba produces a similar issue @bradfitz lmk if you need more and i can probably just dump the whole file
|
Looks like fsync(fd) = EINVAL is the culprit.
Note, this is calling fsync on a directory fd, not file. This corresponds to this code (calling this) (based on the previous rename in the strace). The Very arguably, this could be considered a break of the go 1 promise, but it's certainly not something I'd personally want to try arguing. The type of that error changed in the go1.8 timeframe with this commit. |
@euank, nice debugging. If you update that "func isErrInvalid" in the Perkeep vendored copy of goleveldb, does it all work for you? I can escalate with the Go team if so and figure out what to do. And/or we can fix goleveldb upstream. |
This is to fix perkeep#1185
I PR'd a change against goleveldb (ref). If I vendor in that change, I'm able to run with the blob + leveldb storage on a fresh directory on my test cifs mount (whereas before it reliably errored out immediately). I'd prefer waiting a bit to see if it gets accepted upstream before pointing Gopkg.toml at a fork though. There is still an additional bug I think as well. If I re-use a dirty directory, the broken code causes corruption which can't be recovered automatically. The
I suspect that relying on the underlying types and messages of errors should be out of the go1 promise unless the error is specifically documented or there's a helper like @dcramer if you want to verify that change is sufficient for your setup, you could try pulling in the commit on my branch here to see that it works. |
@euank confirmed that fixed it for me |
I submitted a cr to bump that dependency since the upstream change was merged; this should be fixed by https://perkeep-review.googlesource.com/c/perkeep/+/17126 |
@euank excellent, thanks |
@dcramer but wait, you were saying earlier that moving the index to localdisk and leaving blobs on samba was not enough to fix the issue. By the same logic, @euank 's fix is not enough either, as it will only fix the problem for the index itself, not for the blobs. So is Perkeep as a whole working now for you (while using samba) or not? |
@mpl other than a bunch of seemingly random bugs (i think unrelated) it was working with both the index and the blobs on samba. Previously when I tried placing only blobs on the samba I got the same/similar error. I'm happy to reproduce and dump similar logs, but given the branch fixed it I'm not sure its worth the effort. |
@dcramer oh wait, this is silly. I forgot we were talking about the index for the blobpacked, not the index for Perkeep as a whole. so yeah, by default the index for blobpacked goes wherever you tell the blobs to go, which means it was still being on samba in all cases. Nevermind. |
Not going to be super helpful here given I don't have a lot of details on the project yet (first time setting it up), but let me know if you need more details.
Details:
Config:
Log:
The text was updated successfully, but these errors were encountered: