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

ch7: actionkv v2.0 does not resolve the problem in actionkv v1.0 #84

Open
yamoridon opened this issue Aug 17, 2022 · 6 comments
Open

ch7: actionkv v2.0 does not resolve the problem in actionkv v1.0 #84

yamoridon opened this issue Aug 17, 2022 · 6 comments

Comments

@yamoridon
Copy link

Section 7.7.11 points out a problem of actionkv v1 is that it has long start up time and describes a way to shorten it by storing the index itself on the database. But actionkv v2 still reads all of the database file and rebuilds the index. Thus the problem of actionkv v1 is not resolved. Considring the discussion of this section, I think it should be resolved.

@brunojppb
Copy link

brunojppb commented Jan 28, 2023

Hey folks, I've just been through Chapter 7 and I think @yamoridon is correct. If we look at the source here, the lib.rs implementation still rebuilds the index when we call load. Here is how the source looks like:

Load call site

a.load().expect("unable to load data");

Inserting in the index

self.index.insert(kv.key, current_position);

I was wondering how @timClicks would approach this part.
Appreciate any feedback on that.

@timClicks
Copy link
Contributor

I wondered when this would be pointed out. At one point I did implement a fix for this, but I believe that I pulled it out to simplify the v2 code example.

@anBertoli
Copy link

anBertoli commented Feb 16, 2023

@timClicks do you accept PRs to fix this type of issues? I would be happy to help.

@ladislaff93
Copy link

Why does the store_index_on_disk removing the self.index ? a.index = std::collections::HashMap::new(); I dont get it

@xitep
Copy link

xitep commented Dec 15, 2023

  • i think a less confusing approach to demonstrating the utility of de/serialization would be to store the index (as a kind of cache) in a separate file (this would utilize the the material from section 7.2.) appending the index (for each modification) to the database file itself is wasteful.
  • to demonstrate some more work around HashMaps in this section, maybe extending the ActionKV#load method to prune the index of empty value entries (as created the by the "delete" command) could serve as a case study.

@ladislaff93
Copy link

  • i think a less confusing approach to demonstrating the utility of de/serialization would be to store the index (as a kind of cache) in a separate file (this would utilize the the material from section 7.2.) appending the index (for each modification) to the database file itself is wasteful.
  • to demonstrate some more work around HashMaps in this section, maybe extending the ActionKV#load method to prune the index of empty value entries (as created the by the "delete" command) could serve as a case study.

That is exactly the way I did it. I made index and data files and index is rebuilded from index file

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

6 participants