Skip to content
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

fix(iroh-store): put_many bug #507

Merged
merged 2 commits into from
Nov 17, 2022
Merged

fix(iroh-store): put_many bug #507

merged 2 commits into from
Nov 17, 2022

Conversation

ramfox
Copy link
Contributor

@ramfox ramfox commented Nov 17, 2022

Fixes the following scenario: if you attempt to add a folder with some files, it adds fine. But if you add a new file to that folder and add it again, the store won't add the new file.

closes n0-computer/beetle#114

@ramfox ramfox added the fix Fixes a bug label Nov 17, 2022
@ramfox ramfox added this to the v0.1.1 milestone Nov 17, 2022
@ramfox ramfox self-assigned this Nov 17, 2022
@ramfox ramfox force-pushed the ramfox/put_many_bug_fix branch 2 times, most recently from e562342 to 05ead87 Compare November 17, 2022 19:04
- `continue` instead of `return`, when we find that a CID already exists
  in the store
- ensure we aren't double-adding CIDs from the same batch, by tracking
  the batched CIDs in a HashSet
rklaehn
rklaehn previously approved these changes Nov 17, 2022
Copy link
Contributor

@rklaehn rklaehn left a comment

Choose a reason for hiding this comment

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

See the comments for some minor improvements

iroh-store/src/store.rs Show resolved Hide resolved
iroh-store/src/store.rs Outdated Show resolved Hide resolved
iroh-store/src/store.rs Outdated Show resolved Hide resolved
iroh-store/src/store.rs Show resolved Hide resolved
@@ -367,9 +367,9 @@ impl<'a> WriteStore<'a> {
let mut total_blob_size = 0;

let mut batch = WriteBatch::default();
let mut cid_tracker: HashSet<Cid> = std::collections::HashSet::default();
let mut cid_tracker: AHashSet<Cid> = AHashSet::default();
Copy link
Contributor

Choose a reason for hiding this comment

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

If we had a vec we could use https://docs.rs/ahash/latest/ahash/struct.AHashSet.html#method.with_capacity here to make sure the hashset does not need to rehash.

But with the iterator we could just use the size hint, which is probably not worth it.

Copy link
Contributor

Choose a reason for hiding this comment

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

@ramfox ramfox merged commit 88f1a49 into main Nov 17, 2022
@ramfox ramfox deleted the ramfox/put_many_bug_fix branch November 17, 2022 20:38
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
fix Fixes a bug
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Intermittent add/get roundtrip failure with real store
3 participants