Skip to content
This repository was archived by the owner on Aug 2, 2021. It is now read-only.

chunk, storage: storage with multi chunk Set method#1684

Merged
janos merged 2 commits intomasterfrom
localstore-multi-set
Aug 26, 2019
Merged

chunk, storage: storage with multi chunk Set method#1684
janos merged 2 commits intomasterfrom
localstore-multi-set

Conversation

@janos
Copy link
Copy Markdown
Member

@janos janos commented Aug 22, 2019

This PR changes chunk.Store Set method to accept multiple chunks. This PR is a followup of #1681. This is a performance improvement by using one leveldb batch for multiple chunks, instead to have a separate batch for every chunk.

All localstore Set tests are changed to validate functionality with multiple chunks.

@janos janos requested review from acud, holisticode, jmozah and zelig August 22, 2019 09:46
@janos janos self-assigned this Aug 22, 2019
Copy link
Copy Markdown
Contributor

@acud acud left a comment

Choose a reason for hiding this comment

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

LGTM except a few more comments about the cases would be nice

item.StoreTimestamp = i.StoreTimestamp
item.BinID = i.BinID
case leveldb.ErrNotFound:
db.pushIndex.DeleteInBatch(batch, item)
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

can you explain a bit better the case here? if it isn't in the retrieval data index does this not infer that the index is corrupted?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Yes, this case can happen only if the chunk is removed by gc in between it is got from localstore and its access time is set. In that case, the error would not be relevant. Maybe we could do something else in this case? Maybe just return nil?

@janos janos merged commit d305d9b into master Aug 26, 2019
@janos janos deleted the localstore-multi-set branch August 26, 2019 14:17
@zelig zelig mentioned this pull request Aug 27, 2019
26 tasks
@skylenet skylenet added this to the 0.5.0 milestone Sep 17, 2019
chadsr added a commit to chadsr/swarm that referenced this pull request Sep 23, 2019
* '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)
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants