-
Notifications
You must be signed in to change notification settings - Fork 3.4k
HBASE-29857 better handling NPE in BucketCache #7685
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
base: master
Are you sure you want to change the base?
Conversation
|
🎊 +1 overall
This message was automatically generated. |
|
💔 -1 overall
This message was automatically generated. |
|
|
||
| // HBASE-29857: Handle case where persistence file is empty or corrupted. | ||
| // parseDelimitedFrom() returns null when there's no data to read. | ||
| if (cacheEntry == 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.
@teamconfx
I noticed one more potential NPE in BucketCache.retrieveFromFile e.g. when ProtobufMagic.isPBMagicPrefix(pbuf)=true.
So can we validation check in the caller, BucketCache.retrieveFromFile to check if the persistence file is empty or corrupted?
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 for pointing this out! Will do it.
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.
@vaijosh I added a similar validation check for BucketCache.retrieveFromFile, can you please take another look? Thanks!
|
💔 -1 overall
This message was automatically generated. |
|
Thanks @teamconfx |
This PR is for JIRA issue: https://issues.apache.org/jira/browse/HBASE-29857
The original JIRA issue is found in HBase v2.6.3, and fixed by HBASE-28839.
I tested with the same workload that I see the bug from v2.6.3 and verify the fix works in both master and branch-2.6.
However the current code still not handle NPE when persistence file is empty or corrupted explicitly.
This PR proposes a new checker to better catch the null from
BucketCacheProtos.BucketCacheEntry.parseDelimitedFrom(in);.