Skip to content

Move iterators inside chunk store using MergeSeriesIterator#438

Closed
aaron7 wants to merge 13 commits intomasterfrom
move-iterators-inside-chunkstore
Closed

Move iterators inside chunk store using MergeSeriesIterator#438
aaron7 wants to merge 13 commits intomasterfrom
move-iterators-inside-chunkstore

Conversation

@aaron7
Copy link
Copy Markdown
Contributor

@aaron7 aaron7 commented May 23, 2017

Depends on #430 - view changes for this PR here: https://github.com/weaveworks/cortex/pull/438/files/83bb69d02a11ef471aebf9408c6a1f1190a6a9da..c38224c193bafc21fe4e38be72c33117893f2dc6

Part of #416

  • Move iterators inside the chunk store
  • Create MergeSeriesIterator which is created from multiple iterators and implements SeriesIterator.
  • Move iterator code to util as it is used across packages and cyclic imports would exist in some cases if it was kept in querier.

@aaron7 aaron7 force-pushed the move-iterators-inside-chunkstore branch from 5497751 to c171b2e Compare May 23, 2017 14:56
@aaron7 aaron7 changed the title [WIP] Move iterators inside chunk store [WIP] Move iterators inside chunk store using MergeSeriesIterator May 23, 2017
@aaron7 aaron7 changed the title [WIP] Move iterators inside chunk store using MergeSeriesIterator Move iterators inside chunk store using MergeSeriesIterator May 24, 2017
@aaron7 aaron7 requested a review from tomwilkie May 24, 2017 13:03
@aaron7 aaron7 force-pushed the move-iterators-inside-chunkstore branch from 93df818 to 454f36c Compare May 24, 2017 13:13
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This conversion is a little unfortunate. I think it needs a bit more thought.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why is this needed?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just realised that the samplePair timestamp will always be less than the timestamp given. I will remove.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

MergeNSamplesSets? And MergeSampleSets above?

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Parallelism not necessary here I think.

@aaron7 aaron7 force-pushed the move-iterators-inside-chunkstore branch from 4100dcc to 596ddda Compare May 30, 2017 13:28
@aaron7
Copy link
Copy Markdown
Contributor Author

aaron7 commented Jun 9, 2017

#442

@aaron7 aaron7 closed this Jun 9, 2017
@tomwilkie tomwilkie deleted the move-iterators-inside-chunkstore branch September 24, 2018 14:59
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants