-
Notifications
You must be signed in to change notification settings - Fork 197
Add back config to toggle the preservation of timestamps in consolidated fragments #5515
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
base: main
Are you sure you want to change the base?
Changes from 3 commits
f54dd19
e6fe6ac
754cc3c
69a5c3f
2b2a434
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -303,13 +303,17 @@ class ArrayDirectory { | |
* [`timestamp_start`, `timestamp_end`] will be considered when | ||
* fetching URIs. | ||
* @param mode The mode to load the array directory in. | ||
* @param allow_partial_fragment_overlap If we want to allow matching | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. It looks like the the only usage of this is "if false, |
||
* fragments that overlap only partially with the timestamp range. This | ||
* applies in some cases of consolidated fragments with timestamps. | ||
*/ | ||
ArrayDirectory( | ||
ContextResources& resources, | ||
const URI& uri, | ||
uint64_t timestamp_start, | ||
uint64_t timestamp_end, | ||
ArrayDirectoryMode mode = ArrayDirectoryMode::READ); | ||
ArrayDirectoryMode mode = ArrayDirectoryMode::READ, | ||
bool allow_partial_fragment_overlap = true); | ||
|
||
/** Destructor. */ | ||
~ArrayDirectory() = default; | ||
|
@@ -644,6 +648,10 @@ class ArrayDirectory { | |
/** Mode for the array directory. */ | ||
ArrayDirectoryMode mode_; | ||
|
||
/** True if we allow matching fragments with partial timestamp range overlap | ||
*/ | ||
bool allow_partial_fragment_overlap_; | ||
|
||
/** True if `load` has been run. */ | ||
bool loaded_; | ||
|
||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -409,6 +409,11 @@ class Config { | |
*/ | ||
static const std::string SM_GROUP_TIMESTAMP_END; | ||
|
||
/** | ||
* Enable or disable consolidation with timestamps. | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think this comment gets pulled into docs, right? This is not specific enough, especially if it is user-facing documentation. There's no description here of what the consolidation result actually looks like for the different options, which I think is really important given that one of the options results in something which might qualify as "data loss" for an unwitting customer. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Okay, I'm leaving my comment for posterity, but I see that there is more specific documentation in However, those docs aren't specific enough for my liking, the options should be annotated with a brief description of what happens to duplicate coordinates in consolidated fragments. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. |
||
*/ | ||
static const std::string SM_CONSOLIDATION_WITH_TIMESTAMPS; | ||
|
||
/** | ||
* If `true` MBRs will be loaded at the same time as the rest of fragment | ||
* info, otherwise they will be loaded lazily when some info related to MBRs | ||
|
Uh oh!
There was an error while loading. Please reload this page.