Catch context error in the s3 bucket client #5240

damnever · 2023-03-30T09:10:30Z

What this PR does:

Store gateway may panic if the s3 client return nil error with some nil value.

panic: runtime error: invalid memory address or nil pointer dereference
        panic: runtime error: invalid memory address or nil pointer dereference
[signal SIGSEGV: segmentation violation code=0x1 addr=0x18 pc=0x12a2e04]

goroutine 10142 [running]:
github.com/thanos-io/objstore.(*timingReadCloser).Close(0xc01643ea80)
        /go/src/github.com/cortexproject/cortex/vendor/github.com/thanos-io/objstore/objstore.go:638 +0x24
github.com/thanos-io/objstore.(*tracingReadCloser).Close(0xc0187b2e80)
        /go/src/github.com/cortexproject/cortex/vendor/github.com/thanos-io/objstore/tracing.go:149 +0x2d
github.com/thanos-io/thanos/pkg/runutil.CloseWithLogOnErr({0x3e2b8c0, 0xc000dafb80}, {0x7f16340f7a60?, 0xc0187b2e80?}, {0x373dd29, 0x21}, {0x0, 0x0, 0x0})
        /go/src/github.com/cortexproject/cortex/vendor/github.com/thanos-io/thanos/pkg/runutil/runutil.go:111 +0x77
panic({0x31628e0, 0x5903850})
        /usr/local/go/src/runtime/panic.go:884 +0x213
github.com/thanos-io/objstore.(*timingReadCloser).Read(0xc01643ea80, {0xc0106c1b80?, 0xc004424eb0?, 0xc0187ff860?})
        /go/src/github.com/cortexproject/cortex/vendor/github.com/thanos-io/objstore/objstore.go:650 +0x29
github.com/thanos-io/objstore.(*tracingReadCloser).Read(0xc0187b2e80, {0xc0106c1b80?, 0xc0106c1b80?, 0x0?})
        /go/src/github.com/cortexproject/cortex/vendor/github.com/thanos-io/objstore/tracing.go:138 +0x32
bytes.(*Buffer).ReadFrom(0xc001729d70, {0x7f16340f7a80, 0xc0187b2e80})
        /usr/local/go/src/bytes/buffer.go:202 +0x98
github.com/thanos-io/thanos/pkg/store.(*bucketBlock).readIndexRange(0xc000261180, {0x3e55d38, 0xc004424eb0}, 0x0?, 0x48)
        /go/src/github.com/cortexproject/cortex/vendor/github.com/thanos-io/thanos/pkg/store/bucket.go:2008 +0x1e5
github.com/thanos-io/thanos/pkg/s

Checklist

Tests updated
Documentation added
CHANGELOG.md updated - the order of entries should be [CHANGE], [FEATURE], [ENHANCEMENT], [BUGFIX]

damnever · 2023-03-30T14:04:18Z

We should bring this into v1.15

@damnever

Signed-off-by: Xiaochao Dong (@damnever) <[email protected]>

yeya24

This LGTM! Thanks for the fix.

friedrichg

Fantastic test!

@damnever

Signed-off-by: Xiaochao Dong (@damnever) <[email protected]> Signed-off-by: Ben Ye <[email protected]>

@damnever

