- 
                Notifications
    You must be signed in to change notification settings 
- Fork 3.4k
HBASE-27199 Clean up error-prone findings in hbase-asyncfs #4620
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. | 
| 🎊 +1 overall 
 
 This message was automatically generated. | 
| 💔 -1 overall 
 
 This message was automatically generated. | 
| 🎊 +1 overall 
 
 This message was automatically generated. | 
| 🎊 +1 overall 
 
 This message was automatically generated. | 
| 🎊 +1 overall 
 
 This message was automatically generated. | 
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.
I think we could implement something like a FutureUtils.addListener and add a SuppressWarnings on that method.
And also, usually we do not care about the returned ChannelFuture when calling channel.write, we could also implement something like a logging listener to eat the future(and suppress the warning by using the above addListener method).
| sendSaslMessage(ctx, response, cipherOptions); | ||
| ChannelFuture sendFuture = sendSaslMessage(ctx, response, cipherOptions); | ||
| ctx.flush(); | ||
| sendFuture.get(); | 
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.
This is a no no, we should not do any blocking operations in netty handler...
| PTAL at #4628 ? Where I use spotless replaceRegex to cleanup the MissingSumamry warnings where we have only one  So the patch here could focus on other more important changes. | 
| And this is what I proposed on how to fix the FutureReturnValueIgnored. PTAL to see if it is suitable. Thanks. | 
No description provided.