Skip to content

KAFKA-15045: (KIP-924 pt. 10) Topic partition rack annotation simplified #16034

Merged
ableegoldman merged 8 commits into
apache:trunkfrom
apourchet:KIP-924-10
May 23, 2024
Merged

KAFKA-15045: (KIP-924 pt. 10) Topic partition rack annotation simplified #16034
ableegoldman merged 8 commits into
apache:trunkfrom
apourchet:KIP-924-10

Conversation

@apourchet

@apourchet apourchet commented May 22, 2024

Copy link
Copy Markdown
Contributor

This PR uses the new TaskTopicPartition structure to simplify the build
process for the ApplicationState, which is the input to the new
TaskAssignor#assign call.

apourchet added 7 commits May 21, 2024 12:09
@apourchet
KAFKA-15045: (KIP-924 pt. 8) Added TopicPartitionAssignmentInfo
5f0e07f 
For task assignment purposes, the user needs to have a set of
information available for each topic partition affecting the desired
tasks.

This PR introduces a new interface for a read-only container class that
allows all the important and relevant information to be found in
one place.
@apourchet
license info
aa76fad
@apourchet
addressed comments
026be36
@apourchet
CI retrigger
ff7692d
@apourchet
Merge branch 'trunk' into KIP-924-8
a52ee43
@apourchet
KAFKA-15045: (KIP-924 pt. 10) Topic partition rack annotation simplified
8704c7a 
This PR uses the new TaskTopicPartition structure to simplify the build
process for the ApplicationState, which is the input to the new
TaskAssignor#assign call.
@apourchet
Merge branch 'trunk' into KIP-924-10
d17fc16
ableegoldman
ableegoldman reviewed May 22, 2024
View reviewed changes
Comment thread ...ams/src/main/java/org/apache/kafka/streams/processor/internals/StreamsPartitionAssignor.java Outdated
final DefaultTaskTopicPartition racklessTopicPartition = new DefaultTaskTopicPartition(
topicPartition, isSource, isChangelog, null);
allTopicPartitions.add(racklessTopicPartition);
topicPartitionsForTask.get(taskId).add(racklessTopicPartition);

@ableegoldman ableegoldman May 22, 2024

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

both here and for the changelog loop below

Suggested change
topicPartitionsForTask.get(taskId).add(racklessTopicPartition);
topicPartitions.add(racklessTopicPartition);

@apourchet apourchet May 23, 2024

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Good catch!

