-
Notifications
You must be signed in to change notification settings - Fork 208
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
refactor(storage): separate writing from builder. #499
refactor(storage): separate writing from builder. #499
Conversation
…o fix compactor panic. Signed-off-by: arkbriar <arkbriar@gmail.com>
No need. I'll take a look at this PR next week :) |
Others LGTM! Let @skyzh have a more detailed review then. |
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, good work!
src/storage/secondary/rowset/raw.rs
Outdated
|
||
use crate::catalog::ColumnCatalog; | ||
|
||
/// Raw columns. |
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.
What's the meaning of Raw
? Better to name it as EncodedColumn
or something?
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.
Intended to be "in raw bytes". "Encoded" is better as with different encoders we get different bytes.
I'll fix it.
…as an argument when flush. Signed-off-by: arkbriar <arkbriar@gmail.com>
…EncodedColumn' Signed-off-by: arkbriar <arkbriar@gmail.com>
Head branch was pushed to by a user without write access
4a5b4c2
to
089c8bc
Compare
Signed-off-by: arkbriar arkbriar@gmail.com
Separates the write logic from the rowset builder and also fixes the #493 .
close #110
close #493
I fix it by skipping the entire add rowset procedure. Not sure if it's legal in all circumstances, e.g. compact all rowsets in a table (empty and delete them all. Is it necessary to make sure as least one active rowset in the directory? @skyzh