-
Notifications
You must be signed in to change notification settings - Fork 444
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
storage: add "proxycache" storage type #443
Comments
I meant: Related: issue #442 so diskpacked can be an efficient cache. |
I submitted the start of this: https://camlistore.googlesource.com/camlistore/+/56ed064b6af8b3bda6cf550d272a1a5dba1b5f04%5E%21/ |
Just as an fyi, I've started working on this here. I've got a few things to fix and clean up before I'd call it review-ready. This basically fills in a few TODO items I found in proxycache and adds some tests. The approach steps away from sorted.KeyValue and uses the lru package and the stats blobserver. There's a bit of redundant data being stored, so I'm not in love with it... but I do think the implementation is simpler using the imports. Still hacking a bit, but feel free to make suggestions if you find the time. |
A nice (later) validation of this might be to remove the caching code from |
This change is getting close, so I uploaded to the review server in case anyone wants a head start. |
Something in particular I could use a little input on: when we verify the cache, what should we do with the different errors?
|
So I went with returning an error in all the cases. (Only when you ask to verify specific refs does the cache missing a ref actually become an error) I decided to punt on the BlobHub issue for now, but I think that's okay. Deleting blobs seems like it'd be a rare use case and changing them would involve finding a hash collision ... so even more rare. I also think I found a bug in the go4.org/jsonconfig package while writing a test for the configuration piece of this. That last test is currently failing, but passes with this change applied: https://review.gerrithub.io/#/c/346661/. Apart from that last item, this change is "done" and I would be delighted with some code review :) |
The text was updated successfully, but these errors were encountered: