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

Dynamically change the processor thread name when consuming event #359

Merged
merged 1 commit into from Aug 5, 2019

Conversation

i3wangyi
Copy link
Contributor

@i3wangyi i3wangyi commented Jul 23, 2019

Description
Here are some details about my PR, including screenshots of any UI changes:
(Write a concise description including what, why, how)

What
Change the thread name at run time for processing every event.

Why?
It tries to solve the pain point when developers need to grep the entire log history of a single event based on its event id. We do add the event id in some of our pipeline stages, but it's not convenient and incomplete.

Modifying the running thread at run time whenever an event is consumed is much more convenient.

Example:

Event id:  5f1e26a8_DEFAULT
Enqueue : 5f1e26a8_TASK
Enqueue : 3ddd0a91_DEFAULT
Enqueue : 3ddd0a91_TASK
Current thread name: HelixController-pipeline-default-TestCluster(5f1e26a8_DEFAULT)

Tests
N/A
Manually tested, verified in the above example. It doesn't change any functionality of Helix.

Code Quality
My diff has been formatted using helix-style.xml

@narendly
Copy link
Contributor

narendly commented Jul 24, 2019 via email

@lei-xia
Copy link
Member

lei-xia commented Jul 24, 2019

Please verify this in your local deployment to make sure it works as expected

@i3wangyi
Copy link
Contributor Author

Please verify this in your local deployment to make sure it works as expected

It's working as expected while running controller locally. What I did is to print the thread name, it's something like HelixController-pipeline-default-TestCluster(5f1e26a8_DEFAULT)

@i3wangyi
Copy link
Contributor Author

i3wangyi commented Aug 5, 2019

This PR is ready to be merged, approved by @lei-xia

@junkaixue junkaixue merged commit 7ac3123 into apache:master Aug 5, 2019
@i3wangyi i3wangyi deleted the rename branch November 7, 2019 00:38
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.

None yet

6 participants