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

Remove log overlay #216

Closed
wants to merge 13 commits into from
Closed

Remove log overlay #216

wants to merge 13 commits into from

Conversation

arkpar
Copy link
Member

@arkpar arkpar commented Jul 21, 2023

This introduce a major chage in the WAL format. Current design writes WAL in the format that matches the on-disk format. I.e. index pages and value table entries. This seemed like a good idea at the time, as it allowed to split the work more efficiently between threads, and incorporate the reindex process. However it turned out that this approach has some drawbacks:

  1. Log records are bigger in size. Write amplification is high.
  2. Possibility of page evicton. There's significant time between planning and enactment phases. Planning phase would load a page from disk into OS page cache (to e.g. check if the key matches), and write an update to the log. By the time that log is eanced the OS might have already evicted the page and would need to load it again.

This new design changes the log records to contain high level DB operations, rather than the low level table updates. Operations are now executed at the enactment phase. CommitOverlay holds newly inserted key-values until they are enacted.

@arkpar
Copy link
Member Author

arkpar commented Jul 23, 2023

After some consideraton, this is not really sound w.r.t. reference handling. Operations such ash reference increment and decrement must include the current reference value in the log, as they can't be enacted twice after an unclean shutdown and log replay. So this needs more work. It might be worth it to have a separate table for keeping ref counts. Only entries with more than one RC would go there, so the table would be rather small.

This PR needs more work, so I'm closing it for now.

@arkpar arkpar closed this Jul 23, 2023
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

Successfully merging this pull request may close these issues.

1 participant