-
Notifications
You must be signed in to change notification settings - Fork 455
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
[dbnode] Use StreamingReadMetadata for bootstrapping #2938
Conversation
Codecov Report
@@ Coverage Diff @@
## master #2938 +/- ##
=======================================
Coverage 72.2% 72.2%
=======================================
Files 1084 1084
Lines 100492 100492
=======================================
Hits 72630 72630
Misses 22806 22806
Partials 5056 5056
Flags with carried forward coverage won't be shown. Click here to find out more. Continue to review full report at Codecov.
|
r.streamingID = append(r.streamingID[:0], entry.ID...) | ||
r.streamingTags = append(r.streamingTags[:0], entry.EncodedTags...) |
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.
Will these compete with StreamingRead
also mutating these fields?
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.
No. At least for now (and most probably in the future) data and metadata reads are mutually exclusive (like it was before the introduction of streaming). This is documented in the method comments on the interface.
id.Finalize() | ||
tagsIter.Close() |
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.
Should the id and encoded tags be finalized here still, considering FromRawSeriesIDAndTags
is cloning them?
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.
With streaming reads, id and tags are just raw []byte
slices and thus no longer a subject to finalization.
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
# Conflicts: # src/dbnode/storage/bootstrap/bootstrapper/fs/source.go # src/dbnode/storage/bootstrap/bootstrapper/peers/source.go # src/dbnode/storage/index/convert/convert.go # src/dbnode/storage/index/convert/convert_test.go
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
* master: [dbnode] Add aggregate term limit regression test (#3135) [DOCS] Adding Prometheus steps to quickstart (#3043) [dbnode] Revert AggregateQuery changes (#3133) Fix TestSessionFetchIDs flaky test (#3132) [dbnode] Alter multi-segments builder to order by size before processing (#3128) [dbnode] Emit aggregate usage metrics (#3123) [dbnode] Add Shard.OpenStreamingReader method (#3119) [dtests] Docker tests integration with docker-compose (#3031) [dbnode] Comments / remove unused var (#3124) [query] Handle context.Canceled and map to 499 http status (#3069) [dbnode] Use StreamingReadMetadata for bootstrapping (#2938) [dbnode] Use DefaultTestOptions in test code (#3113) # Conflicts: # src/dbnode/storage/bootstrap/bootstrapper/fs/source.go
What this PR does / why we need it:
DataReaderOpenOptions
had an optionOptimizedReadMetadataOnly
that, when enabled, was avoiding the sorting of data fileset index file after loading it into memory. This change makes a further improvement and avoids loading this file into memory altogether, by using the newly implementedStreamingReadMetadata
method (which is a sister method of already existingStreamingRead
).The change affects
fs/source
andpeers/source
bootstrappers.Special notes for your reviewer:
Does this PR introduce a user-facing and/or backwards incompatible change?:
NONE
Does this PR require updating code package or user-facing documentation?:
NONE