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

changefeedccl: kafka v2 sink does not allow you to set GZIP compression levels less than zero #136492

Closed
asg0451 opened this issue Dec 2, 2024 · 4 comments · Fixed by #137646
Closed
Labels
A-cdc Change Data Capture branch-master Failures and bugs on the master branch. branch-release-24.1 Used to mark GA and release blockers, technical advisories, and bugs for 24.1 branch-release-24.2 Used to mark GA and release blockers, technical advisories, and bugs for 24.2 branch-release-24.3 Used to mark GA and release blockers, technical advisories, and bugs for 24.3 C-bug Code not up to spec/doc, specs & docs deemed correct. Solution expected to change code/behavior. E-easy Easy issue to tackle, requires little or no CockroachDB experience E-quick-win Likely to be a quick win for someone experienced. T-cdc

Comments

@asg0451
Copy link
Contributor

asg0451 commented Dec 2, 2024

The v1 Kafka sink allows GZIP compression levels in the range [-3,9], with the negative ones having special meanings. However, the v2 sink disallows negative GZIP compression levels. We should fix this inconsistency.

@asg0451 asg0451 added C-bug Code not up to spec/doc, specs & docs deemed correct. Solution expected to change code/behavior. A-cdc Change Data Capture branch-master Failures and bugs on the master branch. T-cdc branch-release-24.1 Used to mark GA and release blockers, technical advisories, and bugs for 24.1 branch-release-24.2 Used to mark GA and release blockers, technical advisories, and bugs for 24.2 branch-release-24.3 Used to mark GA and release blockers, technical advisories, and bugs for 24.3 labels Dec 2, 2024
Copy link

blathers-crl bot commented Dec 2, 2024

cc @cockroachdb/cdc

@rharding6373 rharding6373 added E-easy Easy issue to tackle, requires little or no CockroachDB experience E-quick-win Likely to be a quick win for someone experienced. labels Dec 2, 2024
@asg0451 asg0451 changed the title changefeedccl: kafka v2 sink does not allow you to set compression levels less than zero changefeedccl: kafka v2 sink does not allow you to set GZIP compression levels less than zero Dec 3, 2024
@yaothao
Copy link
Contributor

yaothao commented Dec 16, 2024

Hello @asg0451, I'm eager to contribute to this issue as a newcomer to open-source projects. I've encountered some confusion regarding the compression level range you described compared to what's allowed in kgo. Specifically, based on the constant value definitions found in gzip.go and deflate.go, there seems to be no definition for level -3, which you referred to as stateless compression in #19169. Could you please clarify the compression requirements? Your guidance would be greatly appreciated.

@asg0451
Copy link
Contributor Author

asg0451 commented Dec 17, 2024

Hey @yaothao, thanks for reaching out. It looks like the confusion is that our v1 Kafka sink, using sarama, uses the klauspost/compress library, whereas our v2 sink, using franz-go, uses the stdlib's compression/gzip package. The former supports the -3 level, whereas the latter doesn't.

If you still end up picking this up, please mention this inconsistency in your release notes so we can document it.

Thanks!

yaothao pushed a commit to yaothao/cockroach that referenced this issue Dec 17, 2024
Release note (bug fix): Adjust kafka v2 sink to support negative GZIP.
Please document compression level range [-2,9]
yaothao pushed a commit to yaothao/cockroach that referenced this issue Dec 17, 2024
Fixes: cockroachdb#136492.
Release note (bug fix): Adjust kafka v2 sink to support negative GZIP.
Please document compression level range [-2,9]

Fixes: cockroachdb#136492.
Release note (bug fix): Adjust kafka v2 sink to support negative GZIP compression level.
Please document compression level with range [-2,9]
yaothao pushed a commit to yaothao/cockroach that referenced this issue Dec 17, 2024
Release note (bug fix): Adjust kafka v2 sink to support negative GZIP
Please document compression level with range [-2,9]
@yaothao
Copy link
Contributor

yaothao commented Dec 18, 2024

Hi @asg0451, thank you for the clarification. I've submitted a new PR and addressed the -3 issue in the release note. Please let me know if there's anything more I can assist with regarding this matter.

