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

compression/appendToBlockAndRotate-bug-fix: fix a panic caused by under-allocated write buffer #34

Merged
merged 1 commit into from
Jul 8, 2022

Conversation

bom-d-van
Copy link
Member

@bom-d-van bom-d-van commented Jul 6, 2022

The bug was noticed in a production whisper file with a stacktrace attached bellow.

Essentially, it's due to MaxCompressedPointSize size being under-estimated by 0.125 bits and caused an
off-by-1 panics in *archiveInfo.AppendPointsToBlock. Instead of increasing the MaxCompressedPointSize
const value, I decided to go with a more generous buffer allocation in *archiveInfo.appendToBlockAndRotate.

More explanations are in the comment of the changes.

panic: update_many_for_archive panics: runtime error: slice bounds out of range [:20] with capacity 19
goroutine 1 [running]:
runtime/debug.Stack()
	/opt/homebrew/Cellar/go/1.18.1/libexec/src/runtime/debug/stack.go:24 +0x68
github.com/go-graphite/go-whisper.(*Whisper).UpdateManyForArchive.func1()
	~/go/src/github.com/go-graphite/go-whisper/whisper.go:901 +0x40
panic({0x1010b1260, 0x1400014e060})
	/opt/homebrew/Cellar/go/1.18.1/libexec/src/runtime/panic.go:838 +0x204
github.com/go-graphite/go-whisper.(*archiveInfo).AppendPointsToBlock.func1()
	~/go/src/github.com/go-graphite/go-whisper/compress.go:888 +0x390
github.com/go-graphite/go-whisper.(*archiveInfo).AppendPointsToBlock(0x1400014c120, {0x1400014e030, 0x13, 0x0?}, {0x14000160000, 0x1, 0x3c})
	~/go/src/github.com/go-graphite/go-whisper/compress.go:1108 +0x264
github.com/go-graphite/go-whisper.(*archiveInfo).appendToBlockAndRotate(0x1400014c120, {0x14000160000, 0x1, 0x3c})
	~/go/src/github.com/go-graphite/go-whisper/compress.go:606 +0xc4
github.com/go-graphite/go-whisper.(*Whisper).archiveUpdateManyCompressed(0x14000150000, 0x1400014c120, {0x14000124028?, 0x10106d458?, 0x10109e660?})
	~/go/src/github.com/go-graphite/go-whisper/compress.go:537 +0x4b4
github.com/go-graphite/go-whisper.(*Whisper).UpdateManyForArchive(0x14000150000, {0x14000124028?, 0x1, 0x1}, 0xffffffffffffffff)
	~/go/src/github.com/go-graphite/go-whisper/whisper.go:943 +0x2a8
github.com/go-graphite/go-whisper.(*Whisper).UpdateMany(...)
	~/go/src/github.com/go-graphite/go-whisper/whisper.go:871
main.main()
	~/go/src/github.com/go-graphite/go-whisper/cmd/write.go:106 +0x748

goroutine 1 [running]:
main.main()
	~/go/src/github.com/go-graphite/go-whisper/cmd/write.go:107 +0x7d8
exit status 2

@bom-d-van bom-d-van force-pushed the compression/appendToBlockAndRotate-bug-fix branch from 6d80df3 to 026381b Compare July 6, 2022 14:06
…er-allocated write buffer

The bug was noticed in a production whisper file with a stacktrace attached bellow.

Essentially, it's due to MaxCompressedPointSize size being under-estimated by 0.125 bits and caused an
off-by-1 panics in *archiveInfo.AppendPointsToBlock. Instead of increasing the MaxCompressedPointSize
const value, I decided to go with a more geneours buffer allocation in *archiveInfo.appendToBlockAndRotate.

More explanations are in the comment of the changes.

```
panic: update_many_for_archive panics: runtime error: slice bounds out of range [:20] with capacity 19
goroutine 1 [running]:
runtime/debug.Stack()
	/opt/homebrew/Cellar/go/1.18.1/libexec/src/runtime/debug/stack.go:24 +0x68
github.com/go-graphite/go-whisper.(*Whisper).UpdateManyForArchive.func1()
	~/go/src/github.com/go-graphite/go-whisper/whisper.go:901 +0x40
panic({0x1010b1260, 0x1400014e060})
	/opt/homebrew/Cellar/go/1.18.1/libexec/src/runtime/panic.go:838 +0x204
github.com/go-graphite/go-whisper.(*archiveInfo).AppendPointsToBlock.func1()
	~/go/src/github.com/go-graphite/go-whisper/compress.go:888 +0x390
github.com/go-graphite/go-whisper.(*archiveInfo).AppendPointsToBlock(0x1400014c120, {0x1400014e030, 0x13, 0x0?}, {0x14000160000, 0x1, 0x3c})
	~/go/src/github.com/go-graphite/go-whisper/compress.go:1108 +0x264
github.com/go-graphite/go-whisper.(*archiveInfo).appendToBlockAndRotate(0x1400014c120, {0x14000160000, 0x1, 0x3c})
	~/go/src/github.com/go-graphite/go-whisper/compress.go:606 +0xc4
github.com/go-graphite/go-whisper.(*Whisper).archiveUpdateManyCompressed(0x14000150000, 0x1400014c120, {0x14000124028?, 0x10106d458?, 0x10109e660?})
	~/go/src/github.com/go-graphite/go-whisper/compress.go:537 +0x4b4
github.com/go-graphite/go-whisper.(*Whisper).UpdateManyForArchive(0x14000150000, {0x14000124028?, 0x1, 0x1}, 0xffffffffffffffff)
	~/go/src/github.com/go-graphite/go-whisper/whisper.go:943 +0x2a8
github.com/go-graphite/go-whisper.(*Whisper).UpdateMany(...)
	~/go/src/github.com/go-graphite/go-whisper/whisper.go:871
main.main()
	~/go/src/github.com/go-graphite/go-whisper/cmd/write.go:106 +0x748

goroutine 1 [running]:
main.main()
	~/go/src/github.com/go-graphite/go-whisper/cmd/write.go:107 +0x7d8
exit status 2
```
@bom-d-van bom-d-van force-pushed the compression/appendToBlockAndRotate-bug-fix branch from 026381b to 0fdbe10 Compare July 6, 2022 14:09
bom-d-van added a commit to go-graphite/go-carbon that referenced this pull request Jul 8, 2022
bom-d-van added a commit to go-graphite/go-carbon that referenced this pull request Jul 8, 2022
bom-d-van added a commit to go-graphite/go-carbon that referenced this pull request Jul 8, 2022
@bom-d-van bom-d-van merged commit c95739c into master Jul 8, 2022
@bom-d-van bom-d-van deleted the compression/appendToBlockAndRotate-bug-fix branch July 8, 2022 09:49
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.

None yet

1 participant