Skip to content

tsdb: Do not reuse the same crc32 hash between Chunk() calls#6514

Closed
roidelapluie wants to merge 1 commit intoprometheus:masterfrom
roidelapluie:crc32
Closed

tsdb: Do not reuse the same crc32 hash between Chunk() calls#6514
roidelapluie wants to merge 1 commit intoprometheus:masterfrom
roidelapluie:crc32

Conversation

@roidelapluie
Copy link
Copy Markdown
Member

The same crc32 object was called concurrently between chunks. As this
object is only used in that function, create a new one each time the
function is run.

See #6512

Signed-off-by: Julien Pivotto roidelapluie@inuits.eu

@roidelapluie
Copy link
Copy Markdown
Member Author

roidelapluie commented Dec 24, 2019

Now working on a test.

@roidelapluie roidelapluie force-pushed the crc32 branch 4 times, most recently from 9394686 to 9c4eef2 Compare December 24, 2019 11:05
The same crc32 object was called concurrently between chunks. As this
object is only used in that function, create a new one each time the
function is run.

See prometheus#6512

Signed-off-by: Julien Pivotto <roidelapluie@inuits.eu>
@roidelapluie
Copy link
Copy Markdown
Member Author

I am not sure that it is enough ; now it runs fine if r.Chunk(chkExp.Ref) is called concurrently with the same chkExp.Ref but not otherwise.

@roidelapluie
Copy link
Copy Markdown
Member Author

more-complex-than-it-looks

@bwplotka
Copy link
Copy Markdown
Member

Thanks for starting this up. Looks like we are also reusing buf = [binary.MaxVarintLen32]byte{} This is to reuse the same buffer for calculating the checksum. The easiest would be to just do Sum(nil), but we can consider adding sync.Pool for this. For 5 bytes I think we can + slice header I think it's not worth it.

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.

2 participants