yaothao added a commit to yaothao/cockroach that referenced this issue Dec 18, 2024
Fixes: cockroachdb#136492
Release note (bug fix): fixed a bug introduced in kafka v2 sink.
Previously, the Kafka v2 sink configuration did not accept negative
values for the compression level setting, despite -1 and -2 having
special meanings within the system.

Action required: Please update the corresponding documentation
to reflect the correct compression level with range [-2,9]
yaothao added a commit to yaothao/cockroach that referenced this issue Dec 18, 2024
Fixes: cockroachdb#136492
Release note (bug fix): fixed a bug introduced in kafka v2 sink.
Previously, the Kafka v2 sink configuration did not accept negative
values for the compression level setting, despite -1 and -2 having
special meanings within the system.

Action required: Please update the corresponding documentation
to reflect the correct compression level with range [-2,9]
yaothao added a commit to yaothao/cockroach that referenced this issue Dec 19, 2024
Previously, the Kafka v2 sink could not properly handle negative
compression levels due to differences in the underlying compression
libraries used between the v1 and v2 sinks. The Kafka v1 sink
implementation, which relies on the Sarama library, uses the
klauspost/compress library that supports a compression level of -3.
However, our v2 sink has transitioned to using franz-go, which utilizes
the standard library's compression/gzip, and does not support the -3
level. In this update, the validation function now checks the GZIP
compression range between HuffmanOnly (-2) and BestCompression (9).

Fixes: cockroachdb#136492

Epic: none

Release note (bug fix): We have resolved an issue in the Kafka v2 sink
configuration within CockroachDB, where users were previously unable to
set negative GZIP compression levels. Now, users can configure the
CompressionLevel for the Kafka sink in the range of [-2, 9]. Please
update the user guide to include the new valid GZIP compression level
range of [-2, 9], where -2 enables Huffman encoding and -1 sets the
default compression.
`#Release note: None
craig bot pushed a commit that referenced this issue Jan 2, 2025
137646: changefeedccl: Fix Kafka v2 Sink GZIP Compression Level Issue r=asg0451 a=yaothao

Previously, the Kafka v2 sink could not properly handle negative
compression levels due to differences in the underlying compression
libraries used between the v1 and v2 sinks. The Kafka v1 sink
implementation, which relies on the Sarama library, uses the
klauspost/compress library that supports a compression level of -3.
However, our v2 sink has transitioned to using franz-go, which utilizes
the standard library's compression/gzip, and does not support the -3
level. In this update, the validation function now checks the GZIP
compression range between HuffmanOnly (-2) and BestCompression (9).

Fixes: #136492

Epic: none

Release note (bug fix): We have resolved an issue in the Kafka v2 sink
configuration within CockroachDB, where users were previously unable to
set negative GZIP compression levels. Now, users can configure the
CompressionLevel for the Kafka sink in the range of [-2, 9]. Please
update the user guide to include the new valid GZIP compression level
range of [-2, 9], where -2 enables Huffman encoding and -1 sets the
default compression.

137842: testutils: add a helper to capture a side-eye snapshot on demand r=srosenberg a=asg0451

Add a helper to capture a side-eye snapshot on
demand. This can be used to investigate test
failures that you can reproduce locally but only
by running the test many times.

Epic: None

Release note: None


Co-authored-by: yaothao <[email protected]>
Co-authored-by: Miles Frankel <[email protected]>
@craig craig bot closed this as completed in 490865c Jan 2, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-cdc Change Data Capture branch-master Failures and bugs on the master branch. branch-release-24.1 Used to mark GA and release blockers, technical advisories, and bugs for 24.1 branch-release-24.2 Used to mark GA and release blockers, technical advisories, and bugs for 24.2 branch-release-24.3 Used to mark GA and release blockers, technical advisories, and bugs for 24.3 C-bug Code not up to spec/doc, specs & docs deemed correct. Solution expected to change code/behavior. E-easy Easy issue to tackle, requires little or no CockroachDB experience E-quick-win Likely to be a quick win for someone experienced. T-cdc
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants