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

fix tokio thread_name #1318

Merged
merged 3 commits into from
Aug 31, 2023
Merged

fix tokio thread_name #1318

merged 3 commits into from
Aug 31, 2023

Conversation

rukai
Copy link
Member

@rukai rukai commented Aug 31, 2023

pulled out of and continuing discussion from #1315 (comment)

After enabling "show custom thread names" in htop (its disabled by default) we get output like this:
image
It looks truncated because linux does not support thread names that long.

This PR fixes the issue by using a shorter thread name.
It now looks like this:
image

While there could theoretically be some obscure case out there, at our current scale I dont believe this will cause any issues due to the change of name.
Scripts that invoke things like killall shotover-proxy will be unaffected because the main thread is still called shotover-proxy.

@rukai rukai marked this pull request as ready for review August 31, 2023 06:30
Copy link
Member

@conorbros conorbros left a comment

Choose a reason for hiding this comment

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

Scripts that invoke things like killall shotover-proxy will be unaffected because the main thread is still called shotover-proxy.

I hadn't considered this when I raised that original point, thanks for pulling out though. LGTM.

Copy link
Collaborator

@cjrolo cjrolo left a comment

Choose a reason for hiding this comment

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

Approving, but relying on htop as the go-to tool to identify processes is not always the best. I mostly rely on ps for that if working on a server\server tools. For those tools I don't thing we would have limitations on naming size. But I do prefer the shorter name.

@rukai
Copy link
Member Author

rukai commented Aug 31, 2023

It is the Linux API that limits to 16 characters not htop https://man7.org/linux/man-pages/man3/pthread_setname_np.3.html

@rukai rukai enabled auto-merge (squash) August 31, 2023 10:07
@rukai rukai merged commit 8ca860d into shotover:main Aug 31, 2023
37 checks passed
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