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/07/30 14:24:56 UTC
[PR] KAFKA-17876/ KAFKA-19150 Rename AssignmentsManager and RemoteStorageThreadPool metrics [kafka]
m1a2st opened a new pull request, #20265:
URL: https://github.com/apache/kafka/pull/20265
Rename org.apache.kafka.server:type=AssignmentsManager and org.apache.kafka.storage.internals.log.RemoteStorageThreadPool metrics for the consist, these metrics should be
- `kafka.log.remote:type=RemoteStorageThreadPool.RemoteLogReaderTaskQueueSize`
- `kafka.log.remote:type=RemoteStorageThreadPool.RemoteLogReaderAvgIdlePercent`
- `kafka.server:type=AssignmentsManager.QueuedReplicaToDirAssignments`
--
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-17876/ KAFKA-19150 Rename AssignmentsManager and RemoteStorageThreadPool metrics [kafka]
Posted by "chia7712 (via GitHub)" <gi...@apache.org>.
chia7712 commented on PR #20265:
URL: https://github.com/apache/kafka/pull/20265#issuecomment-3343923856
@m1a2st please open a jira for 5.0
--
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-17876/ KAFKA-19150 Rename AssignmentsManager and RemoteStorageThreadPool metrics [kafka]
Posted by "chia7712 (via GitHub)" <gi...@apache.org>.
chia7712 commented on code in PR #20265:
URL: https://github.com/apache/kafka/pull/20265#discussion_r2342897827
##########
server/src/main/java/org/apache/kafka/server/AssignmentsManager.java:
##########
@@ -74,6 +74,10 @@ public final class AssignmentsManager {
/**
* The metric reflecting the number of pending assignments.
*/
+ @Deprecated(since = "4.2")
+ static final MetricName DEPRECATED_QUEUED_REPLICA_TO_DIR_ASSIGNMENTS_METRIC =
+ deprecatedMetricName("QueuedReplicaToDirAssignments");
+
static final MetricName QUEUED_REPLICA_TO_DIR_ASSIGNMENTS_METRIC =
metricName("QueuedReplicaToDirAssignments");
Review Comment:
ditto
##########
storage/src/main/java/org/apache/kafka/storage/internals/log/RemoteStorageThreadPool.java:
##########
@@ -32,7 +32,8 @@
public final class RemoteStorageThreadPool extends ThreadPoolExecutor {
private static final Logger LOGGER = LoggerFactory.getLogger(RemoteStorageThreadPool.class);
- private final KafkaMetricsGroup metricsGroup = new KafkaMetricsGroup(this.getClass());
+ private final KafkaMetricsGroup internalsLogMetricsGroup = new KafkaMetricsGroup(this.getClass());
Review Comment:
Please add comments to remind developers this `internalsLogMetricsGroup` will be removed in 5.0
##########
server/src/main/java/org/apache/kafka/server/AssignmentsManager.java:
##########
@@ -74,6 +74,10 @@ public final class AssignmentsManager {
/**
* The metric reflecting the number of pending assignments.
*/
+ @Deprecated(since = "4.2")
+ static final MetricName DEPRECATED_QUEUED_REPLICA_TO_DIR_ASSIGNMENTS_METRIC =
Review Comment:
Could you please inline `deprecatedMetricName` to streamline code?
--
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-17876/ KAFKA-19150 Rename AssignmentsManager and RemoteStorageThreadPool metrics [kafka]
Posted by "chia7712 (via GitHub)" <gi...@apache.org>.
chia7712 commented on code in PR #20265:
URL: https://github.com/apache/kafka/pull/20265#discussion_r2379636713
##########
storage/src/main/java/org/apache/kafka/storage/internals/log/RemoteStorageThreadPool.java:
##########
@@ -45,9 +48,13 @@ public RemoteStorageThreadPool(String threadNamePattern,
ThreadUtils.createThreadFactory(threadNamePattern, false,
(t, e) -> LOGGER.error("Uncaught exception in thread '{}':", t.getName(), e))
);
- metricsGroup.newGauge(REMOTE_LOG_READER_TASK_QUEUE_SIZE_METRIC.getName(),
+ deprecatedLogMetricsGroup.newGauge(REMOTE_LOG_READER_TASK_QUEUE_SIZE_METRIC.getName(),
Review Comment:
Could you please use `DEPRECATE_REMOTE_LOG_READER_TASK_QUEUE_SIZE_METRIC` for consistency?
##########
storage/src/main/java/org/apache/kafka/storage/internals/log/RemoteStorageThreadPool.java:
##########
@@ -45,9 +48,13 @@ public RemoteStorageThreadPool(String threadNamePattern,
ThreadUtils.createThreadFactory(threadNamePattern, false,
(t, e) -> LOGGER.error("Uncaught exception in thread '{}':", t.getName(), e))
);
- metricsGroup.newGauge(REMOTE_LOG_READER_TASK_QUEUE_SIZE_METRIC.getName(),
+ deprecatedLogMetricsGroup.newGauge(REMOTE_LOG_READER_TASK_QUEUE_SIZE_METRIC.getName(),
() -> getQueue().size());
- metricsGroup.newGauge(REMOTE_LOG_READER_AVG_IDLE_PERCENT_METRIC.getName(),
+ deprecatedLogMetricsGroup.newGauge(REMOTE_LOG_READER_AVG_IDLE_PERCENT_METRIC.getName(),
Review Comment:
ditto
--
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-17876/ KAFKA-19150 Rename AssignmentsManager and RemoteStorageThreadPool metrics [kafka]
Posted by "m1a2st (via GitHub)" <gi...@apache.org>.
m1a2st commented on PR #20265:
URL: https://github.com/apache/kafka/pull/20265#issuecomment-3344594004
FYI: https://issues.apache.org/jira/browse/KAFKA-19741
--
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-17876/ KAFKA-19150 Rename AssignmentsManager and RemoteStorageThreadPool metrics [kafka]
Posted by "m1a2st (via GitHub)" <gi...@apache.org>.
m1a2st commented on PR #20265:
URL: https://github.com/apache/kafka/pull/20265#issuecomment-3285007052
Thanks for @chia7712 review, I addressed all comments.
--
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-17876/ KAFKA-19150 Rename AssignmentsManager and RemoteStorageThreadPool metrics [kafka]
Posted by "chia7712 (via GitHub)" <gi...@apache.org>.
chia7712 commented on PR #20265:
URL: https://github.com/apache/kafka/pull/20265#issuecomment-3298040939
@m1a2st please fix the conflicts
--
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-17876/ KAFKA-19150 Rename AssignmentsManager and RemoteStorageThreadPool metrics [kafka]
Posted by "m1a2st (via GitHub)" <gi...@apache.org>.
m1a2st commented on code in PR #20265:
URL: https://github.com/apache/kafka/pull/20265#discussion_r2296404599
##########
docs/upgrade.html:
##########
@@ -508,10 +509,6 @@ <h5><a id="upgrade_servers_400_notable" href="#upgrade_servers_400_notable">Nota
<li> See <a href="https://cwiki.apache.org/confluence/x/B40ODg">KIP-890</a> and
<a href="https://cwiki.apache.org/confluence/x/8ItyEg">KIP-1050</a> for more details </li>
</ul>
- <li>
- The filename for rotated <code>state-change.log</code> files incorrectly rotates to <code>stage-change.log.[date]</code> (changing state to stage). This issue is corrected in 4.0.1.
Review Comment:
I will revert this change.
--
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-17876/ KAFKA-19150 Rename AssignmentsManager and RemoteStorageThreadPool metrics [kafka]
Posted by "chia7712 (via GitHub)" <gi...@apache.org>.
chia7712 commented on PR #20265:
URL: https://github.com/apache/kafka/pull/20265#issuecomment-3165310946
@m1a2st please fix the conflicts
--
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-17876/ KAFKA-19150 Rename AssignmentsManager and RemoteStorageThreadPool metrics [kafka]
Posted by "chia7712 (via GitHub)" <gi...@apache.org>.
chia7712 commented on code in PR #20265:
URL: https://github.com/apache/kafka/pull/20265#discussion_r2243274230
##########
storage/api/src/main/java/org/apache/kafka/server/log/remote/storage/RemoteStorageMetrics.java:
##########
@@ -94,6 +94,20 @@ public class RemoteStorageMetrics {
"org.apache.kafka.storage.internals.log", "RemoteStorageThreadPool", REMOTE_LOG_READER_TASK_QUEUE_SIZE);
public static final MetricName REMOTE_LOG_READER_AVG_IDLE_PERCENT_METRIC = getMetricName(
"org.apache.kafka.storage.internals.log", "RemoteStorageThreadPool", REMOTE_LOG_READER_AVG_IDLE_PERCENT);
+ /**
+ * Since `REMOTE_LOG_READER_TASK_QUEUE_SIZE_METRIC` and `REMOTE_LOG_READER_AVG_IDLE_PERCENT_METRIC`
+ * are part of the public API, we add new metrics with the same names under the `kafka.log.remote` group
+ * and prefix them with `BRIDGE_` to avoid conflicts with the existing `MetricName` values.
+ * <p>
+ * The new metrics are marked as deprecated. In Kafka 5.0, we will remove the deprecated metrics
+ * and use the `kafka.log.remote` group name as the standard going forward.
+ */
+ @Deprecated(since = "4.2")
Review Comment:
I brought this up in the mail channel (https://lists.apache.org/thread/cpsdrwcbdyys69ozr5kfbsycrkx8zkcn)
--
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-17876/ KAFKA-19150 Rename AssignmentsManager and RemoteStorageThreadPool metrics [kafka]
Posted by "chia7712 (via GitHub)" <gi...@apache.org>.
chia7712 merged PR #20265:
URL: https://github.com/apache/kafka/pull/20265
--
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-17876/ KAFKA-19150 Rename AssignmentsManager and RemoteStorageThreadPool metrics [kafka]
Posted by "chia7712 (via GitHub)" <gi...@apache.org>.
chia7712 commented on code in PR #20265:
URL: https://github.com/apache/kafka/pull/20265#discussion_r2296250817
##########
server/src/main/java/org/apache/kafka/server/AssignmentsManager.java:
##########
@@ -182,12 +191,18 @@ public AssignmentsManager(
this.ready = new ConcurrentHashMap<>();
this.inflight = Map.of();
this.metricsRegistry = metricsRegistry;
- this.metricsRegistry.newGauge(QUEUED_REPLICA_TO_DIR_ASSIGNMENTS_METRIC, new Gauge<Integer>() {
+ this.metricsRegistry.newGauge(DEPRECATED_QUEUED_REPLICA_TO_DIR_ASSIGNMENTS_METRIC, new Gauge<Integer>() {
@Override
Review Comment:
Please also fix the indent
##########
storage/api/src/main/java/org/apache/kafka/server/log/remote/storage/RemoteStorageMetrics.java:
##########
@@ -90,10 +90,16 @@ public class RemoteStorageMetrics {
"kafka.server", "BrokerTopicMetrics", REMOTE_DELETE_LAG_SEGMENTS);
public static final MetricName REMOTE_LOG_MANAGER_TASKS_AVG_IDLE_PERCENT_METRIC = getMetricName(
"kafka.log.remote", "RemoteLogManager", REMOTE_LOG_MANAGER_TASKS_AVG_IDLE_PERCENT);
- public static final MetricName REMOTE_LOG_READER_TASK_QUEUE_SIZE_METRIC = getMetricName(
+ @Deprecated(since = "4.2")
+ private static final MetricName DEPRECATE_REMOTE_LOG_READER_TASK_QUEUE_SIZE_METRIC = getMetricName(
"org.apache.kafka.storage.internals.log", "RemoteStorageThreadPool", REMOTE_LOG_READER_TASK_QUEUE_SIZE);
- public static final MetricName REMOTE_LOG_READER_AVG_IDLE_PERCENT_METRIC = getMetricName(
+ @Deprecated(since = "4.2")
+ private static final MetricName DEPRECATE_REMOTE_LOG_READER_AVG_IDLE_PERCENT_METRIC = getMetricName(
Review Comment:
Have you created the jira for the cleanup in 5.0? If not, please open one
##########
docs/upgrade.html:
##########
@@ -508,10 +509,6 @@ <h5><a id="upgrade_servers_400_notable" href="#upgrade_servers_400_notable">Nota
<li> See <a href="https://cwiki.apache.org/confluence/x/B40ODg">KIP-890</a> and
<a href="https://cwiki.apache.org/confluence/x/8ItyEg">KIP-1050</a> for more details </li>
</ul>
- <li>
- The filename for rotated <code>state-change.log</code> files incorrectly rotates to <code>stage-change.log.[date]</code> (changing state to stage). This issue is corrected in 4.0.1.
Review Comment:
why to remove this?
##########
docs/upgrade.html:
##########
@@ -110,9 +117,9 @@ <h5><a id="upgrade_410_notable" href="#upgrade_410_notable">Notable changes in 4
</li>
</ul>
-<h4><a id="upgrade_4_0_1_from" href="#upgrade_4_0_1_from">Upgrading to 4.0.1</a></h4>
+<h4><a id="upgrade_4_0_0_from" href="#upgrade_4_0_0_from">Upgrading to 4.0.0</a></h4>
Review Comment:
ditto
##########
docs/upgrade.html:
##########
@@ -74,8 +83,6 @@ <h5><a id="upgrade_410_notable" href="#upgrade_410_notable">Notable changes in 4
</li>
<li>
The KIP-966 part 1: Eligible Leader Replicas(ELR) will be enabled by default on the new clusters.
- After the ELR feature enabled, the previously set <code>min.insync.replicas</code> value at the broker-level config will be removed.
Review Comment:
ditto
--
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-17876/ KAFKA-19150 Rename AssignmentsManager and RemoteStorageThreadPool metrics [kafka]
Posted by "m1a2st (via GitHub)" <gi...@apache.org>.
m1a2st commented on code in PR #20265:
URL: https://github.com/apache/kafka/pull/20265#discussion_r2296408599
##########
storage/api/src/main/java/org/apache/kafka/server/log/remote/storage/RemoteStorageMetrics.java:
##########
@@ -90,10 +90,16 @@ public class RemoteStorageMetrics {
"kafka.server", "BrokerTopicMetrics", REMOTE_DELETE_LAG_SEGMENTS);
public static final MetricName REMOTE_LOG_MANAGER_TASKS_AVG_IDLE_PERCENT_METRIC = getMetricName(
"kafka.log.remote", "RemoteLogManager", REMOTE_LOG_MANAGER_TASKS_AVG_IDLE_PERCENT);
- public static final MetricName REMOTE_LOG_READER_TASK_QUEUE_SIZE_METRIC = getMetricName(
+ @Deprecated(since = "4.2")
+ private static final MetricName DEPRECATE_REMOTE_LOG_READER_TASK_QUEUE_SIZE_METRIC = getMetricName(
"org.apache.kafka.storage.internals.log", "RemoteStorageThreadPool", REMOTE_LOG_READER_TASK_QUEUE_SIZE);
- public static final MetricName REMOTE_LOG_READER_AVG_IDLE_PERCENT_METRIC = getMetricName(
+ @Deprecated(since = "4.2")
+ private static final MetricName DEPRECATE_REMOTE_LOG_READER_AVG_IDLE_PERCENT_METRIC = getMetricName(
Review Comment:
FYI: https://issues.apache.org/jira/browse/KAFKA-19640
--
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-17876/ KAFKA-19150 Rename AssignmentsManager and RemoteStorageThreadPool metrics [kafka]
Posted by "chia7712 (via GitHub)" <gi...@apache.org>.
chia7712 commented on code in PR #20265:
URL: https://github.com/apache/kafka/pull/20265#discussion_r2350226883
##########
storage/src/main/java/org/apache/kafka/storage/internals/log/RemoteStorageThreadPool.java:
##########
@@ -32,7 +32,10 @@
public final class RemoteStorageThreadPool extends ThreadPoolExecutor {
private static final Logger LOGGER = LoggerFactory.getLogger(RemoteStorageThreadPool.class);
- private final KafkaMetricsGroup metricsGroup = new KafkaMetricsGroup(this.getClass());
+ @Deprecated(since = "4.2")
+ // This metrics group is used to register deprecated metrics. It will be removed in Kafka 5.0
+ private final KafkaMetricsGroup internalsLogMetricsGroup = new KafkaMetricsGroup(this.getClass());
Review Comment:
Also, should it be named `deprecatedLogMetricsGroup` instead of `internalsLogMetricsGroup`?
##########
storage/src/main/java/org/apache/kafka/storage/internals/log/RemoteStorageThreadPool.java:
##########
@@ -32,7 +32,10 @@
public final class RemoteStorageThreadPool extends ThreadPoolExecutor {
private static final Logger LOGGER = LoggerFactory.getLogger(RemoteStorageThreadPool.class);
- private final KafkaMetricsGroup metricsGroup = new KafkaMetricsGroup(this.getClass());
+ @Deprecated(since = "4.2")
+ // This metrics group is used to register deprecated metrics. It will be removed in Kafka 5.0
+ private final KafkaMetricsGroup internalsLogMetricsGroup = new KafkaMetricsGroup(this.getClass());
Review Comment:
Could you please use `KafkaMetricsGroup(String, String)` instead to avoid breaking the compatibility again?
##########
server/src/main/java/org/apache/kafka/server/AssignmentsManager.java:
##########
@@ -74,8 +74,12 @@ public final class AssignmentsManager {
/**
* The metric reflecting the number of pending assignments.
Review Comment:
Could you please move the comment to the new metric name? it would also be helpful to describe the purpose of the deprecated one
--
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