Skip to content
This repository was archived by the owner on Jan 28, 2026. It is now read-only.

adds retrieval from cold storage back#34

Merged
brunocalza merged 2 commits intomainfrom
bcalza/coldstorage
Feb 9, 2024
Merged

adds retrieval from cold storage back#34
brunocalza merged 2 commits intomainfrom
bcalza/coldstorage

Conversation

@brunocalza
Copy link
Contributor

@brunocalza brunocalza commented Feb 6, 2024

This PR adds retrieval from cold storage back. Fetching from cold storage is the default behavior of the retrieve command. If you want to fetch from the cache you have to provide true to the --cache flag, e.g. --cache true.

One caveat: because of how we're building the car files now (not wrapping inside a Unix dir), the project https://github.com/ipld/go-car/ does not work for unpacking the car file, you have to use https://github.com/ipfs/go-cid

Signed-off-by: Bruno Calza <brunoangelicalza@gmail.com>
@brunocalza brunocalza self-assigned this Feb 6, 2024
@brunocalza brunocalza marked this pull request as ready for review February 6, 2024 21:22
@sanderpick
Copy link
Contributor

Maybe a silly question, but would it make sense to by default pull from cache if available, then fallback to cold?

@brunocalza
Copy link
Contributor Author

Maybe a silly question, but would it make sense to by default pull from the cache if available, then fall back to cold?

Yeah, makes sense. This flag implementation comes from the assumption that most of the files wouldn't be stored in the cache (because storing in cache would be more expensive). And if that's true, making a call that most of the time fails would not make sense. But that assumption is probably old. I guess from current architecture thinking all files would go to a DA layer, right? So, checking the cache first would make sense

@dtbuchholz
Copy link
Contributor

Hm yeah agreed.

A related, future feature idea—something else that would be nice is if there was another config file that lets you set default settings or contexts, like the vault name, path to (or value of) the private key, the default retrieval method (cache vs cold), etc. Then, commands that use --private-key, --account (from pk value), --vault, etc. can take these values by default. Idk the best approach for what's in this config file, but my use case is: I'm either creating new vaults and want to use the same settings every time w/o passing as flags, or I'm interacting with the same vault and want to reduce the flags I have to pass.

@sanderpick
Copy link
Contributor

Maybe a silly question, but would it make sense to by default pull from the cache if available, then fall back to cold?

Yeah, makes sense. This flag implementation comes from the assumption that most of the files wouldn't be stored in the cache (because storing in cache would be more expensive). And if that's true, making a call that most of the time fails would not make sense. But that assumption is probably old. I guess from current architecture thinking all files would go to a DA layer, right? So, checking the cache first would make sense

You're assumption is still correct... we're still on track w/ the expensive cache idea (allowing people to optimize costs by trading off cold storage for cache). But even so, may as well get it from the cache assuming it's faster as long as they are paying for it?

@sanderpick
Copy link
Contributor

Than again, I don't know how easy it is to check the cache and fallback

@brunocalza
Copy link
Contributor Author

oh okay 👍 Well, which one is faster is tricky, because the cold storage retrieval is not really hitting Filecoin, it's hitting IPFS where the content is pinned by web3 storage. So, at the end of the day is it just as fast as hitting the cache? 😅 but I get your point, theoretically speaking the cold storage should be way slower

it's an easy change. i think it makes sense to do it

return fmt.Errorf("failed to create request: %s", err)
}

if _, err := lassie.Fetch(ctx, request, []types.FetchOption{}...); err != nil {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

How does lassie know to fetch is from Filecoin (cold storage) and not from IPFS?

Copy link
Contributor Author

@brunocalza brunocalza Feb 7, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

in this setup we don't, it kind of checks multiple protocols until it finds it. but we can specify a specific protocol for retrieval if that's what we want

Usage: "Retrieves from cache by setting this flag",
DefaultText: "current directory",
Destination: &cache,
Value: true,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

unrelated to this PR, but the current "DefaultText" here is not too relevant here

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

good catch! but looks like we'll be removing the flag and just rely on cache by default, and if not available cold store

Signed-off-by: Bruno Calza <brunoangelicalza@gmail.com>
@brunocalza brunocalza merged commit 3429c2b into main Feb 9, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants