You are viewing a plain text version of this content. The canonical link for it is here.
Posted to jira@kafka.apache.org by GitBox <gi...@apache.org> on 2020/06/15 12:25:12 UTC
[GitHub] [kafka] cadonna opened a new pull request #8872: Fix log message for transition from standby to active
cadonna opened a new pull request #8872:
URL: https://github.com/apache/kafka/pull/8872
Without this fix the log message is
```
task [0_2] Transitioning state manager for ACTIVE task 0_2 to ACTIVE
```
but it should probably be
```
task [0_2] Transitioning state manager for STANDBY task 0_2 to ACTIVE
```
### Committer Checklist (excluded from commit message)
- [ ] Verify design and implementation
- [ ] Verify test coverage and CI build status
- [ ] Verify documentation (including upgrade notes)
----------------------------------------------------------------
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
[GitHub] [kafka] cadonna commented on pull request #8872: Fix log message for transition from standby to active
Posted by GitBox <gi...@apache.org>.
cadonna commented on pull request #8872:
URL: https://github.com/apache/kafka/pull/8872#issuecomment-644104290
Call for review: @ableegoldman @guozhangwang
----------------------------------------------------------------
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
[GitHub] [kafka] guozhangwang commented on a change in pull request #8872: Fix log message for transition from standby to active
Posted by GitBox <gi...@apache.org>.
guozhangwang commented on a change in pull request #8872:
URL: https://github.com/apache/kafka/pull/8872#discussion_r440348339
##########
File path: streams/src/main/java/org/apache/kafka/streams/processor/internals/ProcessorStateManager.java
##########
@@ -509,11 +509,12 @@ void transitionTaskType(final TaskType newType, final LogContext logContext) {
throw new IllegalStateException("Tried to recycle state for task type conversion but new type was the same.");
}
+ TaskType oldType = taskType;
taskType = newType;
log = logContext.logger(ProcessorStateManager.class);
logPrefix = logContext.logPrefix();
- log.debug("Transitioning state manager for {} task {} to {}", taskType, taskId, newType);
+ log.debug("Transitioning state manager for {} task {} to {}", oldType, taskId, newType);
Review comment:
I think using oldType/newType is fine here since then we do not need to keep in mind of the ordering of each call?
----------------------------------------------------------------
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
[GitHub] [kafka] guozhangwang merged pull request #8872: Fix log message for transition from standby to active
Posted by GitBox <gi...@apache.org>.
guozhangwang merged pull request #8872:
URL: https://github.com/apache/kafka/pull/8872
----------------------------------------------------------------
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
[GitHub] [kafka] cadonna commented on a change in pull request #8872: Fix log message for transition from standby to active
Posted by GitBox <gi...@apache.org>.
cadonna commented on a change in pull request #8872:
URL: https://github.com/apache/kafka/pull/8872#discussion_r440139800
##########
File path: streams/src/main/java/org/apache/kafka/streams/processor/internals/ProcessorStateManager.java
##########
@@ -509,11 +509,12 @@ void transitionTaskType(final TaskType newType, final LogContext logContext) {
throw new IllegalStateException("Tried to recycle state for task type conversion but new type was the same.");
}
+ TaskType oldType = taskType;
taskType = newType;
log = logContext.logger(ProcessorStateManager.class);
logPrefix = logContext.logPrefix();
- log.debug("Transitioning state manager for {} task {} to {}", taskType, taskId, newType);
+ log.debug("Transitioning state manager for {} task {} to {}", oldType, taskId, newType);
Review comment:
Another alternative for the log message would be
```
standby-task [0_2] Transitioning state manager for STANDBY task 0_2 to ACTIVE
```
----------------------------------------------------------------
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
[GitHub] [kafka] guozhangwang commented on pull request #8872: Fix log message for transition from standby to active
Posted by GitBox <gi...@apache.org>.
guozhangwang commented on pull request #8872:
URL: https://github.com/apache/kafka/pull/8872#issuecomment-644281735
Cherry-picked to 2.6.
----------------------------------------------------------------
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
[GitHub] [kafka] ableegoldman commented on a change in pull request #8872: Fix log message for transition from standby to active
Posted by GitBox <gi...@apache.org>.
ableegoldman commented on a change in pull request #8872:
URL: https://github.com/apache/kafka/pull/8872#discussion_r440299033
##########
File path: streams/src/main/java/org/apache/kafka/streams/processor/internals/ProcessorStateManager.java
##########
@@ -509,11 +509,12 @@ void transitionTaskType(final TaskType newType, final LogContext logContext) {
throw new IllegalStateException("Tried to recycle state for task type conversion but new type was the same.");
}
+ TaskType oldType = taskType;
taskType = newType;
log = logContext.logger(ProcessorStateManager.class);
logPrefix = logContext.logPrefix();
- log.debug("Transitioning state manager for {} task {} to {}", taskType, taskId, newType);
+ log.debug("Transitioning state manager for {} task {} to {}", oldType, taskId, newType);
Review comment:
I agree it would be useful to prefix the logs with `standby` or `active`, but I'd prefer to do that everywhere in a separate PR.
Can we just move the log message to before we reassign `taskType = newType`? Or do you think we might forget/not notice that the ordering is relevant and accidentally move it back at some future time?
----------------------------------------------------------------
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
[GitHub] [kafka] cadonna commented on a change in pull request #8872: Fix log message for transition from standby to active
Posted by GitBox <gi...@apache.org>.
cadonna commented on a change in pull request #8872:
URL: https://github.com/apache/kafka/pull/8872#discussion_r440139800
##########
File path: streams/src/main/java/org/apache/kafka/streams/processor/internals/ProcessorStateManager.java
##########
@@ -509,11 +509,12 @@ void transitionTaskType(final TaskType newType, final LogContext logContext) {
throw new IllegalStateException("Tried to recycle state for task type conversion but new type was the same.");
}
+ TaskType oldType = taskType;
taskType = newType;
log = logContext.logger(ProcessorStateManager.class);
logPrefix = logContext.logPrefix();
- log.debug("Transitioning state manager for {} task {} to {}", taskType, taskId, newType);
+ log.debug("Transitioning state manager for {} task {} to {}", oldType, taskId, newType);
Review comment:
Another alternative for the log message would be
```
standby-task [0_2] Transitioning state manager for STANDBY task 0_2 to ACTIVE
```
That is, the log prefix would change from `task` to `standby-task`.
----------------------------------------------------------------
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