MINOR: clean up unneeded @SuppressWarnings on Streams module#10855
Merged
chia7712 merged 1 commit intoapache:trunkfrom Jun 13, 2021
Merged
MINOR: clean up unneeded @SuppressWarnings on Streams module#10855chia7712 merged 1 commit intoapache:trunkfrom
@SuppressWarnings on Streams module#10855chia7712 merged 1 commit intoapache:trunkfrom
Conversation
Remove unneeded `@SuppressWarnings("unchecked")` in source and test
Remove unneeded `@SuppressWarnings("deprecated")` in source and test
jlprat
changed the title
@SuppressWarnings@SuppressWarnings on Streams module
Contributor
Author
mjsax
approved these changes
Jun 9, 2021
Member
mjsax
left a comment
mjsax
left a comment
There was a problem hiding this comment.
LGTM. Rerunning Jenkins to make sure it passes.
(I remove some annotation in the passed [as it was suggested by IntelliJ], but the build failed later with checkstyle errors -- need to make sure we don't break anything by accident.)
Contributor
Author
|
Failures seem to be:
And
While doing this PR, I also encountered a similar thing you mention @mjsax , somehow IntelliJ needed less |
Contributor
Author
|
@mjsax Could you restart the build? Last time it failed with the |
Contributor
Author
|
Unless the CI is still broken, of course |
showuon
approved these changes
Jun 11, 2021
Member
showuon
left a comment
showuon
left a comment
There was a problem hiding this comment.
LGTM! And @jlprat , yes, I've checked, your exit code 1 is a known issue and I've opened KAFKA-12892 to track it and PR #10820 is ready to bypass it. Thanks
Contributor
Author
|
Thanks for the review and the ticket + PR to fix the issue @showuon! |
Contributor
Author
|
As far as I understand, problem with the failing builds has been solved, is there any way I can run again the Jenkins build? Or only committers can do it? |
Member
|
@jlprat , you can merge with trunk branch to trigger jenkins build again if you want. Thanks. |
chia7712
approved these changes
Jun 13, 2021
Member
chia7712
left a comment
chia7712
left a comment
There was a problem hiding this comment.
LGTM. the failed tests are related to this patch (I re-run them on my local. It seems they are flaky). Merge it to trunk
Contributor
Author
mjsax
added a commit
to confluentinc/kafka
that referenced
this pull request
Jun 15, 2021
Resolve merge conflicts in Jenkins file. * MINOR: clean up unneeded `@SuppressWarnings` (apache#10855) Reviewers: Luke Chen <showuon@gmail.com>, Matthias J. Sax <mjsax@apache.org>, Chia-Ping Tsai <chia7712@gmail.com> * KAFKA-12940: Enable JDK 16 builds in Jenkins (apache#10702) JDK 15 no longer receives updates, so we want to switch from JDK 15 to JDK 16. However, we have a number of tests that don't yet pass with JDK 16. Instead of replacing JDK 15 with JDK 16, we have both for now and we either disable (via annotations) or exclude (via gradle) the tests that don't pass with JDK 16 yet. The annotations approach is better, but it doesn't work for tests that rely on the PowerMock JUnit 4 runner. Also add `--illegal-access=permit` when building with JDK 16 to make MiniKdc work for now. This has been removed in JDK 17, so we'll have to figure out another solution when we migrate to that. Relevant JIRAs for the disabled tests: KAFKA-12790, KAFKA-12941, KAFKA-12942. Moved some assertions from `testTlsDefaults` to `testUnsupportedTlsVersion` since the former claims to test the success case while the former tests the failure case. Reviewers: Chia-Ping Tsai <chia7712@gmail.com> * KAFKA-12921: Upgrade zstd-jni to 1.5.0-2 (apache#10847) This PR aims to upgrade `zstd-jni` from `1.4.9-1` to `1.5.0-2`. This change will incorporate a number of bug fixes and performance improvements made in `1.5.0` of `zstd`: - https://github.com/facebook/zstd/releases/tag/v1.5.0 - https://github.com/luben/zstd-jni/releases/tag/v1.5.0-1 - https://github.com/luben/zstd-jni/releases/tag/v1.5.0-2 The most recent `1.5.0` release offers +25%-140% (compression) and +15% (decompression) performance improvements under certain conditions. Those conditions are unlikely to apply to Kafka with the default configuration, however. Since this is a dependency change, this should pass all the existing CIs. Reviewers: Lee Dongjin <dongjin@apache.org>, Ismael Juma <ismael@juma.me.uk> * KAFKA-8940: decrease session timeout to make test faster and reliable (apache#10871) While there might still be some issue about the test as described here by @ableegoldman , but I found the reason why this test failed quite frequently recently. It's because we increased the session timeout to 45 sec in KIP-735. The reason why increasing session timeout affected this test is because in this test, we will keep adding new stream clients and remove old one, to maintain only 3 stream clients alive. The problem here is, when old stream closed, we won't trigger rebalance immediately due to the stream clients are all static members as described in KIP-345, which means, we will trigger trigger group rebalance only when session.timeout expired. That said, when old client closed, we'll have at least 45 sec with some tasks not working. Also, in this test, we have 2 timeout conditions to fail this test before verification passed: 1. 6 minutes timeout 2. polling 30 times (each with 5 seconds) without getting any data. (that is, 5 * 30 = 150 sec without consuming any data) For (1), in my test under 45 session timeout, we'll create 8 stream clients, which means, we'll have 5 clients got closed. And each closed client need 45 sec to trigger rebalance, so we'll have 45 * 5 = 225 sec (~4 mins) of the time having some tasks not working. For (2), during new client created and old client closed, it need some time to do rebalance. With 45 session timeout, we only got ~100 sec left. In slow jenkins env, it might reach the 30 retries without getting any data timeout. Therefore, decreasing session timeout can make this test completes faster and more reliable. Reviewers: Guozhang Wang <wangguoz@gmail.com> * MINOR: enable EOS during smoke test IT (apache#10870) This IT has been failing on trunk recently. Enabling EOS during the integration test makes it easier to be sure that the test's assumptions are really true during verification and should make the test more reliable. I also noticed that in the actual system test file, we are using the deprecated property name "beta" instead of "v2". Reviewers: Boyang Chen <boyang@apache.org> * MINOR: Log formatting for exceptions during configuration related operations (apache#10843) Format configuration logging during exceptions or errors. Also make sure it redacts sensitive information or unknown values. Reviewers: Luke Chen <showuon@gmail.com>, David Jacot <djacot@confluent.io> * KAFKA-12914: StreamSourceNode should return `null` topic name for pattern subscription (apache#10846) Reviewers: Luke Chen <showuon@gmail.com>, Bruno Cadonna <bruno@confluent.io>, Guozhang Wang <guozhang@confluent.io> * KAFKA-12948: Remove node from ClusterConnectionStates.connectingNodes when node is removed (apache#10882) NetworkClient.poll() throws IllegalStateException when checking isConnectionSetupTimeout if all nodes in ClusterConnectionStates.connectingNodes aren't present in ClusterConnectionStates.nodeState. This commit ensures that when we remove a node from nodeState, we also remove from connectingNodes. Reviewers: David Jacot <djacot@confluent.io> * KAFKA-12701: NPE in MetadataRequest when using topic IDs (apache#10584) We prevent handling MetadataRequests where the topic name is null (to prevent NPE) as well as prevent requests that set topic IDs since this functionality has not yet been implemented. When we do implement it in apache#9769, we should bump the request/response version. Added tests to ensure the error is thrown. Reviewers: dengziming <swzmdeng@163.com>, Ismael Juma <ismael@juma.me.uk> Co-authored-by: Josep Prat <josep.prat@aiven.io> Co-authored-by: Ismael Juma <ismael@juma.me.uk> Co-authored-by: David Christle <dchristle@users.noreply.github.com> Co-authored-by: Luke Chen <showuon@gmail.com> Co-authored-by: John Roesler <vvcephei@users.noreply.github.com> Co-authored-by: YiDing-Duke <dingyi.zj@gmail.com> Co-authored-by: Rajini Sivaram <rajinisivaram@googlemail.com> Co-authored-by: Justine Olshan <jolshan@confluent.io>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
4 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Remove unneeded
@SuppressWarnings("unchecked")in source and testRemove unneeded
@SuppressWarnings("deprecated")in source and testSeveral of those annotations were either never needed, or the code that force its introduction is already gone.
Committer Checklist (excluded from commit message)