-
Notifications
You must be signed in to change notification settings - Fork 16
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
Merge LogStructured in SQLLog #194
Merge LogStructured in SQLLog #194
Conversation
Depends on #193 |
BenchmarkResults
Current status
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Awesome, thanks for splitting this into separate commits this helped tremendously to follow along the changes one-by-one.
A few questions while I was reading through popped up:
This is used to help move bits from logstructured to sqllog in an incremental fashion
I did not change the query yet, but it should be done as now the list query does not need to return the prev value
After all, the data structure was never a Log Structured.
e4c34ed
to
ba2a7c4
Compare
I rebased on top of |
Dropped dependency from #193 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM, just removing the comment before merging.
This PR merges the functionality from LogStructured into SQLLog and removes LogStructured.
There are many reasons for this change but the most important one is the lack of a real separation between the two structs, making LogStructured a mere translation layer which adds no useful functionality (and actually acts as a limitation, forcing the implementation to be sub-optimal).
The PR is quite big, but each commit takes care of a single aspect of the transition, making it hopefully possible to perform a review.
Wherever made sense the PR also takes the opportunity to make some of the sub-optimal things better both in term of correctness and in terms of memory footprint/speed (for example avoiding some useless copy-paste or some useless memory allocation).
Some part of this PR (namely, the
List
/Get
query) could be improved/made simpler, however this PR does not attempt to do that, so that it should be compatible with other outstanding PRs.As a last note, the write path and the indexes do not change, so this PR is compatible with all existing versions.