Skip to content
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.

Sign up for GitHub

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

Jump to bottom

IngestSegmentFirehose may not handle overlapping segments properly #1678

Closed
gianm opened this issue Aug 27, 2015 · 4 comments
Closed

IngestSegmentFirehose may not handle overlapping segments properly #1678

gianm opened this issue Aug 27, 2015 · 4 comments

Comments

@gianm
Copy link
Contributor

gianm commented Aug 27, 2015

See https://groups.google.com/d/msg/druid-development/ZCXRGzGfXSM/qVVl3W1jDgAJ

It looks like the factory is using a VersionedIntervalTimeline to figure out what segments to read, but doesn't use the interval of the timeline holders to filter the segments.
@gianm
Copy link
Contributor Author

gianm commented Aug 27, 2015

Failing test at: https://github.com/druid-io/druid/compare/master...gianm:ingest-segment-overlapping?expand=1#diff-8b56e359211a285701b559f7bd1296f4R244

@himanshug
Copy link
Contributor

looks like this one is going to impact batch delta ingestion as well, because same code is reused there to read data. We probably should put this one as a known bug inside 0.8.1 release notes till fixed.
Also IngestSegmentFirehoseFactory usage of VersionedIntervalTimeline looks redundant because IndexerSQLMetadataStorageCoordinator (see https://github.com/druid-io/druid/blob/master/server/src/main/java/io/druid/metadata/IndexerSQLMetadataStorageCoordinator.java#L75 ) does that as well.

@gianm gianm mentioned this issue Aug 28, 2015
Merged
@gianm
Copy link
Contributor Author

gianm commented Aug 28, 2015

This doesn't compile anymore after merging #1609, rebasing back onto master.

gianm added a commit to gianm/druid that referenced this issue Aug 28, 2015
@gianm
Fix overlapping segments in IngestSegmentFirehose, DatasourceInputFor…
414a6fb 
…mat.

Fixes apache#1678. IngestSegmentFirehose (and its users) need to remember which
windows of which segments should actually be read, based on a timeline.
@gianm
Copy link
Contributor Author

gianm commented Aug 28, 2015

ok, now it compiles, and that is cool.

drcrallen pushed a commit to metamx/druid that referenced this issue Aug 31, 2015
@gianm @drcrallen
Fix overlapping segments in IngestSegmentFirehose, DatasourceInputFor…
9110df0 
…mat.

Fixes apache#1678. IngestSegmentFirehose (and its users) need to remember which
windows of which segments should actually be read, based on a timeline.
seoeun25 pushed a commit to seoeun25/incubator-druid that referenced this issue Jan 10, 2020
@navis
apache#1678 Fix regression from apache#1655
d689170
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
2 participants
@himanshug @gianm