* Batch Iterator optimization (#5237) * Batch Opmization Signed-off-by: Alan Protasio <[email protected]> * Add test bacj Signed-off-by: Alan Protasio <[email protected]> * Testing Multiples scrape intervals Signed-off-by: Alan Protasio <[email protected]> * no assimption Signed-off-by: Alan Protasio <[email protected]> * Using max chunk ts Signed-off-by: Alan Protasio <[email protected]> * test with scrape 10 Signed-off-by: Alan Protasio <[email protected]> * rename method Signed-off-by: Alan Protasio <[email protected]> * comments Signed-off-by: Alan Protasio <[email protected]> * using next Signed-off-by: Alan Protasio <[email protected]> * change test name Signed-off-by: Alan Protasio <[email protected]> * changelog/comments Signed-off-by: Alan Protasio <[email protected]> --------- Signed-off-by: Alan Protasio <[email protected]> Signed-off-by: Ben Ye <[email protected]> * Store Gateway: Convert metrics from summary to histograms (#5239) * Convert following metrics from summary to histogram cortex_bucket_store_series_blocks_queried cortex_bucket_store_series_data_fetched cortex_bucket_store_series_data_size_touched_bytes cortex_bucket_store_series_data_size_fetched_bytes cortex_bucket_store_series_data_touched cortex_bucket_store_series_result_series Signed-off-by: Friedrich Gonzalez <[email protected]> * Update changelog Signed-off-by: Friedrich Gonzalez <[email protected]> * fix changelog Signed-off-by: Friedrich Gonzalez <[email protected]> --------- Signed-off-by: Friedrich Gonzalez <[email protected]> Signed-off-by: Ben Ye <[email protected]> * update changelog Signed-off-by: Ben Ye <[email protected]> * Catch context error in the s3 bucket client (#5240) Signed-off-by: Xiaochao Dong (@damnever) <[email protected]> Signed-off-by: Ben Ye <[email protected]> * bump RC version Signed-off-by: Ben Ye <[email protected]> --------- Signed-off-by: Alan Protasio <[email protected]> Signed-off-by: Ben Ye <[email protected]> Signed-off-by: Friedrich Gonzalez <[email protected]> Signed-off-by: Xiaochao Dong (@damnever) <[email protected]> Co-authored-by: Alan Protasio <[email protected]> Co-authored-by: Friedrich Gonzalez <[email protected]> Co-authored-by: Xiaochao Dong <[email protected]>

@damnever

* prepare 1.15.0-rc release (#5235) Signed-off-by: Ben Ye <[email protected]> * Cherry-pick fixes to release 1.15 branch (#5241) * Batch Iterator optimization (#5237) * Batch Opmization Signed-off-by: Alan Protasio <[email protected]> * Add test bacj Signed-off-by: Alan Protasio <[email protected]> * Testing Multiples scrape intervals Signed-off-by: Alan Protasio <[email protected]> * no assimption Signed-off-by: Alan Protasio <[email protected]> * Using max chunk ts Signed-off-by: Alan Protasio <[email protected]> * test with scrape 10 Signed-off-by: Alan Protasio <[email protected]> * rename method Signed-off-by: Alan Protasio <[email protected]> * comments Signed-off-by: Alan Protasio <[email protected]> * using next Signed-off-by: Alan Protasio <[email protected]> * change test name Signed-off-by: Alan Protasio <[email protected]> * changelog/comments Signed-off-by: Alan Protasio <[email protected]> --------- Signed-off-by: Alan Protasio <[email protected]> Signed-off-by: Ben Ye <[email protected]> * Store Gateway: Convert metrics from summary to histograms (#5239) * Convert following metrics from summary to histogram cortex_bucket_store_series_blocks_queried cortex_bucket_store_series_data_fetched cortex_bucket_store_series_data_size_touched_bytes cortex_bucket_store_series_data_size_fetched_bytes cortex_bucket_store_series_data_touched cortex_bucket_store_series_result_series Signed-off-by: Friedrich Gonzalez <[email protected]> * Update changelog Signed-off-by: Friedrich Gonzalez <[email protected]> * fix changelog Signed-off-by: Friedrich Gonzalez <[email protected]> --------- Signed-off-by: Friedrich Gonzalez <[email protected]> Signed-off-by: Ben Ye <[email protected]> * update changelog Signed-off-by: Ben Ye <[email protected]> * Catch context error in the s3 bucket client (#5240) Signed-off-by: Xiaochao Dong (@damnever) <[email protected]> Signed-off-by: Ben Ye <[email protected]> * bump RC version Signed-off-by: Ben Ye <[email protected]> --------- Signed-off-by: Alan Protasio <[email protected]> Signed-off-by: Ben Ye <[email protected]> Signed-off-by: Friedrich Gonzalez <[email protected]> Signed-off-by: Xiaochao Dong (@damnever) <[email protected]> Co-authored-by: Alan Protasio <[email protected]> Co-authored-by: Friedrich Gonzalez <[email protected]> Co-authored-by: Xiaochao Dong <[email protected]> * Cherry-pick fixes to release 1.15 for new RC (#5259) * fix remote read error in query frontend (#5257) * fix remote read error in query frontend Signed-off-by: Ben Ye <[email protected]> * fix integration test Signed-off-by: Ben Ye <[email protected]> * add extra one query Signed-off-by: Ben Ye <[email protected]> --------- Signed-off-by: Ben Ye <[email protected]> * bump RC version Signed-off-by: Ben Ye <[email protected]> --------- Signed-off-by: Ben Ye <[email protected]> * Fix splitByInterval incorrect error response format (#5260) (#5261) * fix query frontend incorrect error response format * update changelog * fix integration test --------- Signed-off-by: Ben Ye <[email protected]> * release 1.15.0 (#5274) Signed-off-by: Ben Ye <[email protected]> * merge 1.15 into master and resolve changelog conflicts Signed-off-by: Ben Ye <[email protected]> --------- Signed-off-by: Ben Ye <[email protected]> Signed-off-by: Alan Protasio <[email protected]> Signed-off-by: Friedrich Gonzalez <[email protected]> Signed-off-by: Xiaochao Dong (@damnever) <[email protected]> Co-authored-by: Alan Protasio <[email protected]> Co-authored-by: Friedrich Gonzalez <[email protected]> Co-authored-by: Xiaochao Dong <[email protected]>

@damnever

Signed-off-by: Xiaochao Dong (@damnever) <[email protected]>

@damnever

* prepare 1.15.0-rc release (cortexproject#5235) Signed-off-by: Ben Ye <[email protected]> * Cherry-pick fixes to release 1.15 branch (cortexproject#5241) * Batch Iterator optimization (cortexproject#5237) * Batch Opmization Signed-off-by: Alan Protasio <[email protected]> * Add test bacj Signed-off-by: Alan Protasio <[email protected]> * Testing Multiples scrape intervals Signed-off-by: Alan Protasio <[email protected]> * no assimption Signed-off-by: Alan Protasio <[email protected]> * Using max chunk ts Signed-off-by: Alan Protasio <[email protected]> * test with scrape 10 Signed-off-by: Alan Protasio <[email protected]> * rename method Signed-off-by: Alan Protasio <[email protected]> * comments Signed-off-by: Alan Protasio <[email protected]> * using next Signed-off-by: Alan Protasio <[email protected]> * change test name Signed-off-by: Alan Protasio <[email protected]> * changelog/comments Signed-off-by: Alan Protasio <[email protected]> --------- Signed-off-by: Alan Protasio <[email protected]> Signed-off-by: Ben Ye <[email protected]> * Store Gateway: Convert metrics from summary to histograms (cortexproject#5239) * Convert following metrics from summary to histogram cortex_bucket_store_series_blocks_queried cortex_bucket_store_series_data_fetched cortex_bucket_store_series_data_size_touched_bytes cortex_bucket_store_series_data_size_fetched_bytes cortex_bucket_store_series_data_touched cortex_bucket_store_series_result_series Signed-off-by: Friedrich Gonzalez <[email protected]> * Update changelog Signed-off-by: Friedrich Gonzalez <[email protected]> * fix changelog Signed-off-by: Friedrich Gonzalez <[email protected]> --------- Signed-off-by: Friedrich Gonzalez <[email protected]> Signed-off-by: Ben Ye <[email protected]> * update changelog Signed-off-by: Ben Ye <[email protected]> * Catch context error in the s3 bucket client (cortexproject#5240) Signed-off-by: Xiaochao Dong (@damnever) <[email protected]> Signed-off-by: Ben Ye <[email protected]> * bump RC version Signed-off-by: Ben Ye <[email protected]> --------- Signed-off-by: Alan Protasio <[email protected]> Signed-off-by: Ben Ye <[email protected]> Signed-off-by: Friedrich Gonzalez <[email protected]> Signed-off-by: Xiaochao Dong (@damnever) <[email protected]> Co-authored-by: Alan Protasio <[email protected]> Co-authored-by: Friedrich Gonzalez <[email protected]> Co-authored-by: Xiaochao Dong <[email protected]> * Cherry-pick fixes to release 1.15 for new RC (cortexproject#5259) * fix remote read error in query frontend (cortexproject#5257) * fix remote read error in query frontend Signed-off-by: Ben Ye <[email protected]> * fix integration test Signed-off-by: Ben Ye <[email protected]> * add extra one query Signed-off-by: Ben Ye <[email protected]> --------- Signed-off-by: Ben Ye <[email protected]> * bump RC version Signed-off-by: Ben Ye <[email protected]> --------- Signed-off-by: Ben Ye <[email protected]> * Fix splitByInterval incorrect error response format (cortexproject#5260) (cortexproject#5261) * fix query frontend incorrect error response format * update changelog * fix integration test --------- Signed-off-by: Ben Ye <[email protected]> * release 1.15.0 (cortexproject#5274) Signed-off-by: Ben Ye <[email protected]> * merge 1.15 into master and resolve changelog conflicts Signed-off-by: Ben Ye <[email protected]> --------- Signed-off-by: Ben Ye <[email protected]> Signed-off-by: Alan Protasio <[email protected]> Signed-off-by: Friedrich Gonzalez <[email protected]> Signed-off-by: Xiaochao Dong (@damnever) <[email protected]> Co-authored-by: Alan Protasio <[email protected]> Co-authored-by: Friedrich Gonzalez <[email protected]> Co-authored-by: Xiaochao Dong <[email protected]>

pull-request-size bot added the size/S label Mar 30, 2023

damnever force-pushed the fix/retry-panic branch from f62db14 to 24f71bc Compare March 30, 2023 09:12

Catch context error in the s3 bucket client

dd5c560

Signed-off-by: Xiaochao Dong (@damnever) <[email protected]>

damnever force-pushed the fix/retry-panic branch from 24f71bc to dd5c560 Compare March 31, 2023 03:41

damnever mentioned this pull request Mar 31, 2023

Cherry-pick fixes to release 1.15 branch #5241

Merged

yeya24 approved these changes Mar 31, 2023

View reviewed changes

friedrichg approved these changes Mar 31, 2023

View reviewed changes

yeya24 merged commit 3b84671 into cortexproject:master Mar 31, 2023

alvinlin123 added this to the Release 1.15 milestone Mar 31, 2023

yeya24 pushed a commit to yeya24/cortex that referenced this pull request Apr 1, 2023

Catch context error in the s3 bucket client (cortexproject#5240)

711c9b7

Signed-off-by: Xiaochao Dong (@damnever) <[email protected]> Signed-off-by: Ben Ye <[email protected]>

yeya24 pushed a commit to yeya24/cortex that referenced this pull request Apr 28, 2023

Catch context error in the s3 bucket client (cortexproject#5240)

34f0424

Signed-off-by: Xiaochao Dong (@damnever) <[email protected]>

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Catch context error in the s3 bucket client #5240

Catch context error in the s3 bucket client #5240

Uh oh!

damnever commented Mar 30, 2023 •

edited

Loading

Uh oh!

damnever commented Mar 30, 2023

Uh oh!

yeya24 left a comment

Uh oh!

friedrichg left a comment

Uh oh!

Uh oh!

Catch context error in the s3 bucket client #5240

Catch context error in the s3 bucket client #5240

Uh oh!

Conversation

damnever commented Mar 30, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

damnever commented Mar 30, 2023

Uh oh!

yeya24 left a comment

Choose a reason for hiding this comment

Uh oh!

friedrichg left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

damnever commented Mar 30, 2023 •

edited

Loading