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.

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

store: Setting the start and end to prior posting changes #837

Merged
merged 7 commits into from
Feb 13, 2019

Conversation

domgreen
Copy link
Contributor

No description provided.

@domgreen
Copy link
Contributor Author

Not sure if its better to revert to:
return r.loadChunks(ctx, offsets[m:n], seq, offsets[m],offsets[n-1]+maxChunkSize)

or go to:
return r.loadChunks(ctx, offsets[m:n], seq, uint32(p.start), uint32(p.end))

Really need some way of testing this. :/

@domgreen domgreen requested a review from devnev February 12, 2019 20:58
@domgreen
Copy link
Contributor Author

I'm still unsure if we should be adding maxChunkSize/maxSeriesSize onto the end parameter when calling loadChunks/loadSeries functions the rng(k) func already adds this ... could be a minor detail.

@bwplotka
Copy link
Member

This is it! Will add unit test for this into your branch.

maxChunkSize/maxSeriesSize onto the end parameter when calling loadChunks/loadSeries

I agree - it might be a bug as well, will remove it. We can always add it. Test should catch this hopefully.

Moved bucket e2e tests to table test.

Signed-off-by: Bartek Plotka <bwplotka@gmail.com>
@bwplotka bwplotka changed the title setting the start and end to prior posting changes store: Setting the start and end to prior posting changes Feb 13, 2019
@bwplotka
Copy link
Member

Added test with naive "partitioner" to simulate multiple parts. This on master triggers:

runtime error: slice bounds out of range - on this branch it works. Awesome 👍

@bwplotka
Copy link
Member

And yea, that extra maxSeries/ChunkSize is definitely bug on new version - since we moved to what parts gives us, we should avoid this. Added commit fixing this as well.

Signed-off-by: Bartek Plotka <bwplotka@gmail.com>
@bwplotka
Copy link
Member

Thanks for spotting this 👍 Classic golang bug 🤦‍♂️ function closure + loop... 💥

@bwplotka
Copy link
Member

Fixes #829

Signed-off-by: Bartek Plotka <bwplotka@gmail.com>
@domgreen
Copy link
Contributor Author

Thanks for spotting this Classic golang bug function closure + loop...

@devnev spotted it within seconds 😂

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.

3 participants