Skip to content

KAFKA-17747: [2/N] Add compute topic and group hash #19523

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 30 commits into from
May 15, 2025
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
30 commits
Select commit Hold shift + click to select a range
36cb999
KAFKA-17747: Add compute topic and group hash
FrankYang0529 Apr 21, 2025
f4c9749
KAFKA-17747: add guava license
FrankYang0529 Apr 21, 2025
ced64d7
Merge branch 'trunk' into KAFKA-17747-2
FrankYang0529 Apr 21, 2025
7e00a14
Merge branch 'trunk' into KAFKA-17747-2
FrankYang0529 Apr 28, 2025
77b99da
Revert "KAFKA-17747: add guava license"
FrankYang0529 Apr 28, 2025
aff55ce
KAFKA-17747: Move Murmur3 to org.apache.kafka.common.internals
FrankYang0529 Apr 28, 2025
7f7649a
KAFKA-17747: Replace guava with Murmur3
FrankYang0529 Apr 28, 2025
8fc5b33
remove unused package in import-control-group-coordinator.xml
FrankYang0529 Apr 28, 2025
d2427f9
remove sbt.json
FrankYang0529 Apr 28, 2025
b5eebbf
Merge branch 'trunk' into KAFKA-17747-2
FrankYang0529 Apr 29, 2025
b6ccef3
Revert "KAFKA-17747: Move Murmur3 to org.apache.kafka.common.internals"
FrankYang0529 Apr 29, 2025
a1be1f6
KAFKA-17747: Replace Murmur3 with XXHash64
FrankYang0529 Apr 29, 2025
9b4294f
KAFKA-17747: Move static function to Utils
FrankYang0529 Apr 29, 2025
478dd98
Merge branch 'trunk' into KAFKA-17747-2
FrankYang0529 Apr 30, 2025
9fc8e8e
KAFKA-17747: Replace ByteArrayOutputStream with ByteBufferOutputStream
FrankYang0529 Apr 30, 2025
167cd4a
KAFKA-17747: Add comment
FrankYang0529 Apr 30, 2025
6e219a9
Merge branch 'trunk' into KAFKA-17747-2
FrankYang0529 May 1, 2025
08cdd37
KAFKA-17747: Change index to length in rack string
FrankYang0529 May 1, 2025
527b387
Merge branch 'trunk' into KAFKA-17747-2
FrankYang0529 May 4, 2025
cde92d8
KAFKA-17747: remove : and , in rack string and change streams to for-…
FrankYang0529 May 4, 2025
b238375
Merge branch 'trunk' into KAFKA-17747-2
FrankYang0529 May 6, 2025
a1b658a
KAFKA-17747: change to use hash4j
FrankYang0529 May 13, 2025
ed7b450
Merge branch 'trunk' into KAFKA-17747-2
FrankYang0529 May 13, 2025
44c1337
Merge branch 'trunk' into KAFKA-17747-2
FrankYang0529 May 13, 2025
f1ea74b
address comment
FrankYang0529 May 13, 2025
bb1edb3
address comment
FrankYang0529 May 14, 2025
3382702
Merge branch 'trunk' into KAFKA-17747-2
FrankYang0529 May 14, 2025
c031a29
KAFKA-17747: handle empty map in computeGroupHash
FrankYang0529 May 14, 2025
ec7024c
Merge branch 'trunk' into KAFKA-17747-2
FrankYang0529 May 14, 2025
cf35b7a
address comment
FrankYang0529 May 14, 2025
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions LICENSE-binary
Original file line number Diff line number Diff line change
Expand Up @@ -212,6 +212,7 @@ License Version 2.0:
- commons-lang3-3.12.0
- commons-logging-1.3.2
- commons-validator-1.9.0
- hash4j-0.22.0
- jackson-annotations-2.16.2
- jackson-core-2.16.2
- jackson-databind-2.16.2
Expand Down
1 change: 1 addition & 0 deletions build.gradle
Original file line number Diff line number Diff line change
Expand Up @@ -1418,6 +1418,7 @@ project(':group-coordinator') {
implementation libs.hdrHistogram
implementation libs.re2j
implementation libs.slf4jApi
implementation libs.hash4j
Copy link
Member

Choose a reason for hiding this comment

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

I suppose that we need to update the license file too.

Copy link
Member Author

Choose a reason for hiding this comment

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

Yes, I added it to LICENSE-binary and python committer-tools/verify_license.py can pass. Do I need to modify other files? Thanks.

Copy link
Member

Choose a reason for hiding this comment

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

This is fine. Thanks.


testImplementation project(':clients').sourceSets.test.output
testImplementation project(':server-common').sourceSets.test.output
Expand Down
2 changes: 2 additions & 0 deletions checkstyle/import-control-group-coordinator.xml
Original file line number Diff line number Diff line change
Expand Up @@ -51,6 +51,7 @@

<subpackage name="coordinator">
<subpackage name="group">
<allow pkg="net.jpountz.xxhash" />
<allow pkg="org.apache.kafka.clients.consumer" />
<allow pkg="org.apache.kafka.common.annotation" />
<allow pkg="org.apache.kafka.common.config" />
Expand All @@ -76,6 +77,7 @@
<allow pkg="org.apache.kafka.coordinator.common" />
<allow pkg="org.apache.kafka.coordinator.common.runtime" />
<allow pkg="com.google.re2j" />
<allow pkg="com.dynatrace.hash4j.hashing" />
<allow pkg="org.apache.kafka.metadata" />
<subpackage name="metrics">
<allow pkg="com.yammer.metrics"/>
Expand Down
6 changes: 4 additions & 2 deletions gradle/dependencies.gradle
Original file line number Diff line number Diff line change
Expand Up @@ -127,7 +127,8 @@ versions += [
// Also make sure the compression levels in org.apache.kafka.common.record.CompressionType are still valid
zstd: "1.5.6-10",
junitPlatform: "1.10.2",
hdrHistogram: "2.2.2"
hdrHistogram: "2.2.2",
hash4j: "0.22.0"
]

libs += [
Expand Down Expand Up @@ -225,5 +226,6 @@ libs += [
mavenArtifact: "org.apache.maven:maven-artifact:$versions.mavenArtifact",
zstd: "com.github.luben:zstd-jni:$versions.zstd",
httpclient: "org.apache.httpcomponents:httpclient:$versions.httpclient",
hdrHistogram: "org.hdrhistogram:HdrHistogram:$versions.hdrHistogram"
hdrHistogram: "org.hdrhistogram:HdrHistogram:$versions.hdrHistogram",
hash4j: "com.dynatrace.hash4j:hash4j:$versions.hash4j",
]
Original file line number Diff line number Diff line change
Expand Up @@ -25,10 +25,15 @@
import org.apache.kafka.common.protocol.ApiMessage;
import org.apache.kafka.coordinator.group.generated.ConsumerGroupCurrentMemberAssignmentValue;
import org.apache.kafka.coordinator.group.generated.ShareGroupCurrentMemberAssignmentValue;
import org.apache.kafka.image.ClusterImage;
import org.apache.kafka.image.MetadataImage;
import org.apache.kafka.image.TopicImage;
import org.apache.kafka.image.TopicsImage;
import org.apache.kafka.metadata.BrokerRegistration;
import org.apache.kafka.server.common.ApiMessageAndVersion;

import com.dynatrace.hash4j.hashing.HashStream64;
import com.dynatrace.hash4j.hashing.Hashing;
import com.google.re2j.Pattern;
import com.google.re2j.PatternSyntaxException;

Expand Down Expand Up @@ -324,4 +329,99 @@ static void throwIfRegularExpressionIsInvalid(
regex, ex.getDescription()));
}
}

