-
Notifications
You must be signed in to change notification settings - Fork 9.2k
HDFS-16947. RBF NamenodeHeartbeatService to report error for not being able to register namenode in state store #5470
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
…g able to register namenode in state store
|
@goiri @slfan1989 could you please review this PR? |
|
💔 -1 overall
This message was automatically generated. |
| } catch (IOException e) { | ||
| LOG.info("Cannot register namenode in the State Store"); | ||
| LOG.error("Cannot register namenode in the State Store", e); | ||
| } catch (Exception ex) { | ||
| LOG.error("Unhandled exception updating NN registration for {}", | ||
| getNamenodeDesc(), ex); |
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.
can we merge the both blocks, now both are logging error, with the later just having additional getNamenodeDesc
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.
Sounds good, so you mean having getNamenodeDesc in general for both is fine right? I think that's good idea
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.
Yep
|
💔 -1 overall
This message was automatically generated. |
ayushtkn
left a comment
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
…g able to register namenode in state store (apache#5470)
Namenode heartbeat service should provide error with full stacktrace if it cannot register namenode in the state store. As of today, we only log info msg.
For zookeeper based impl, this might mean either a) curator manager is not initialized or b) if it fails to write to znode after exhausting retries. For either of these cases, reporting only INFO log might not be good enough and we might have to look for errors elsewhere.
Sample example:
If we could log full stacktrace: