chunk, shed, storage: chunk.Store GetMulti method #1691
Conversation
| } | ||
| return nil, err | ||
| } | ||
| chunks = make([]chunk.Chunk, len(out)) |
There was a problem hiding this comment.
there might be cases where len(out) < len(addrs), this in the calling context must be checked and somehow handled, and so I am wondering if it is not better to have chunks := make([]chunk.Chunk, len(addrs) and within this slice pad the chunks of set addr which were not found with nil values, that is to allow the calling context to verify which chunks were not found with time complexity of O(1) rather than O(n) in this case.
I think this might be a premature optimization and maybe we could consider to have this as a future iteration and so I won't block on this, but would be happy to have @zelig's opinion on this (and everyone else that feels like commenting on this)
There was a problem hiding this comment.
Thanks @acud. Actually, len(out) is equal to len(addrs) as out is constructed on line 63 in getMulti with len(addrs) as length.
I like your idea of passing nils for not found chunks. I am also interested in other opinions, as this would require changing shed.Index.Fill to handle []*Item instead []Item, but Item is used with value semantics.
There was a problem hiding this comment.
OK I missed the fact they are of equal length :)
See my remark on Fill in this context
| if err != nil { | ||
| return err | ||
| } | ||
| value, err := snapshot.Get(key, nil) |
There was a problem hiding this comment.
So this assumes that all requested chunks are in the DB. Does this account for the case where GC was run in between and value is not found in the db? only by returning an error right? I guess this is also a feasible approach
There was a problem hiding this comment.
Yes, if the chunk is missing the error will be returned.
As the snapshot is acquired, gc will not influence this result if it runs while this function is called. The chunk will be returned in that case.
There was a problem hiding this comment.
Cool 👍 let's keep it so then. I think for now it would also make error handling more clear and easy on the calling context
| } | ||
| return nil, err | ||
| } | ||
| chunks = make([]chunk.Chunk, len(out)) |
There was a problem hiding this comment.
OK I missed the fact they are of equal length :)
See my remark on Fill in this context
* 'master' of github.com:ethersphere/swarm: pss: Modularize crypto and remove Whisper. Step 1 - isolate whisper code (ethersphere#1698) pss: Improve pressure backstop queue handling - no mutex (ethersphere#1695) cmd/swarm-snapshot: if 2 nodes to create snapshot use connectChain (ethersphere#1709) network: Add API for Capabilities (ethersphere#1675) pss: fixed flaky test that was using a global variable instead of a local one (ethersphere#1702) pss: Port tests to `network/simulation` (ethersphere#1682) storage: fix hasherstore seen check to happen when error is nil (ethersphere#1700) vendor: upgrade go-ethereum to 1.9.2 (ethersphere#1689) bzzeth: initial support for bzz-eth protocol (ethersphere#1571) network/stream: terminate runUpdateSyncing on peer quit (ethersphere#1696) all: first working SWAP version (ethersphere#1554) version: update to v0.5.0 unstable (ethersphere#1694) chunk, storage: storage with multi chunk Set method (ethersphere#1684) chunk, storage: add HasMulti to chunk.Store (ethersphere#1686) chunk, shed, storage: chunk.Store GetMulti method (ethersphere#1691) api, chunk: progress bar support (ethersphere#1649)
This PR adds GetMulti method to chunk.Store and localstore. It is the last method to be added for multi chunk operations beside Put, Set and HasMulti, which are already submitted.