/**
* The magic byte used to identify the version of topic hash function.
*/
static final byte TOPIC_HASH_MAGIC_BYTE = 0x00;

/**
* Computes the hash of the topics in a group.
* <p>
* The computed hash value is stored as the metadata hash in the *GroupMetadataValue.
* <p>
* If there is no topic, the hash value is set to 0.
* The hashing process involves the following steps:
* 1. Sort the topic hashes by topic name.
* 2. Write each topic hash in order.
*
* @param topicHashes The map of topic hashes. Key is topic name and value is the topic hash.
* @return The hash of the group.
*/
static long computeGroupHash(Map<String, Long> topicHashes) {
Copy link
Member

Choose a reason for hiding this comment

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

nit: public?

Copy link
Member Author

Choose a reason for hiding this comment

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

IIUC, this function will only be used in GroupMetadataManager like throwIfRegularExpressionIsInvalid. I think we can add public when we need it. WDYT?

Copy link
Member

Choose a reason for hiding this comment

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

Ok. It works for me.

if (topicHashes.isEmpty()) {
return 0;
}

// Sort entries by topic name
List<Map.Entry<String, Long>> sortedEntries = new ArrayList<>(topicHashes.entrySet());
sortedEntries.sort(Map.Entry.comparingByKey());

HashStream64 hasher = Hashing.xxh3_64().hashStream();
for (Map.Entry<String, Long> entry : sortedEntries) {
hasher.putLong(entry.getValue());
}

return hasher.getAsLong();
}

