-
Notifications
You must be signed in to change notification settings - Fork 9.1k
HDFS-16350. Datanode start time should be set after RPC server starts successfully #3711
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
Thanks @virajjasani for reminding me here. At present, both |
Thanks @tomscut. Yes this is a minor improvement in the behaviour. Basically, we should set the time according to the actual server startup time. The point is by the time RPC servers are started, Datanode is not considered started so we should keep the start time accordingly, but as I mentioned, it's minor improvement but the timing should be more accurate I believe.
In that case, they might also require change. |
Starttime calculation for Namenode and Router seem better than Datanode case, because it's not the main classes that have starttime defined (i.e. NameNode and DFSRouter) but rather other classes that are internally initialized (e.g. FSNamesystem and Router) as part of the main process initialization, have the starttime defined. In the case of Datanode, the main class itself (i.e. DataNode) has starttime defined but it is initialized soon as the Datanode is instantiated. Hence, I think we should only improve Datanode's starttime to match with the actual process start time. I understand that for a healthy process initialization, there should not be much difference in starttime if we keep it at instance init level (without this patch) vs if we keep it after RPC server is started (with this patch), but my concern is that if for some reason, RPC handler start time is significantly delayed, then we would have incorrect starttime reported. |
🎊 +1 overall
This message was automatically generated. |
@ferhui could you please also take a look? 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
@virajjasani Thanks for contribution. @tomscut Thanks for review! |
Description of PR
We set start time of Datanode when the class is instantiated but it should be ideally set only after RPC server starts and RPC handlers are initialized to serve client requests.
How was this patch tested?
Tested locally. UI screenshot:
For code changes: