chunk, storage: chunk.Store multiple chunk Put #1681
Conversation
There was a problem hiding this comment.
LGTM, it should also be extended to Set series :) e.g., https://github.com/ethersphere/swarm/pull/1392/files#diff-ead00e2d8820171b56d4e1d1504b8bceR132
| log.Trace("netstore.put slow chunk delivery", "ref", ch.Address().String()) | ||
| } | ||
| for _, ch := range chs { | ||
| // notify RemoteGet (or SwarmSyncerClient) about a chunk delivery and it being stored |
There was a problem hiding this comment.
shall we just use the exist bool array?
for i, ch := range chs {
if exist[i] {
continue
}
fi, ok := n.fetchers.Get(ch.Address().String())
if !ok { panic("fetcher should exist") }
}There was a problem hiding this comment.
This is a nice proposal, but I am not sure if we should change the netstore so much in this PR. I have just added multi chunks while preserving the same individual chunk logic as before. Can we address this later or maybe in new stream package?
storage/localstore/mode_put.go
Outdated
| return exists, gcSizeChange, nil | ||
| } | ||
|
|
||
| // putRequest adds an Item to the batch by updating required indexes: |
storage/localstore/mode_put.go
Outdated
| return false, nil | ||
| } | ||
|
|
||
| // putRequest adds an Item to the batch by updating required indexes: |
|
Thank, @zelig. Yes, the next would be to do the same thing for Set method. Also it can be beneficial for Get and Has as every get or has on leveldb creates a new snapshot and getting multiple chunks from the same snapshot may be better. I think that this PR is not ready to be merged until duplicate chunks scenario in Put arguments is handled properly. |
* 'master' of github.com:ethersphere/swarm: chunk, storage: chunk.Store multiple chunk Put (ethersphere#1681) storage: fix pyramid chunker and hasherstore possible deadlocks (ethersphere#1679) pss: Use distance to determine single guaranteed recipient (ethersphere#1672) storage: fix possible hasherstore deadlock on waitC channel (ethersphere#1674) network: Add adaptive capabilities message (ethersphere#1619) p2p/protocols, p2p/testing; conditional propagation of context (ethersphere#1648) api/http: remove unnecessary conversion (ethersphere#1673) storage: fix LazyChunkReader.join potential deadlock (ethersphere#1670) HTTP API support for pinning contents (ethersphere#1658) pot: Add Distance methods with tests (ethersphere#1621) README.md: Update Vendored Dependencies section (ethersphere#1667) network, p2p, vendor: move vendored p2p/testing under swarm (ethersphere#1647) build, vendor: use go modules for vendoring (ethersphere#1532)
This PR adds support for multiple chunks to be Put by localstore. This is a performance improvement by using one leveldb batch for multiple chunks, instead to have a separate batch for every chunk.
All localstore Put tests are changed to validate functionality with multiple chunks.
This PR addresses only the Put method for easier review. If this approach is approved, other methods will be changed in a similar way. This change is targeted for master, but new stream package should use it. This way the diff on new stream PR should not get even larger.