Comment thread streams/src/main/java/org/apache/kafka/streams/processor/internals/assignment/RackUtils.java Outdated
} else {
topicsToDescribe.addAll(topicsWithMissingMetadata(cluster, topicPartitions));
}
public static void annotateWithTopicPartitionsWithRackInfo(final Cluster cluster,

@ableegoldman ableegoldman May 22, 2024

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

this doesn't sound quite right, should it be annotateTopicPartitionsWithRackInfo? (ie extra "with" )

Comment thread streams/src/main/java/org/apache/kafka/streams/processor/internals/assignment/RackUtils.java Outdated
topicsToDescribe.addAll(topicsWithMissingMetadata(cluster, topicPartitions));
}
public static void annotateWithTopicPartitionsWithRackInfo(final Cluster cluster,
final InternalTopicManager internalTopicManager,

@ableegoldman ableegoldman May 22, 2024

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

nit: fix the formatting/indentation (although this will change when you change the method name anyway)

ableegoldman
ableegoldman approved these changes May 23, 2024
View reviewed changes

@ableegoldman ableegoldman left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

LGTM

@ableegoldman
ableegoldman merged commit 93238ae into apache:trunk May 23, 2024
@ableegoldman

ableegoldman commented May 23, 2024 •
edited
Loading

Copy link
Copy Markdown
Member

Ah shoot, I was looking at the wrong test results, this is actually breaking RackAwareTaskAssignorTest. I'll revert this and we can fix the failing test in the resubmitted PR

it's actually #15972, not this PR

ableegoldman added a commit to ableegoldman/kafka that referenced this pull request May 23, 2024
@ableegoldman
Revert "KAFKA-15045: (KIP-924 pt. 10) Topic partition rack annotation…
28b7368 
… simplified (apache#16034)"

This reverts commit 93238ae.
apourchet added a commit to apourchet/kafka that referenced this pull request May 23, 2024
@apourchet
Squashed commit of the following (during trunk merge):
03db088 
commit 93238ae
Author: Antoine Pourchet <antoine@responsive.dev>
Date:   Thu May 23 13:45:29 2024 -0600

    KAFKA-15045: (KIP-924 pt. 10) Topic partition rack annotation simplified  (apache#16034)

    This PR uses the new TaskTopicPartition structure to simplify the build
    process for the ApplicationState, which is the input to the new
    TaskAssignor#assign call.

    Reviewers: Anna Sophie Blee-Goldman <ableegoldman@apache.org>

commit 4020307
Author: Kuan-Po (Cooper) Tseng <brandboat@gmail.com>
Date:   Fri May 24 02:51:26 2024 +0800

    KAFKA-16795 Fix broken compatibility in kafka.tools.NoOpMessageFormatter, kafka.tools.DefaultMessageFormatter, and kafka.tools.LoggingMessageFormatter (apache#16020)

    This commit allows users to apply the scala version Formatters, but users will receive the warning messages about deprecation.

    This compatibility support will be removed from 4.0.0

    Reviewers: Chia-Ping Tsai <chia7712@gmail.com>

commit c3018ef
Author: TingIāu "Ting" Kì <51072200+frankvicky@users.noreply.github.com>
Date:   Fri May 24 01:15:56 2024 +0800

    KAFKA-16804: Replace archivesBaseName with archivesName (apache#16016)

    Reviewers: Chia-Ping Tsai <chia7712@gmail.com>, Greg Harris <greg.harris@aiven.io>

commit 0ba15ad
Author: Edoardo Comar <ecomar@uk.ibm.com>
Date:   Thu May 23 17:17:56 2024 +0100

    KAFKA-15905 Restarts of MirrorCheckpointTask should not permanently i… (apache#15910)

    * KAFKA-15905 Restarts of MirrorCheckpointTask should not permanently interrupt offset translation

    MirrorCheckpointTask reloads the last checkpoint at start,
    OffsetSyncStore stores OffsetSyncs before reading till end.

    If CheckpointTask cannot read checkpoints at startup,
    use previous OffsetSyncStore load logic, with
    warning log message about degraded offset translation.

    Also addresses KAFKA-16622 Mirromaker2 first Checkpoint not emitted until
    consumer group fully catches up once because the OffsetSyncStore store
    is populated before reading to log end.

    Co-Authored-By: Adrian Preston <prestona@uk.ibm.com>
    Reviewers: Greg Harris <greg.harris@aiven.io>

commit 5a48984
Author: Viktor Somogyi-Vass <viktorsomogyi@gmail.com>
Date:   Thu May 23 17:36:39 2024 +0200

    KAFKA-15649: Handle directory failure timeout (apache#15697)

    A broker that is unable to communicate with the controller will shut down
    after the configurable log.dir.failure.timeout.ms.

    The implementation adds a new event to the Kafka EventQueue. This event
    is deferred by the configured timeout and will execute the shutdown
    if the heartbeat communication containing the failed log dir is still
    pending with the controller.

    Reviewers: Igor Soarez <soarez@apple.com>

commit 8d117a1
Author: Mickael Maison <mimaison@users.noreply.github.com>
Date:   Thu May 23 17:03:24 2024 +0200

    KAFKA-16825: Update netty/jetty/jackson/zstd dependencies (apache#16038)

    Reviewers: Luke Chen <showuon@gmail.com>

commit ab0cc72
Author: Mickael Maison <mimaison@users.noreply.github.com>
Date:   Thu May 23 16:01:45 2024 +0200

    MINOR: Move parseCsvList to server-common (apache#16029)

    Reviewers: Chia-Ping Tsai <chia7712@gmail.com>

commit 14b5c4d
Author: Dongnuo Lyu <139248811+dongnuo123@users.noreply.github.com>
Date:   Thu May 23 02:27:00 2024 -0400

    KAFKA-16793; Heartbeat API for upgrading ConsumerGroup (apache#15988)

    This patch implements the heartbeat api to the members that use the classic protocol in a ConsumerGroup.

    Reviewers: Jeff Kim <jeff.kim@confluent.io>, David Jacot <djacot@confluent.io>

commit e692fee
Author: Jeff Kim <kimkb2011@gmail.com>
Date:   Thu May 23 02:24:23 2024 -0400

    MINOR: fix flaky testRecordThreadIdleRatio (apache#15987)

    DelayEventAccumulator should return immediately if there are no events in the queue. Also removed some unused fields inside EventProcessorThread.

    Reviewers: Gaurav Narula <gaurav_narula2@apple.com>, Chia-Ping Tsai <chia7712@gmail.com>, David Jacot <djacot@confluent.io>

commit bef83ce
Author: Nick Telford <nick.telford@gmail.com>
Date:   Thu May 23 05:34:31 2024 +0100

    KAFKA-15541: Add iterator-duration metrics (apache#16028)

    Part of [KIP-989](https://cwiki.apache.org/confluence/x/9KCzDw).

    This new `StateStore` metric tracks the average and maximum amount of
    time between creating and closing Iterators.

    Iterators with very high durations can indicate to users performance
    problems that should be addressed.

    If a store reports no data for these metrics, despite the user opening
    Iterators on the store, it suggests those iterators are not being
    closed, and have therefore leaked.

    Reviewers: Matthias J. Sax <matthias@confluent.io>
@ableegoldman ableegoldman mentioned this pull request May 23, 2024
Closed
chiacyu pushed a commit to chiacyu/kafka that referenced this pull request Jun 1, 2024
@apourchet @chiacyu
KAFKA-15045: (KIP-924 pt. 10) Topic partition rack annotation simplif…
7e2e6fe 
…ied (apache#16034)

This PR uses the new TaskTopicPartition structure to simplify the build
process for the ApplicationState, which is the input to the new
TaskAssignor#assign call.

Reviewers: Anna Sophie Blee-Goldman <ableegoldman@apache.org>
TaiJuWu pushed a commit to TaiJuWu/kafka that referenced this pull request Jun 8, 2024
@apourchet @TaiJuWu
KAFKA-15045: (KIP-924 pt. 10) Topic partition rack annotation simplif…
d78f64c 
…ied (apache#16034)

This PR uses the new TaskTopicPartition structure to simplify the build
process for the ApplicationState, which is the input to the new
TaskAssignor#assign call.

Reviewers: Anna Sophie Blee-Goldman <ableegoldman@apache.org>
gongxuanzhang pushed a commit to gongxuanzhang/kafka that referenced this pull request Jun 12, 2024
@apourchet @gongxuanzhang
KAFKA-15045: (KIP-924 pt. 10) Topic partition rack annotation simplif…
ed89346 
…ied (apache#16034)

This PR uses the new TaskTopicPartition structure to simplify the build
process for the ApplicationState, which is the input to the new
TaskAssignor#assign call.

Reviewers: Anna Sophie Blee-Goldman <ableegoldman@apache.org>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@ableegoldman ableegoldman ableegoldman approved these changes

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@apourchet @ableegoldman