Updated spawner logging to improve debugging and traceability. #8
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
dodget
approved these changes
Mar 18, 2020
Comment on lines
+124
to
+127
| def log_user(self, message='', level=logging.INFO): | ||
| user = self.user.name if self.user else None | ||
| log_message = "[user:%s] %s" % (user, message) | ||
| self.log.log(level, log_message) |
| return None #its up! | ||
| else: | ||
| self.log.debug("Poll, notebook is not running for user %s" % self.user.name) | ||
| self.log_user("poll: notebook is NOT running") | ||
| return "server up, no instance running for user %s" % self.user.name |
There was a problem hiding this comment.
Hi @arthurian, I just noticed a possible typo here. It should probably say "Server up, no notebook running for user ..." instead of "Server up, no instance running for user...". This was there from before, not introduced by you. Just pointing it out.
There was a problem hiding this comment.
Yeah I think you're right about that.
joshuagetega
approved these changes
Mar 19, 2020
There was a problem hiding this comment.
Works great, @arthurian. I have successfully tested it on the miniconda sandbox cluster.
Changes include increasing the poll interval and increasing the number of attempts the is_notebook_running() function will be called with from within the poll() function. This is meant to reduce the chances of the poll() function wrongly determining that the jupyterhub-singleuser process is not running on a worker instance when it actually is.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This PR updates
spawner.py. The changes are related to logging and checking whether a notebook is running.When issues occur, it can be difficult to determine the cause since logging is a bit ad-hoc and has been added over time. The goal is to streamline the logging in such a way that it is easier to trace/debug spawner activity on a per user basis and make it possible to correlate the data with other metrics.
Streamlining logging
self.log_user()to the spawner, which automatically logs the user ID. By default, this will log anINFOmessage unless otherwise specified.poll()function.Checking if a worker notebook is running on the server:
This change is slightly speculative, but given that the manager must SSH into every worker instance to verify that jupyter is running, the
is_notebook_running()check should be as efficient as possible.Currently, we run
ps -ef | grep jupyterhub-singleuserwhich spawns 2 processes joined by a pipeline. That should work fine 99.999% of the time, but since we're seeing high loads when users are training their models, it might be worth switching tonice -5 pgrep -a -f jupyterhub-singleuser. This should be functionally equivalent, except that it executes one process with a slightly higher priority.@joshuagetega @dodget