Skip to content

Revert "[SPARK-45502][BUILD] Upgrade Kafka to 3.6.0"#43379

Closed
LuciferYang wants to merge 1 commit intoapache:masterfrom
LuciferYang:Revert-SPARK-45502
Closed

Revert "[SPARK-45502][BUILD] Upgrade Kafka to 3.6.0"#43379
LuciferYang wants to merge 1 commit intoapache:masterfrom
LuciferYang:Revert-SPARK-45502

Conversation

@LuciferYang
Copy link
Copy Markdown
Contributor

This reverts commit d1bd21a.

What changes were proposed in this pull request?

This pr aims to revert SPARK-45502 to make the test case KafkaSourceStressSuite stable.

Why are the changes needed?

The test case KafkaSourceStressSuite has become very unstable after the merger of SPARK-45502, with 10 out of the recent 22 tests failing because of it. Revert it for now, and we can upgrade Kafka again after resolving the test issues.

Does this PR introduce any user-facing change?

No

How was this patch tested?

Pass GitHub Actions

Was this patch authored or co-authored using generative AI tooling?

No

Copy link
Copy Markdown
Member

@HyukjinKwon HyukjinKwon left a comment

Choose a reason for hiding this comment

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

I am fine with this but cc @HeartSaVioR

@LuciferYang
Copy link
Copy Markdown
Contributor Author

#43362

I see that the author is trying to fix it, we can also wait a little longer. But since the sbt test is in a fail-fast mode, once KafkaSourceStressSuite fails, the modules to be tested later will not run, making it hard to confirm whether the subsequent tests are healthy. This is a risk point.

also cc @dongjoon-hyun @dengziming

Copy link
Copy Markdown
Contributor

@HeartSaVioR HeartSaVioR left a comment

Choose a reason for hiding this comment

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

+1 on reverting. Thanks @LuciferYang to figure out the unstability.

In addition, I really want to see the "benefits" being listed in PR description when we bump the version. Have we run any benchmark to verify there is no regression? Have we looked into the release notes to highlight the benefits and critical bugfixes to provide rationale of the version bump? Could we please be conservative and pick bugfix versions unless the minor release unblocks us/users for some specific functionalities?

Copy link
Copy Markdown
Member

@dengziming dengziming left a comment

Choose a reason for hiding this comment

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

LGTM, I already find some clues but it's still not enough, lets revert it firstly.

@LuciferYang LuciferYang marked this pull request as ready for review October 16, 2023 03:03
@LuciferYang
Copy link
Copy Markdown
Contributor Author

@LuciferYang
Copy link
Copy Markdown
Contributor Author

Merged into master. Thanks @HyukjinKwon @HeartSaVioR @dengziming ~

@dongjoon-hyun
Copy link
Copy Markdown
Member

Thank you, @LuciferYang and all.

@dengziming
Copy link
Copy Markdown
Member

Could we please be conservative and pick bugfix versions unless the minor release unblocks us/users for some specific functionalities?

Thank you @HeartSaVioR for mention this, I agree with you about picking bugfix versions, however, this bug is introduced by me when I haven't tested it carefully and the last build accidentally succeed, so it was merged by mistake. lets wait and upgrade it to 3.6.1 later.

BTW, I have found the root cause is apache/kafka#12590, we will send a fetch request when closing FetchSession which contains a deleted topic, and we are setting "auto.create.topics.enable"=true so deleted topics will come back again.

@dongjoon-hyun
Copy link
Copy Markdown
Member

Thank you for the info, @dengziming .

@LuciferYang LuciferYang deleted the Revert-SPARK-45502 branch October 18, 2023 05:24
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants