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

fix: TestRecordBatchDecoding failing sporadically #2154

Merged
merged 1 commit into from
Feb 24, 2022

Conversation

k-wall
Copy link
Contributor

@k-wall k-wall commented Feb 19, 2022

TestRecordBatchDecoding was sporadically failing for me on main (6d970c7). This test command normally brings out the failure:

go test -p 2  -run 'TestRecordBatch.*' -count 10
--- FAIL: TestRecordBatchDecoding (0.00s)
    record_test.go:291: invalid decode of snappy compressed record
        got {FirstOffset:0 PartitionLeaderEpoch:0 Version:2 Codec:snappy CompressionLevel:0 Control:false LogAppendTime:false LastOffsetDelta:0 FirstTimestamp:2016-11-22 20:49:55 +0000 GMT MaxTimestamp:1970-01-01 01:00:00 +0100 BST ProducerID:0 ProducerEpoch:0 FirstSequence:0 Records:[<*>(0xc00020e0e0){Headers:[<*>(0xc000210090){Key:[8 9 10] Value:[11 12]}] Attributes:0 TimestampDelta:5ms OffsetDelta:0 Key:[1 2 3 4] Value:[5 6 7] length:{startOffset:0 length:0}}] PartialTrailingRecord:false IsTransactional:false compressedRecords:<nil> recordsLen:21}
        wanted {FirstOffset:0 PartitionLeaderEpoch:0 Version:2 Codec:snappy CompressionLevel:0 Control:false LogAppendTime:false LastOffsetDelta:0 FirstTimestamp:2016-11-22 20:49:55 +0000 GMT MaxTimestamp:1970-01-01 01:00:00 +0100 BST ProducerID:0 ProducerEpoch:0 FirstSequence:0 Records:[<*>(0x17b9000){Headers:[<*>(0x17b4680){Key:[8 9 10] Value:[11 12]}] Attributes:0 TimestampDelta:5ms OffsetDelta:0 Key:[1 2 3 4] Value:[5 6 7] length:{startOffset:0 length:20}}] PartialTrailingRecord:false IsTransactional:false compressedRecords:<nil> recordsLen:21}
--- FAIL: TestRecordBatchDecoding (0.00s)
    record_test.go:291: invalid decode of snappy compressed record
        got {FirstOffset:0 PartitionLeaderEpoch:0 Version:2 Codec:snappy CompressionLevel:0 Control:false LogAppendTime:false LastOffsetDelta:0 FirstTimestamp:2016-11-22 20:49:55 +0000 GMT MaxTimestamp:1970-01-01 01:00:00 +0100 BST ProducerID:0 ProducerEpoch:0 FirstSequence:0 Records:[<*>(0xc0001d60e0){Headers:[<*>(0xc000c160c0){Key:[8 9 10] Value:[11 12]}] Attributes:0 TimestampDelta:5ms OffsetDelta:0 Key:[1 2 3 4] Value:[5 6 7] length:{startOffset:0 length:0}}] PartialTrailingRecord:false IsTransactional:false compressedRecords:<nil> recordsLen:21}
        wanted {FirstOffset:0 PartitionLeaderEpoch:0 Version:2 Codec:snappy CompressionLevel:0 Control:false LogAppendTime:false LastOffsetDelta:0 FirstTimestamp:2016-11-22 20:49:55 +0000 GMT MaxTimestamp:1970-01-01 01:00:00 +0100 BST ProducerID:0 ProducerEpoch:0 FirstSequence:0 Records:[<*>(0x17b9000){Headers:[<*>(0x17b4680){Key:[8 9 10] Value:[11 12]}] Attributes:0 TimestampDelta:5ms OffsetDelta:0 Key:[1 2 3 4] Value:[5 6 7] length:{startOffset:0 length:20}}] PartialTrailingRecord:false IsTransactional:false compressedRecords:<nil> recordsLen:21}
FAIL

The issues has been brought out by the switch to parallel tests. TestRecordBatchDecoding and TestRecordBatchEncoding use the same test data - which includes *Record references. If TestRecordBatchEncoding races with TestRecordBatchDecoding, the action of the encode() causes the mutation of Record.varintLengthField within the test data. This can then cause the test assertions made by TestRecordBatchDecoding to fail.

uses copy of test data to ensure independence between encoding and decoding tests.

Signed-off-by: kwall <[email protected]>
@k-wall k-wall force-pushed the fix-record-batch-decoding branch from 2ea4f69 to cad675a Compare February 19, 2022 17:00
Copy link
Collaborator

@dnwe dnwe left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks! Good catch

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants