Skip to content
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

set thread names on startup #21

Merged
merged 6 commits into from
Aug 21, 2016
Merged

set thread names on startup #21

merged 6 commits into from
Aug 21, 2016

Conversation

binary1230
Copy link
Contributor

  • this code monkeypatches the start of each thread to call prctl() which sets the thread name
  • this allows external utilities, like top and ps, to show thread names
  • this is great because if a particular thread is going off the rails, we can now see which one using external tools
  • requires build-package and libcap to be installed on the system

requires magfest-archive/ubersystem-puppet#110

- this code monkeypatches the start of each thread to call prctl() which sets the thread name
- this allows external utilities, like top and ps, to show thread names
- this is great because if a particular thread is going off the rails, we can now see which one using external tools
- requires build-package and libcap to be installed on the system
@binary1230
Copy link
Contributor Author

when applied, it looks like this from htop:

image

@binary1230
Copy link
Contributor Author

there's also a long-running python bug to fix this in CPython runtime https://bugs.python.org/issue15500 but hasn't been touched since 2012.

@binary1230
Copy link
Contributor Author

fixes magfest/ubersystem#1999

@EliAndrewC
Copy link
Contributor

This looks awesome!

The reason why Travis is failing right now is that the library you're using is Python 3 only and Sideboard runs on both Python 2 and 3. Since this feature is optional, the easiest thing to do is a little version checking to only turn it on if we're running on Python 3. I'll go ahead and make that change now and push a fix in a few minutes.

@EliAndrewC
Copy link
Contributor

Ok, I've updated this with the changes I mentioned.

@EliAndrewC
Copy link
Contributor

Hmm, still failing due to Travis not having the correct headers installed. I did a quick skim through https://docs.travis-ci.com/user/languages/python but didn't see at a glance how to tell it to install that package, though it does indicate that we probably can install Ubuntu packages.

@EliAndrewC
Copy link
Contributor

If that ended up not working, we could always put an extra clause to our if statement in setup.py to check whether we're on Travis and not try to install the dependency in that case.

@binary1230
Copy link
Contributor Author

Oh wait I remember, that internal thread function I'm overriding was
renamed in python 3, so for 2 to work we need to use the older name.
Instead of _bootstrap it was _Thread_bootstrap or something like that.

On Aug 21, 2016 1:46 AM, "Eli Courtwright" notifications@github.com wrote:

If that ended up not working, we could always put an extra clause to our if
statement in setup.py to check whether we're on Travis and not try to
install the dependency in that case.


You are receiving this because you authored the thread.
Reply to this email directly, view it on GitHub
#21 (comment), or mute
the thread
https://github.com/notifications/unsubscribe-auth/AFKYyPuRlKRC2z8JDFVmk2FVmtH-NFmgks5qh-YZgaJpZM4JpFYy
.

- required for use in prctl module calls (thread naming setting)
@binary1230
Copy link
Contributor Author

hey, i'm pretty sure (though not certain) that prctl is a python2 library as well, will look into it real quick

@binary1230
Copy link
Contributor Author

confirmed, python-prctl does indeed work in python 2.7.

@binary1230
Copy link
Contributor Author

I know that thread function will not work without the 2.7 stuff though, so will fix that up to make it work and remove the safeguards around the import stuff

@binary1230
Copy link
Contributor Author

ok I just pushed some new code which makes this work regardless of python2 or python3.

I tested this with a local install of just sideboard running under python2, so it definitely is working OK. this is ready for merge as far as I can tell. @EliAndrewC could you give this one more look, I pulled out the try/except import stuff and the setup.py stuff in favor of just installing python-prctl for both python2 and 3


# set the name of the main thread
prctl.set_name('sideboard_main')

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

notes:
in python3, the inner function is named threading.Thread._bootstrap_inner
in python2, the inner function is named threading.Thread._Thread__bootstrap

@binary1230
Copy link
Contributor Author

so TLDR things changed:

  • verified python-prctl is a python2 and python 3 package
  • told travis to apt-get install libcap-devel which makes python-prctl installable, which also fixes the broken build for python2
  • patch the correct internal thread function based on python2 vs python3
  • tested this stuff on both python2 and python3

@EliAndrewC
Copy link
Contributor

+1 looks great!

@EliAndrewC EliAndrewC merged commit 9493061 into master Aug 21, 2016
@EliAndrewC EliAndrewC deleted the name_threads branch August 21, 2016 21:34
@@ -1,6 +1,9 @@
from __future__ import unicode_literals
import time
import heapq
import prctl
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yo, this has broken my ability to run unit tests:

Traceback (most recent call last):
  File "/home/vagrant/uber/sideboard/env/lib/python3.4/site-packages/_pytest/config.py", line 513, in getconftestmodules
    return self._path2confmods[path]
KeyError: local('/home/vagrant/uber/sideboard/plugins/uber/uber/tests')

During handling of the above exception, another exception occurred:
Traceback (most recent call last):
  File "/home/vagrant/uber/sideboard/env/lib/python3.4/site-packages/_pytest/config.py", line 537, in importconftest
    return self._conftestpath2mod[conftestpath]
KeyError: local('/home/vagrant/uber/sideboard/plugins/uber/conftest.py')

During handling of the above exception, another exception occurred:
Traceback (most recent call last):
  File "/home/vagrant/uber/sideboard/sideboard/__init__.py", line 9, in <module>
    import sideboard.server
  File "/home/vagrant/uber/sideboard/sideboard/server.py", line 10, in <module>
    from sideboard.internal import connection_checker
  File "/home/vagrant/uber/sideboard/sideboard/internal/connection_checker.py", line 7, in <module>
    from sideboard.lib import services, entry_point
  File "/home/vagrant/uber/sideboard/sideboard/lib/__init__.py", line 13, in <module>
    from sideboard.lib._threads import DaemonTask, Caller, GenericCaller, TimeDelayQueue
  File "/home/vagrant/uber/sideboard/sideboard/lib/_threads.py", line 4, in <module>
    import prctl
ImportError: No module named 'prctl'

During handling of the above exception, another exception occurred:
Traceback (most recent call last):
  File "/home/vagrant/uber/sideboard/env/lib/python3.4/site-packages/_pytest/config.py", line 543, in importconftest
    mod = conftestpath.pyimport()
  File "/home/vagrant/uber/sideboard/env/lib/python3.4/site-packages/py/_path/local.py", line 650, in pyimport
    __import__(modname)
  File "/home/vagrant/uber/sideboard/plugins/uber/conftest.py", line 1, in <module>
    import sideboard
  File "/home/vagrant/uber/sideboard/sideboard/__init__.py", line 11, in <module>
    from sideboard.lib import log
  File "/home/vagrant/uber/sideboard/sideboard/lib/__init__.py", line 13, in <module>
    from sideboard.lib._threads import DaemonTask, Caller, GenericCaller, TimeDelayQueue
  File "/home/vagrant/uber/sideboard/sideboard/lib/_threads.py", line 4, in <module>
    import prctl
ImportError: No module named 'prctl'
ERROR: could not load /home/vagrant/uber/sideboard/plugins/uber/conftest.py

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

From Dom in Slack, this is because the module isn't installed and re-running puppet (cd ~/uber/puppet && ./apply_node.sh localhost) will make it work.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants