You are viewing a plain text version of this content. The canonical link for it is here.
Posted to jira@kafka.apache.org by "m1a2st (via GitHub)" <gi...@apache.org> on 2025/12/01 13:15:08 UTC
[PR] KAFKA-19940 Reduce the log4j2 cpu usage in PersisterStateManager [kafka]
m1a2st opened a new pull request, #21027:
URL: https://github.com/apache/kafka/pull/21027
FYI: https://github.com/apache/kafka/pull/20175/files#r2572693459
--
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.
To unsubscribe, e-mail: jira-unsubscribe@kafka.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] KAFKA-19940 Reduce the log4j2 cpu usage in PersisterStateManager [kafka]
Posted by "smjn (via GitHub)" <gi...@apache.org>.
smjn commented on PR #21027:
URL: https://github.com/apache/kafka/pull/21027#issuecomment-3617250145
Hi @m1a2st ,
Thanks for the PR. While the code is valid, it would mutate the logger key for the internal handlers to
```
org.apache.kafka.server.share.persister.PersisterStateManager$WriteStateHandler
```
This would make it infeasible to turn on DEBUG, TRACE logging for these classes. For example on a local kafka cluster, these fail:
1. ```
bin/kafka-configs.sh --bootstrap-server localhost:9092 --alter --add-config "org.apache.kafka.server.share.persister.PersisterStateManager$WriteStateHandler=DEBUG" --entity-type broker-loggers --entity-name 1
Invalid broker logger(s): org.apache.kafka.server.share.persister.PersisterStateManager
```
2. ```
bin/kafka-configs.sh --bootstrap-server localhost:9092 --alter --add-config "org.apache.kafka.server.share.persister.PersisterStateManager\$WriteStateHandler=DEBUG" --entity-type broker-loggers --entity-name 1
Invalid character found for config key: org.apache.kafka.server.share.persister.PersisterStateManager$WriteStateHandler
```
This was the reason why original code used `canonical` name, where possible.
--
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.
To unsubscribe, e-mail: jira-unsubscribe@kafka.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] KAFKA-19940 Reduce the log4j2 cpu usage in PersisterStateManager [kafka]
Posted by "m1a2st (via GitHub)" <gi...@apache.org>.
m1a2st commented on PR #21027:
URL: https://github.com/apache/kafka/pull/21027#issuecomment-3619591946
Thanks for @smjn comment, I have updated to use the `getCanonicalName()`, and I did the following step for testing
1. Create a local Kafka cluster.
2. Produce and Consumer some messge
```
bin/kafka-console-producer.sh --topic test --bootstrap-server localhost:9092
>4
>5
>6
./bin/kafka-console-share-consumer.sh --bootstrap-server localhost:9092 --topic test --group test
4
5
6
```
3. Alfer the log level
```
bin/kafka-figs.sh --bootstrap-server localhost:9092 --alter --add-config "org.apache.kafka.server.share.persister.PersisterStateManager.WriteStateHandler=DEBUG" --entity-type broker-loggers --entity-name 1
Completed updating config for broker-logger 1.
```
--
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.
To unsubscribe, e-mail: jira-unsubscribe@kafka.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] KAFKA-19940 Reduce the log4j2 cpu usage in PersisterStateManager [kafka]
Posted by "m1a2st (via GitHub)" <gi...@apache.org>.
m1a2st commented on code in PR #21027:
URL: https://github.com/apache/kafka/pull/21027#discussion_r2610310543
##########
server-common/src/main/java/org/apache/kafka/server/share/persister/DefaultStatePersister.java:
##########
@@ -44,6 +44,21 @@ public class DefaultStatePersister implements Persister {
private final PersisterStateManager stateManager;
private static final Logger log = LoggerFactory.getLogger(DefaultStatePersister.class);
+ private static final Logger INITIALIZE_HANDLER_LOG = LoggerFactory.getLogger(
Review Comment:
Using abstract method is better approach, I have addressed 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.
To unsubscribe, e-mail: jira-unsubscribe@kafka.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] KAFKA-19940 Reduce the log4j2 cpu usage in PersisterStateManager [kafka]
Posted by "chia7712 (via GitHub)" <gi...@apache.org>.
chia7712 commented on code in PR #21027:
URL: https://github.com/apache/kafka/pull/21027#discussion_r2609517453
##########
server-common/src/main/java/org/apache/kafka/server/share/persister/DefaultStatePersister.java:
##########
@@ -44,6 +44,21 @@ public class DefaultStatePersister implements Persister {
private final PersisterStateManager stateManager;
private static final Logger log = LoggerFactory.getLogger(DefaultStatePersister.class);
+ private static final Logger INITIALIZE_HANDLER_LOG = LoggerFactory.getLogger(
Review Comment:
Another option is to introduce an abstract `logger` method within `PersisterStateManagerHandler`. This ensures that each subclass provides its owe dedicated logger, enabling us to initialize the logger within each class rather than grouping them in a single place
--
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.
To unsubscribe, e-mail: jira-unsubscribe@kafka.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] KAFKA-19940 Reduce the log4j2 cpu usage in PersisterStateManager [kafka]
Posted by "chia7712 (via GitHub)" <gi...@apache.org>.
chia7712 commented on PR #21027:
URL: https://github.com/apache/kafka/pull/21027#issuecomment-3635203634
> This was the reason why original code used canonical name, where possible.
I think that is a bug in `ConfigCommand`. Many log names include the `$` symbol, and that is completely legal when using `Admin#incrementalAlterConfigs`
It seems to me using `LoggerFactory.getLogger(DefaultStatePersister.class)` is good and simple, and then we could fix `ConfigCommand` in https://issues.apache.org/jira/browse/KAFKA-19980
--
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.
To unsubscribe, e-mail: jira-unsubscribe@kafka.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] KAFKA-19940 Reduce the log4j2 cpu usage in PersisterStateManager [kafka]
Posted by "chia7712 (via GitHub)" <gi...@apache.org>.
chia7712 commented on code in PR #21027:
URL: https://github.com/apache/kafka/pull/21027#discussion_r2616064335
##########
server-common/src/main/java/org/apache/kafka/server/share/persister/PersisterStateManager.java:
##########
@@ -494,6 +490,9 @@ protected void setOnCompleteCallback(Consumer<ClientResponse> callback) {
}
public class InitializeStateHandler extends PersisterStateManagerHandler {
+
+ private static final Logger LOG = LoggerFactory.getLogger(PersisterStateManager.InitializeStateHandler.class);
Review Comment:
Is the prefix `PersisterStateManager` redundant here?
--
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.
To unsubscribe, e-mail: jira-unsubscribe@kafka.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] KAFKA-19940 Reduce the log4j2 cpu usage in PersisterStateManager [kafka]
Posted by "chia7712 (via GitHub)" <gi...@apache.org>.
chia7712 merged PR #21027:
URL: https://github.com/apache/kafka/pull/21027
--
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.
To unsubscribe, e-mail: jira-unsubscribe@kafka.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] KAFKA-19940 Reduce the log4j2 cpu usage in PersisterStateManager [kafka]
Posted by "chia7712 (via GitHub)" <gi...@apache.org>.
chia7712 commented on PR #21027:
URL: https://github.com/apache/kafka/pull/21027#issuecomment-3645044000
> It seems to me using LoggerFactory.getLogger(DefaultStatePersister.class) is good and simple, and then we could fix ConfigCommand in https://issues.apache.org/jira/browse/KAFKA-19980
@smjn What do you think about this comment? Your feedback would be conducive before I merge the changes
--
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.
To unsubscribe, e-mail: jira-unsubscribe@kafka.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] KAFKA-19940 Reduce the log4j2 cpu usage in PersisterStateManager [kafka]
Posted by "chia7712 (via GitHub)" <gi...@apache.org>.
chia7712 commented on code in PR #21027:
URL: https://github.com/apache/kafka/pull/21027#discussion_r2616063796
##########
server-common/src/main/java/org/apache/kafka/server/share/persister/PersisterStateManager.java:
##########
@@ -211,13 +210,10 @@ public PersisterStateManagerHandler(
this.onCompleteCallback = response -> {
}; // noop
partitionKey = SharePartitionKey.getInstance(groupId, topicId, partition);
- String canonicalName = getClass().getCanonicalName();
- if (canonicalName == null) {
- canonicalName = getClass().getName();
- }
- log = LoggerFactory.getLogger(canonicalName);
}
+ protected abstract Logger log();
Review Comment:
Could you add comments for this abstract method? otherwise, it looks like the method is implemented by a java tourist
--
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.
To unsubscribe, e-mail: jira-unsubscribe@kafka.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org