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/10/12 22:23:37 UTC
[GitHub] [kafka] guozhangwang opened a new pull request #9416: MINOR: more log4j entry on elect / resignation of coordinators
guozhangwang opened a new pull request #9416:
URL: https://github.com/apache/kafka/pull/9416
When a coordinator module is being elected / resigned, our log entry is usually associated with a background scheduler on loading / unloading entries and hence it is unclear at the exact time when the election or resignation happens, and we have to then compare with the KafkaAPI's log entry for leaderAndISR / StopReplica to infer the actual time. I think add a couple new log entries indicating the exact time when it happens is helpful.
### 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] guozhangwang commented on pull request #9416: MINOR: more log4j entry on elect / resignation of coordinators
Posted by GitBox <gi...@apache.org>.
guozhangwang commented on pull request #9416:
URL: https://github.com/apache/kafka/pull/9416#issuecomment-707375065
cc @abbccdda @cadonna for reviews.
----------------------------------------------------------------
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 #9416: MINOR: more log4j entry on elect / resignation of coordinators
Posted by GitBox <gi...@apache.org>.
cadonna commented on a change in pull request #9416:
URL: https://github.com/apache/kafka/pull/9416#discussion_r503732448
##########
File path: core/src/main/scala/kafka/coordinator/group/GroupCoordinator.scala
##########
@@ -917,6 +918,7 @@ class GroupCoordinator(val brokerId: Int,
* @param offsetTopicPartitionId The partition we are no longer leading
*/
def onResignation(offsetTopicPartitionId: Int): Unit = {
+ info(s"Resigning the group coordinator for partition $offsetTopicPartitionId")
Review comment:
Shouldn't this be `Resigning as the group coordinator` or `Resigning as group coordinator`?
##########
File path: core/src/main/scala/kafka/coordinator/transaction/TransactionCoordinator.scala
##########
@@ -329,6 +330,7 @@ class TransactionCoordinator(brokerId: Int,
* are resigning after receiving a StopReplica request from the controller
*/
def onResignation(txnTopicPartitionId: Int, coordinatorEpoch: Option[Int]): Unit = {
+ info(s"Resigning the txn coordinator for partition $txnTopicPartitionId at epoch $coordinatorEpoch")
Review comment:
Same as above.
##########
File path: core/src/main/scala/kafka/coordinator/group/GroupCoordinator.scala
##########
@@ -908,6 +908,7 @@ class GroupCoordinator(val brokerId: Int,
* @param offsetTopicPartitionId The partition we are now leading
*/
def onElection(offsetTopicPartitionId: Int): Unit = {
+ info(s"Becoming the group coordinator for partition $offsetTopicPartitionId")
Review comment:
It would also be nice to have the groups that are coordinated in the log message. I guess that is hardly possible, isn't it?
----------------------------------------------------------------
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 #9416: MINOR: more log4j entry on elect / resignation of coordinators
Posted by GitBox <gi...@apache.org>.
guozhangwang commented on a change in pull request #9416:
URL: https://github.com/apache/kafka/pull/9416#discussion_r505170871
##########
File path: core/src/main/scala/kafka/coordinator/group/GroupCoordinator.scala
##########
@@ -908,6 +908,7 @@ class GroupCoordinator(val brokerId: Int,
* @param offsetTopicPartitionId The partition we are now leading
*/
def onElection(offsetTopicPartitionId: Int): Unit = {
+ info(s"Becoming the group coordinator for partition $offsetTopicPartitionId")
Review comment:
But during the loading process, we do have log entry as
```
Loading / Unloading group metadata for ...
```
per group.
----------------------------------------------------------------
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 #9416: MINOR: more log4j entry on elect / resignation of coordinators
Posted by GitBox <gi...@apache.org>.
guozhangwang commented on a change in pull request #9416:
URL: https://github.com/apache/kafka/pull/9416#discussion_r505164292
##########
File path: core/src/main/scala/kafka/coordinator/group/GroupCoordinator.scala
##########
@@ -908,6 +908,7 @@ class GroupCoordinator(val brokerId: Int,
* @param offsetTopicPartitionId The partition we are now leading
*/
def onElection(offsetTopicPartitionId: Int): Unit = {
+ info(s"Becoming the group coordinator for partition $offsetTopicPartitionId")
Review comment:
Yeah that cannot be known until loading is completed.
----------------------------------------------------------------
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 #9416: MINOR: more log4j entry on elect / resignation of coordinators
Posted by GitBox <gi...@apache.org>.
guozhangwang merged pull request #9416:
URL: https://github.com/apache/kafka/pull/9416
----------------------------------------------------------------
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 #9416: MINOR: more log4j entry on elect / resignation of coordinators
Posted by GitBox <gi...@apache.org>.
cadonna commented on a change in pull request #9416:
URL: https://github.com/apache/kafka/pull/9416#discussion_r505588367
##########
File path: core/src/main/scala/kafka/coordinator/group/GroupCoordinator.scala
##########
@@ -908,6 +908,7 @@ class GroupCoordinator(val brokerId: Int,
* @param offsetTopicPartitionId The partition we are now leading
*/
def onElection(offsetTopicPartitionId: Int): Unit = {
+ info(s"Becoming the group coordinator for partition $offsetTopicPartitionId")
Review comment:
That is correct. Thank you for reminding me!
----------------------------------------------------------------
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 #9416: MINOR: more log4j entry on elect / resignation of coordinators
Posted by GitBox <gi...@apache.org>.
cadonna commented on a change in pull request #9416:
URL: https://github.com/apache/kafka/pull/9416#discussion_r505572558
##########
File path: core/src/main/scala/kafka/coordinator/group/GroupCoordinator.scala
##########
@@ -908,6 +908,7 @@ class GroupCoordinator(val brokerId: Int,
* @param offsetTopicPartitionId The partition we are now leading
*/
def onElection(offsetTopicPartitionId: Int): Unit = {
+ info(s"Electing as the group coordinator for partition $offsetTopicPartitionId")
Review comment:
Shouldn't that read `Elected as the group coordinator ...`?
##########
File path: core/src/main/scala/kafka/coordinator/transaction/TransactionCoordinator.scala
##########
@@ -311,6 +311,7 @@ class TransactionCoordinator(brokerId: Int,
* @param coordinatorEpoch The partition coordinator (or leader) epoch from the received LeaderAndIsr request
*/
def onElection(txnTopicPartitionId: Int, coordinatorEpoch: Int): Unit = {
+ info(s"Becoming the txn coordinator for partition $txnTopicPartitionId at epoch $coordinatorEpoch")
Review comment:
Do you also plan to change this to `Elected as ...`?
##########
File path: core/src/main/scala/kafka/coordinator/transaction/TransactionCoordinator.scala
##########
@@ -329,6 +330,7 @@ class TransactionCoordinator(brokerId: Int,
* are resigning after receiving a StopReplica request from the controller
*/
def onResignation(txnTopicPartitionId: Int, coordinatorEpoch: Option[Int]): Unit = {
+ info(s"Resigning the txn coordinator for partition $txnTopicPartitionId at epoch $coordinatorEpoch")
Review comment:
This still needs to be changed.
----------------------------------------------------------------
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