store-gateway: bucketStore tests with and without streaming #3658
Conversation
This PR also removes the streaming implementation from * `TestSeries_BlockWithMultipleChunks`, which I think is ok because we already have unit tests for multiple chunks in our units * `TestLabelNamesAndValuesHints` label values aren't using streaming still anyway * `TestSeries_ErrorUnmarshallingRequestHints` this should be independent of how series are fetched (streaming or not) pkg/storegateway now takes 109.121s to run on my machine compared to the 94.677s it took before Signed-off-by: Dimitar Dimitrov <dimitar.dimitrov@grafana.com>
pkg/storegateway/bucket_e2e_test.go
Outdated
| assert.NoError(t, err) | ||
| factory := func(opts ...prepareStoreConfigOption) *storeSuite { | ||
| opts = append(opts, withBucketStoreOptions(WithStreamingSeriesPerBatch(1000))) |
There was a problem hiding this comment.
Each Series() call in the tests should use multiple batches. Is it the case if we use such a large batch? What if we drastically reduce to 10 and leave a comment on why?
There was a problem hiding this comment.
makes sense to me, i did the change in d0d52a8
pkg/storegateway/bucket_test.go
Outdated
| } | ||
| for testName, bucketStoreOpts := range map[string][]BucketStoreOption{ | ||
| "run with default options": {WithLogger(logger), WithChunkPool(chunkPool)}, | ||
| "with series streaming": {WithLogger(logger), WithChunkPool(chunkPool), WithStreamingSeriesPerBatch(5000)}, |
There was a problem hiding this comment.
Same comment here. How many series do we have in this test?
You may also consider having 2 test cases, one with small batches and one with big batches.
There was a problem hiding this comment.
this runs with many different setups for series - from 1 series to 1M series and different number of samples per series
I made two variations - one with 1K series and one with 10K series. I think those two are more realistic. WDYT?
Signed-off-by: Dimitar Dimitrov <dimitar.dimitrov@grafana.com>
Signed-off-by: Dimitar Dimitrov <dimitar.dimitrov@grafana.com>
Signed-off-by: Dimitar Dimitrov <dimitar.dimitrov@grafana.com>
Signed-off-by: Dimitar Dimitrov <dimitar.dimitrov@grafana.com>
…stAndResponseHints Signed-off-by: Marco Pracucci <marco@pracucci.com>
This PR also removes the streaming implementation from
TestSeries_BlockWithMultipleChunks, which I think is ok because wealready have unit tests for multiple chunks in our units
TestLabelNamesAndValuesHintslabel values aren't using streamingstill anyway
TestSeries_ErrorUnmarshallingRequestHintsthis should be independentof how series are fetched (streaming or not)
pkg/storegateway now takes 109.121s to run on my machine compared to the
94.677s it took before