Skip to content

KAFKA-10799 AlterIsr utilizes ReplicaManager ISR metrics#9677

Merged
mumrah merged 11 commits intoapache:trunkfrom
mumrah:KAFKA-10799-alter-isr-metrics
Dec 3, 2020
Merged

KAFKA-10799 AlterIsr utilizes ReplicaManager ISR metrics#9677
mumrah merged 11 commits intoapache:trunkfrom
mumrah:KAFKA-10799-alter-isr-metrics

Conversation

@mumrah
Copy link
Copy Markdown
Member

@mumrah mumrah commented Dec 2, 2020

In #9100, we missed the inclusion of the high level ISR shrink/expand metrics that are managed by ReplicaManager. This PR adds a small abstraction that allows us to mark these metrics without bringing ReplicaManager in as a dependency of Partition.

This allows us to update the ISR metrics in ReplicaManager without introducing it
as a dependency to Partition.

Also, this change includes updating these metrics for ISR changes done through
AlterIsr
Copy link
Copy Markdown
Contributor

@hachikuji hachikuji left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks reasonable. Would it be possible to alter one of the existing test cases to verify that the meters are updated?

Comment thread core/src/main/scala/kafka/cluster/Partition.scala Outdated
Comment thread core/src/main/scala/kafka/cluster/Partition.scala
Comment thread core/src/main/scala/kafka/cluster/Partition.scala
Comment thread core/src/main/scala/kafka/cluster/Partition.scala Outdated
Copy link
Copy Markdown
Contributor

@hachikuji hachikuji left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM except for the one case you called it.

Comment thread core/src/main/scala/kafka/cluster/Partition.scala
Copy link
Copy Markdown
Contributor

@hachikuji hachikuji left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM. Left one nit.

@mumrah
Copy link
Copy Markdown
Member Author

mumrah commented Dec 3, 2020

Failed tests are known flaky

https://ci-builds.apache.org/job/Kafka/job/kafka-pr/job/PR-9677/8/
image

@mumrah mumrah merged commit 633f7cf into apache:trunk Dec 3, 2020
mumrah added a commit that referenced this pull request Dec 3, 2020
Add small interface to Partition.scala that allows AlterIsr and ZK code paths to update the ISR metrics managed by ReplicaManager. This opens the door for consolidating even more code between the two ISR update code paths.

Cherry-picked from trunk
ijuma added a commit to ijuma/kafka that referenced this pull request Dec 3, 2020
…t-for-generated-requests

* apache-github/trunk:
MINOR: Fix flaky test shouldQueryOnlyActivePartitionStoresByDefault
(apache#9681)
  KAFKA-10799 AlterIsr utilizes ReplicaManager ISR metrics (apache#9677)
  MINOR: Fix KTable-KTable foreign-key join example (apache#9683)
KAFKA-10473: Add docs on partition size-on-disk, and other log-related
metrics (apache#9276)
  KAFKA-10739; Replace EpochEndOffset with automated protocol (apache#9630)
KAFKA-10460: ReplicaListValidator format checking is incomplete
(apache#9326)
KAFKA-10554; Perform follower truncation based on diverging epochs in
Fetch response (apache#9382)
  MINOR: Align the UID inside/outside container (apache#9652)
KAFKA-10794 Replica leader election is too slow in the case of too
many partitions (apache#9675)
KAFKA-10090 Misleading warnings: The configuration was supplied but i…
(apache#8826)

clients/src/main/java/org/apache/kafka/common/requests/OffsetsForLeaderEpochResponse.java
clients/src/test/java/org/apache/kafka/clients/consumer/internals/FetcherTest.java
core/src/test/scala/unit/kafka/server/epoch/util/ReplicaFetcherMockBlockingSend.scala
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants