Skip to content
This repository has been archived by the owner on Jan 24, 2024. It is now read-only.

[FEATURE] Add fetch down converted when entryFormat=kafka #660

Merged
merged 20 commits into from
Aug 19, 2021

Conversation

wenbingshen
Copy link
Contributor

@wenbingshen wenbingshen commented Aug 18, 2021

Fixes #656

Motivation

When entryFormat=kafka

If SASL/PLAIN authentication is not turned on, so kop does not limit the version of the producer client. When a lower version producer writes a message of magic=0 or magic=1, it will cause the higher version consumer to check the record error and the consumer client will be down.

Modifications

When entryFormat=kafka, increase KafkaEntryFormatter.decode to check batch.magic and client magic of kafka records. When batch.magic is higher than client magic, perform down conversion.

This pr is part of the support for the lower version of Kafka less than 0.11.x. Since the FETCH version is still 4, it is also a bugfix for the higher version consumer to read the lower version of the magic record verification error.

@wenbingshen wenbingshen marked this pull request as draft August 18, 2021 11:08
@wenbingshen wenbingshen marked this pull request as ready for review August 18, 2021 16:20
@wenbingshen wenbingshen changed the title [Don't merge only test for shade ] Add fetch down converted when entryFormat=kafka [FEATURE] Add fetch down converted when entryFormat=kafka Aug 18, 2021
@wenbingshen
Copy link
Contributor Author

@BewareMyPower For the unit test, I reused BasicEndToEndTestBase, because after adding the kafka-0-10 module, the lower version producer will write a message of magic=1. If the error mentioned in the pr is not fixed, the higher version consumer will report an error and the unit test will fail. For down-conversion, due to some known reasons, the FETCH version is still limited to 4, so after I removed 4, the test lower version consumers can successfully read the higher version data, but I did not release the restriction on the FETCH version=4 to this pr. PTAL.

@wenbingshen
Copy link
Contributor Author

@BewareMyPower Flaky test seems to be related to the new code citations I introduced after merging the master branch. I will investigate, and please review this pr first.

@BewareMyPower BewareMyPower added the type/feature Indicates new functionality label Aug 19, 2021
@BewareMyPower BewareMyPower merged commit 12f7808 into streamnative:master Aug 19, 2021
wangjialing218 pushed a commit to wangjialing218/kop that referenced this pull request Aug 24, 2021
…ve#660)

Fixes streamnative#656 

### Motivation
When entryFormat=kafka

If SASL/PLAIN authentication is not turned on, so kop does not limit the version of the producer client. When a lower version producer writes a message of magic=0 or magic=1, it will cause the higher version consumer to check the record error and the consumer client will be down.

### Modifications
When entryFormat=kafka, increase KafkaEntryFormatter.decode to check batch.magic and client magic of kafka records. When batch.magic is higher than client magic, perform down conversion.

This pr is part of the support for the lower version of Kafka less than 0.11.x. Since the FETCH version is still 4, it is also a bugfix for the higher version consumer to read the lower version of the magic record verification error.
BewareMyPower pushed a commit that referenced this pull request Aug 25, 2021
Fixes #656 

### Motivation
When entryFormat=kafka

If SASL/PLAIN authentication is not turned on, so kop does not limit the version of the producer client. When a lower version producer writes a message of magic=0 or magic=1, it will cause the higher version consumer to check the record error and the consumer client will be down.

### Modifications
When entryFormat=kafka, increase KafkaEntryFormatter.decode to check batch.magic and client magic of kafka records. When batch.magic is higher than client magic, perform down conversion.

This pr is part of the support for the lower version of Kafka less than 0.11.x. Since the FETCH version is still 4, it is also a bugfix for the higher version consumer to read the lower version of the magic record verification error.
@BewareMyPower BewareMyPower mentioned this pull request Aug 27, 2021
9 tasks
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
type/feature Indicates new functionality
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[BUG] Consumer report: Record batch for partition topic-3 at offset xxx is invalid, cause: Record is corrupt
2 participants