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

Fixed index out of range issue when loading large snapshot #88

Merged
merged 1 commit into from Jul 2, 2023

Conversation

sgosiaco
Copy link
Contributor

I recently found an issue when trying to load a snapshot created with a large number of rows (in my case 10 million).
It would result in an index out of range panic. I was able to reproduce this in the unit tests with around 3 million players.

Running tool: /usr/local/go/bin/go test -timeout 30s -run ^TestLargeSnapshot$ github.com/kelindar/column

--- FAIL: TestLargeSnapshot (4.20s)
panic: runtime error: index out of range [128] with length 128 [recovered]
	panic: runtime error: index out of range [128] with length 128

goroutine 4 [running]:
testing.tRunner.func1.2({0x1053a25c0, 0x14059d74930})
	/usr/local/go/src/testing/testing.go:1389 +0x1c8
testing.tRunner.func1()
	/usr/local/go/src/testing/testing.go:1392 +0x384
panic({0x1053a25c0, 0x14059d74930})
	/usr/local/go/src/runtime/panic.go:838 +0x204
github.com/kelindar/column.(*Collection).readState.func1.1(0x14010b50090)
	/Users/sgosiaco/repos/column/snapshot.go:188 +0x274
github.com/kelindar/column.(*Collection).Query(0x1400012c370, 0x1400005bd18)
	/Users/sgosiaco/repos/column/collection.go:354 +0x48
github.com/kelindar/column.(*Collection).readState.func1(0x1400005bd88?, 0x105250968?)
	/Users/sgosiaco/repos/column/snapshot.go:184 +0x78
github.com/kelindar/iostream.(*Reader).ReadRange(0x1402a07f020, 0x1400010fe10)
	/Users/sgosiaco/go/pkg/mod/github.com/kelindar/iostream@v1.3.0/reader.go:227 +0x8c
github.com/kelindar/column.(*Collection).readState(0x1400012c370, {0x1053c3380?, 0x14000163900?})
	/Users/sgosiaco/repos/column/snapshot.go:183 +0x1d4
github.com/kelindar/column.(*Collection).Restore(0x1400012c370, {0x1053c3080, 0x140001014a0})
	/Users/sgosiaco/repos/column/snapshot.go:44 +0x54
github.com/kelindar/column.TestLargeSnapshot(0x140001151e0)
	/Users/sgosiaco/repos/column/snapshot_test.go:229 +0x174
testing.tRunner(0x140001151e0, 0x1053bf558)
	/usr/local/go/src/testing/testing.go:1439 +0x110
created by testing.(*T).Run
	/usr/local/go/src/testing/testing.go:1486 +0x300
FAIL	github.com/kelindar/column	4.339s
FAIL

Looking at the code I saw that currently the maximum number of chunks able to be read is 128 due to the initialized size of the commits array. I've changed this to a map to allow for an unknown number of chunks/commits to be loaded since when saving the snapshot there doesn't seem to be a limit on the potential number of chunks that can be saved.

@kelindar kelindar enabled auto-merge (squash) July 2, 2023 09:31
@kelindar kelindar disabled auto-merge July 2, 2023 09:38
@kelindar kelindar merged commit 9ba4676 into kelindar:main Jul 2, 2023
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

2 participants