-
Notifications
You must be signed in to change notification settings - Fork 9.1k
HDFS-17461. Fix spotbugs in PeerCache#getInternal #6721
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
Conversation
💔 -1 overall
This message was automatically generated. |
List<Value> sockStreamList = multimap.get(new Key(dnId, isDomain)); | ||
if (sockStreamList == null) { | ||
if (sockStreamList.isEmpty()) { | ||
return null; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We can just drop this if check itself, the below logic can safely handle an empty list
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks @ayushtkn for your comment.
Update PR, please help me review it again, thanks~
💔 -1 overall
This message was automatically generated. |
Still report spotbugs at latest. Try to trigger CI again, let's wait what it will say. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
💔 -1 overall
This message was automatically generated. |
Hi @ayushtkn , 'With the patch it shows fixed' where is this report link, hadoop-yetus comment here is not the correct one? Thanks. |
@Hexiaoqiao If you check the previous Jenkins result, it has two runs for the spotbugs, one without patch, just on trunk & then with patch. On trunk without patch it is red, because Spotbugs is broken without this patch, Since we fixed it in the current patch, it went green for the patch run |
@ayushtkn Got it. Thanks! |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM. +1.
Committed to trunk. Thanks @haiyang1987 , @ZanderXu and @ayushtkn ! |
Thanks @Hexiaoqiao @ayushtkn @ZanderXu for your review and merge it. |
…ributed by Haiyang Hu. Reviewed-by: ZanderXu <[email protected]> Reviewed-by: Ayush Saxena <[email protected]> Signed-off-by: He Xiaoqiao <[email protected]> (cherry picked from commit 6ccb223)
…ributed by Haiyang Hu. Reviewed-by: ZanderXu <[email protected]> Reviewed-by: Ayush Saxena <[email protected]> Signed-off-by: He Xiaoqiao <[email protected]>
Description of PR
https://issues.apache.org/jira/browse/HDFS-17461
Fix spotbugs in PeerCache#getInternal
Spotbugs warnings:
https://ci-hadoop.apache.org/job/hadoop-multibranch/job/PR-6710/4/artifact/out/branch-spotbugs-hadoop-hdfs-project_hadoop-hdfs-client-warnings.html
private final LinkedListMultimap<Key, Value> multimap = LinkedListMultimap.create();
Returns a collection view containing the values associated with key in this multimap, if any. Note that even when (containsKey(key) is false, get(key) still returns an empty collection, not null.