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

Add support for assigning names to worker_threads #32434

Closed
cTn-dev opened this issue Mar 23, 2020 · 12 comments
Closed

Add support for assigning names to worker_threads #32434

cTn-dev opened this issue Mar 23, 2020 · 12 comments
Labels
feature request Issues that request new features to be added to Node.js. libuv Issues and PRs related to the libuv dependency or the uv binding. worker Issues and PRs related to Worker support.

Comments

@cTn-dev
Copy link

cTn-dev commented Mar 23, 2020

Is your feature request related to a problem? Please describe.

  1. Having the ability to easily name threads would improve/simplify looking at the resource usage while monitoring/debugging in htop (with "Show custom thread names") enabled.
  2. Commonly used process exporter https://github.com/ncabatoff/process-exporter could expose thread names for prometheus and other monitoring tools in human readable format

Describe the solution you'd like

  • accept name or customName in the options object in new Worker constructor
  • add .setName() or .setCustomName() to the worker class
  • the above format should be fully backwards compatible

Describe alternatives you've considered
Haven't considered any alternatives at the moment.
I am not sure if process.execArgv can be leveraged here to set a thread name as an alternative way of setting it.

@cTn-dev
Copy link
Author

cTn-dev commented Mar 23, 2020

@addaleax Do you think this would be possible?

@addaleax addaleax added libuv Issues and PRs related to the libuv dependency or the uv binding. worker Issues and PRs related to Worker support. labels Mar 23, 2020
@addaleax
Copy link
Member

I think the only concern here would be portability – Windows has SetThreadDescription()/GetThreadDescription(), but pthread_setname_np() and pthread_getname_np() are non-portable. I guess that means that libuv needs to be taught how to recognize a platform that supports these, and that we have to accept it being a no-op on platforms that don’t.

@addaleax addaleax added the feature request Issues that request new features to be added to Node.js. label Mar 23, 2020
@gireeshpunathil
Copy link
Member

@addaleax - couldn't we keep the name in the Worker object level only, rather than attaching it to the native thread?

@addaleax
Copy link
Member

@gireeshpunathil You can already assign any properties to the Worker object that you want. That doesn’t seem to make sense to me in the use case described here, though.

@bnoordhuis
Copy link
Member

Adding support for thread naming to libuv isn't complicated but some thought has to be given w.r.t. #30061 (comment) - the current 1-to-1 correspondence between threads and workers need not be true forever.

I think all currently platforms let you set the thread name more than once, making it trivial to fix, but e.g. Fuchsia support would be problematic, because it only lets you set the name at thread creation time (AFAIK anyway - LMK if there is a way around that.)

@cTn-dev
Copy link
Author

cTn-dev commented May 19, 2020

@addaleax While the custom thread name functionality is not supported, can you recommend a workaround for measuring the worker thread utilization on the application level?
I am only familiar with the process.cpuUsage which is for the whole process.

@addaleax
Copy link
Member

@cTn-dev You could write an addon that does that for Linux if you want, but there’s no cross-platform way to get that information, unfortunately. :/

@mmomtchev
Copy link
Contributor

I am willing to take this, but this is in fact a mostly libuv project
Currently setting the process title uses a combination of pthread_set_name and prctl on Linux where you need to use both to have the same name in /proc/<pid>/comm and /proc/<pid>/cmdline
So one will have to take the existing uv_set_process_title() and extract a uv_set_thread_title() from it for every supported OS
see #35503

@jasnell
Copy link
Member

jasnell commented Oct 13, 2020

@cTn-dev ... Worker thread utilization with Node.js is a bit difficult. I would have to ask exactly what you're looking for. @trevnorris just recently landed a new metric capability in Node.js called "event look utilization" that measures how efficiently the event loop is being used. There are separate metrics available for the main event loop thread and each individual worker and Trevor's research has demonstrated it to be a solid replacement for CPU analysis -- even better in that there's zero noise from the entire process or other CPU tasks to filter out.

As both @addaleax and @bnoordhuis have pointed out, there's likely going to be some difficulty in consistently implementing the ability to name threads and it's not clear what the utility of doing so is really going to be, so better understanding the use case would be helpful.

@cTn-dev
Copy link
Author

cTn-dev commented Oct 13, 2020

@mmomtchev @jasnell Thank you for looking into this ticket again.

I have been indeed keeping an eye on the #34938 (haven't had time to try it yet, i will make sure to do so when v14 goes LTS).
From the instrumentation point of view i think its a viable alternative for "what i initially needed to do" (as the above functionality wasn't available before).

The exact use case i was trying to tackle was long term monitoring of process & thread utilization when the utilization of worker threads is high or high amount of them is being used in a single process.
(I was tracking down a bottleneck that only manifested in production/couldn't repro locally, the inability to see which of the worker threads is being maxed out made the investigation much more time consuming).
While the process cpu usage clearly indicated that the cpu usage is for example 300%+, there was no "easy" way to determinate which of the worker threads are running "hot/cold" (beside seeing the threadId reporting utilization of X%).

The end goal in my case was that named threads would provide quick and easy way of identifying worker threads that are heavily utilized/under utilized without additional instrumentation on the application level.
This would also likely help people get deeper insight into how cpu time is being spent on each thread inside the process (libuv thread pool, threads used for GC, worker threads, etc).

@mmomtchev
Copy link
Contributor

@cTn-dev a quick and dirty solution will be to install one of the several syscall packages from NPM and then call pthread_setname_np

@jasnell
Copy link
Member

jasnell commented May 12, 2021

Just revisiting this... for all the reasons already given, it's unlikely that there will be a reliable way of assigning a thread name other than by attaching a property to the actual Worker object. Given that the use case is in monitoring the activity, having a centralize way of knowing that a thread has been made created and attaching any additional instrumentation to it is helpful. See #38659

I'm going to recommend that we close this issue, however. If something gets added to libuv down the road we can revisit, but for now, using a userland module is likely the best option.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature request Issues that request new features to be added to Node.js. libuv Issues and PRs related to the libuv dependency or the uv binding. worker Issues and PRs related to Worker support.
Projects
None yet
Development

No branches or pull requests

6 participants