-
-
Notifications
You must be signed in to change notification settings - Fork 712
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
Allow for a same-thread doc compressor. #1510
Conversation
e3e59a7
to
5602efe
Compare
Codecov Report
@@ Coverage Diff @@
## main #1510 +/- ##
==========================================
+ Coverage 93.96% 93.98% +0.02%
==========================================
Files 242 243 +1
Lines 44936 45089 +153
==========================================
+ Hits 42226 42379 +153
Misses 2710 2710
Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here. |
456e0db
to
dbd1ddd
Compare
f02ff44
to
22b5d11
Compare
@@ -235,6 +235,14 @@ impl InnerSegmentMeta { | |||
} | |||
} | |||
|
|||
fn return_true() -> bool { |
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.
fn return_true() -> bool { | |
fn docstore_compress_dedicated_thread() -> bool { |
/// If set to true, docstore compression will happen on a dedicated thread. | ||
/// (defaults: true) | ||
#[doc(hidden)] | ||
#[serde(default = "return_true")] |
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.
#[serde(default = "return_true")] | |
#[serde(default = "docstore_compress_dedicated_thread")] |
src/store/store_compressor.rs
Outdated
enum Impls { | ||
SameThread(BlockCompressorImpl), | ||
DedicatedThread(DedicatedThreadBlockCompressorImpl), |
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.
I think the naming could be improved here a little
enum Impls { | |
SameThread(BlockCompressorImpl), | |
DedicatedThread(DedicatedThreadBlockCompressorImpl), | |
enum BlockCompressorVariants { | |
SameThread(SameThreadBlockCompressor), | |
DedicatedThread(DedicatedThreadBlockCompressor), |
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 except some naming comments
In addition, it isolates the doc compressor logic, better reports io::Result. In the case of the same-thread doc compressor, the blocks are also not copied.
22b5d11
to
d6bd9a9
Compare
In addition, it isolates the doc compressor logic, better reports io::Result.
In the case of the same-thread doc compressor,
the blocks are also not copied.