/**
* Computes the hash of the topic id, name, number of partitions, and partition racks by streaming XXH3.
* <p>
* The computed hash value for the topic is utilized in conjunction with the {@link #computeGroupHash(Map)}
* method and is stored as part of the metadata hash in the *GroupMetadataValue.
* It is important to note that if the hash algorithm is changed, the magic byte must be updated to reflect the
* new hash version.
* <p>
* For non-existent topics, the hash value is set to 0.
* For existent topics, the hashing process involves the following steps:
* 1. Write a magic byte to denote the version of the hash function.
* 2. Write the hash code of the topic ID with mostSignificantBits and leastSignificantBits.
* 3. Write the topic name.
* 4. Write the number of partitions associated with the topic.
* 5. For each partition, write the partition ID and a sorted list of rack identifiers.
* - Rack identifiers are formatted as "<length1><value1><length2><value2>" to prevent issues with simple separators.
*
* @param topicName The topic image.
* @param metadataImage The cluster image.
* @return The hash of the topic.
*/
static long computeTopicHash(String topicName, MetadataImage metadataImage) {
TopicImage topicImage = metadataImage.topics().getTopic(topicName);
if (topicImage == null) {
return 0;
}

HashStream64 hasher = Hashing.xxh3_64().hashStream();
hasher = hasher
.putByte(TOPIC_HASH_MAGIC_BYTE)
.putLong(topicImage.id().getMostSignificantBits())
.putLong(topicImage.id().getLeastSignificantBits())
.putString(topicImage.name())
.putInt(topicImage.partitions().size());

ClusterImage clusterImage = metadataImage.cluster();
List<String> racks = new ArrayList<>();
for (int i = 0; i < topicImage.partitions().size(); i++) {
hasher = hasher.putInt(i);
racks.clear(); // Clear the list for reuse
for (int replicaId : topicImage.partitions().get(i).replicas) {
BrokerRegistration broker = clusterImage.broker(replicaId);
if (broker != null) {
broker.rack().ifPresent(racks::add);
}
}

Collections.sort(racks);
for (String rack : racks) {
// Format: "<length><value>"
// The rack string combination cannot use simple separator like ",", because there is no limitation for rack character.
// If using simple separator like "," it may hit edge case like ",," and ",,," / ",,," and ",,".
// Add length before the rack string to avoid the edge case.
hasher = hasher.putInt(rack.length()).putString(rack);
}
}

return hasher.getAsLong();
Copy link
Member

Choose a reason for hiding this comment

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

nit: Should we put an empty line before the return?

Copy link
Member Author

Choose a reason for hiding this comment

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

Updated it.

}
}
Loading