You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@helix.apache.org by GitBox <gi...@apache.org> on 2019/07/24 06:23:44 UTC
[GitHub] [helix] narendly commented on issue #359: Dynamically change the
processor thread name when consuming event
narendly commented on issue #359: Dynamically change the processor thread name when consuming event
URL: https://github.com/apache/helix/pull/359#issuecomment-514497403
Could we format the PR per the guidelines? :)
On Jul 24, 2019, 07:17, at 07:17, jiajunwang <no...@github.com> wrote:
>jiajunwang commented on this pull request.
>
>
>
>> @@ -1165,7 +1164,11 @@ public void run() {
> + _processorName);
> while (!isInterrupted()) {
> try {
>- handleEvent(_eventBlockingQueue.take(), _cache);
>+ ClusterEvent newClusterEvent = _eventBlockingQueue.take();
>+ // rename the current processor's thread name to include
>event id
>+ String currentThreadName = this.getName().split("_")[0];
>
>The thread name shall be fixed, You can just regenerate the name
>according to our name convention.
>
>> @@ -1165,7 +1164,11 @@ public void run() {
> + _processorName);
> while (!isInterrupted()) {
> try {
>- handleEvent(_eventBlockingQueue.take(), _cache);
>+ ClusterEvent newClusterEvent = _eventBlockingQueue.take();
>+ // rename the current processor's thread name to include
>event id
>+ String currentThreadName = this.getName().split("_")[0];
>+ this.setName(currentThreadName + "_" +
>newClusterEvent.getEventId());
>
>Can we use "()" to wrap the event? I feel it's easier to identify which
>part is the default thread name, and which part is the event name.
>
>> @@ -1165,7 +1164,11 @@ public void run() {
> + _processorName);
> while (!isInterrupted()) {
> try {
>- handleEvent(_eventBlockingQueue.take(), _cache);
>+ ClusterEvent newClusterEvent = _eventBlockingQueue.take();
>+ // rename the current processor's thread name to include
>event id
>+ String currentThreadName = this.getName().split("_")[0];
>+ this.setName(currentThreadName + "_" +
>newClusterEvent.getEventId());
>+ handleEvent(newClusterEvent, _cache);
>
>Would it be better to set the event id inside handleEvent()? Logic will
>be more concise.
>And remember that we'd better recover the thread name after the
>handling logic. Probably in some final block.
>
>--
>You are receiving this because you are subscribed to this thread.
>Reply to this email directly or view it on GitHub:
>https://github.com/apache/helix/pull/359#pullrequestreview-265794277
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services