-
Notifications
You must be signed in to change notification settings - Fork 8.9k
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
HDFS-16377. Should CheckNotNull before access FsDatasetSpi #3784
Conversation
💔 -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.
+1 (non-binding). Makes sense to include this check for the remaining Datanode APIs that are triggered by DFSAdmin e.g. getVolumeReport(), refreshVolumes() etc. Thanks @tomscut
Thanks @virajjasani for your review. |
Hi @aajisaka @tasanuma @jojochuang @Hexiaoqiao , PTAL. Thanks. |
...-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/datanode/DataNode.java
Outdated
Show resolved
Hide resolved
💔 -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.
LGTM.
Thanks for your contribution, @tomscut, and thanks for your review, @virajjasani. |
Reviewed-by: Viraj Jasani <[email protected]> Signed-off-by: Takanobu Asanuma <[email protected]> (cherry picked from commit 22f5e18)
Reviewed-by: Viraj Jasani <[email protected]> Signed-off-by: Takanobu Asanuma <[email protected]> (cherry picked from commit 22f5e18)
Reviewed-by: Viraj Jasani <[email protected]> Signed-off-by: Takanobu Asanuma <[email protected]>
Reviewed-by: Viraj Jasani <[email protected]> Signed-off-by: Takanobu Asanuma <[email protected]> (cherry picked from commit 22f5e18)
Reviewed-by: Viraj Jasani <[email protected]> Signed-off-by: Takanobu Asanuma <[email protected]>
apache#3784) Reviewed-by: Viraj Jasani <[email protected]> Signed-off-by: Takanobu Asanuma <[email protected]> (cherry picked from commit 22f5e18) (cherry picked from commit 3257646) Change-Id: Icae290571be06f81cd15f82441201a201d350d9f
JIRA: HDFS-16377.
When starting the DN, we found NPE in the staring DN's log, as follows:
The logs of the upstream DN are as follows:
This is mainly because
FsDatasetSpi
has not been initialized at the time of access.I noticed that checkNotNull is already done in these two method(
DataNode#getBlockLocalPathInfo
andDataNode#getVolumeInfo
). So we should add it to other places(interfaces that clients and other DN can access directly) so that we can add a message(Storage not yet initialized
) when throwing exceptions.Therefore, the client and the upstream DN know that
FsDatasetSpi
has not been initialized, rather than blindly unaware of the specific cause of the NPE.