You are viewing a plain text version of this content. The canonical link for it is here.
Posted to jira@kafka.apache.org by "frankvicky (via GitHub)" <gi...@apache.org> on 2026/03/20 15:47:48 UTC

[PR] MINOR: follow-up of KAFKA-20317 [kafka]

frankvicky opened a new pull request, #21838:
URL: https://github.com/apache/kafka/pull/21838

   ref: https://github.com/apache/kafka/pull/21780#pullrequestreview-3969893622
   


-- 
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] MINOR: follow-up of KAFKA-20317 [kafka]

Posted by "chia7712 (via GitHub)" <gi...@apache.org>.
chia7712 commented on PR #21838:
URL: https://github.com/apache/kafka/pull/21838#issuecomment-4174771010

   @frankvicky please fix 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] MINOR: follow-up of KAFKA-20317 [kafka]

Posted by "aliehsaeedii (via GitHub)" <gi...@apache.org>.
aliehsaeedii commented on code in PR #21838:
URL: https://github.com/apache/kafka/pull/21838#discussion_r2993125507


##########
streams/src/main/java/org/apache/kafka/streams/state/ValueWithHeaders.java:
##########
@@ -24,62 +24,62 @@
  * Combines an aggregated value with its associated record headers.
  * This is used by SessionStoreWithHeaders to store session aggregations along with headers.
  *
- * @param <AGG> the aggregation type
+ * @param <V> the aggregation type
  */
-public final class AggregationWithHeaders<AGG> {
+public final class ValueWithHeaders<V> {
 
-    private final AGG aggregation;
+    private final V value;
     private final Headers headers;
 
-    private AggregationWithHeaders(final AGG aggregation, final Headers headers) {
+    private ValueWithHeaders(final V value, final Headers headers) {
         Objects.requireNonNull(headers, "headers must not be null");
-        this.aggregation = aggregation;
+        this.value = value;
         this.headers = headers;
     }
 
     /**
-     * Create a new {@link AggregationWithHeaders} instance if the provided {@code aggregation} is not {@code null}.
+     * Create a new {@link ValueWithHeaders} instance if the provided {@code aggregation} is not {@code null}.
      *
      * @param aggregation the aggregation
      * @param headers     the headers (may be {@code null}, treated as empty)
      * @param <AGG>       the type of the aggregation
-     * @return a new {@link AggregationWithHeaders} instance if the provided {@code aggregation} is not {@code null};
+     * @return a new {@link ValueWithHeaders} instance if the provided {@code aggregation} is not {@code null};
      * otherwise {@code null} is returned
      */
-    public static <AGG> AggregationWithHeaders<AGG> make(final AGG aggregation, final Headers headers) {
+    public static <AGG> ValueWithHeaders<AGG> make(final AGG aggregation, final Headers headers) {
         if (aggregation == null) {
             return null;
         }
-        return new AggregationWithHeaders<>(aggregation, headers);
+        return new ValueWithHeaders<>(aggregation, headers);
     }
 
     /**
-     * Create a new {@link AggregationWithHeaders} instance.
+     * Create a new {@link ValueWithHeaders} instance.
      * The provided {@code aggregation} may be {@code null}.
      *
      * @param aggregation the aggregation (may be {@code null})
      * @param headers     the headers (may be {@code null}, treated as empty)
      * @param <AGG>       the type of the aggregation
-     * @return a new {@link AggregationWithHeaders} instance
+     * @return a new {@link ValueWithHeaders} instance
      */
-    public static <AGG> AggregationWithHeaders<AGG> makeAllowNullable(final AGG aggregation, final Headers headers) {
-        return new AggregationWithHeaders<>(aggregation, headers);
+    public static <AGG> ValueWithHeaders<AGG> makeAllowNullable(final AGG aggregation, final Headers headers) {
+        return new ValueWithHeaders<>(aggregation, headers);
     }
 
     /**
-     * Return the wrapped {@code aggregation} of the given {@code aggregationWithHeaders} parameter
+     * Return the wrapped {@code aggregation} of the given {@code valueWithHeaders} parameter
      * if the parameter is not {@code null}.
      *
-     * @param aggregationWithHeaders an {@link AggregationWithHeaders} instance; can be {@code null}
+     * @param valueWithHeaders an {@link ValueWithHeaders} instance; can be {@code null}
      * @param <AGG>                  the type of the aggregation
-     * @return the wrapped {@code aggregation} of {@code aggregationWithHeaders} if not {@code null}; otherwise {@code null}
+     * @return the wrapped {@code aggregation} of {@code valueWithHeaders} if not {@code null}; otherwise {@code null}
      */
-    public static <AGG> AGG getAggregationOrNull(final AggregationWithHeaders<AGG> aggregationWithHeaders) {
-        return aggregationWithHeaders == null ? null : aggregationWithHeaders.aggregation;
+    public static <AGG> AGG getValueOrNull(final ValueWithHeaders<AGG> valueWithHeaders) {
+        return valueWithHeaders == null ? null : valueWithHeaders.value;
     }
 
-    public AGG aggregation() {
-        return aggregation;
+    public V aggregation() {
+        return value;

Review Comment:
    The internal field is renamed to `value`, but the public getter is still `aggregation()`. It may result into confusion.



##########
streams/src/main/java/org/apache/kafka/streams/state/ValueWithHeaders.java:
##########
@@ -24,62 +24,62 @@
  * Combines an aggregated value with its associated record headers.
  * This is used by SessionStoreWithHeaders to store session aggregations along with headers.
  *
- * @param <AGG> the aggregation type
+ * @param <V> the aggregation type
  */
-public final class AggregationWithHeaders<AGG> {
+public final class ValueWithHeaders<V> {
 
-    private final AGG aggregation;
+    private final V value;
     private final Headers headers;
 
-    private AggregationWithHeaders(final AGG aggregation, final Headers headers) {
+    private ValueWithHeaders(final V value, final Headers headers) {
         Objects.requireNonNull(headers, "headers must not be null");
-        this.aggregation = aggregation;
+        this.value = value;
         this.headers = headers;
     }
 
     /**
-     * Create a new {@link AggregationWithHeaders} instance if the provided {@code aggregation} is not {@code null}.
+     * Create a new {@link ValueWithHeaders} instance if the provided {@code aggregation} is not {@code null}.
      *
      * @param aggregation the aggregation
      * @param headers     the headers (may be {@code null}, treated as empty)
      * @param <AGG>       the type of the aggregation
-     * @return a new {@link AggregationWithHeaders} instance if the provided {@code aggregation} is not {@code null};
+     * @return a new {@link ValueWithHeaders} instance if the provided {@code aggregation} is not {@code null};
      * otherwise {@code null} is returned
      */
-    public static <AGG> AggregationWithHeaders<AGG> make(final AGG aggregation, final Headers headers) {
+    public static <AGG> ValueWithHeaders<AGG> make(final AGG aggregation, final Headers headers) {

Review Comment:
   To be consistent cold we change the var name from `aggregation` to `value` as well? Also updating the javadocs correspondingly.



##########
streams/src/main/java/org/apache/kafka/streams/state/ValueWithHeaders.java:
##########
@@ -91,23 +91,23 @@ public boolean equals(final Object o) {
         if (this == o) {
             return true;
         }
-        if (!(o instanceof AggregationWithHeaders)) {
+        if (!(o instanceof ValueWithHeaders)) {
             return false;
         }
-        final AggregationWithHeaders<?> that = (AggregationWithHeaders<?>) o;
-        return Objects.equals(aggregation, that.aggregation)
+        final ValueWithHeaders<?> that = (ValueWithHeaders<?>) o;
+        return Objects.equals(value, that.value)
             && Objects.equals(this.headers, that.headers);
     }
 
     @Override
     public int hashCode() {
-        return Objects.hash(aggregation, headers);
+        return Objects.hash(value, headers);
     }
 
     @Override
     public String toString() {
-        return "AggregationWithHeaders{" +
-            "aggregation=" + aggregation +
+        return "ValueWithHeaders{" +
+            "aggregation=" + value +

Review Comment:
   ```suggestion
               "value=" + value +
   ```



-- 
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] MINOR: follow-up of KAFKA-20317 [kafka]

Posted by "frankvicky (via GitHub)" <gi...@apache.org>.
frankvicky commented on PR #21838:
URL: https://github.com/apache/kafka/pull/21838#issuecomment-4336702019

   Open a new PR for this one: https://github.com/apache/kafka/pull/22171


-- 
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] MINOR: follow-up of KAFKA-20317 [kafka]

Posted by "mjsax (via GitHub)" <gi...@apache.org>.
mjsax commented on code in PR #21838:
URL: https://github.com/apache/kafka/pull/21838#discussion_r3033834807


##########
streams/src/main/java/org/apache/kafka/streams/state/internals/MeteredSessionStoreWithHeaders.java:
##########
@@ -190,23 +190,23 @@ public <R> QueryResult<R> query(final Query<R> query,
     }
 
     @Override
-    public KeyValueIterator<Windowed<K>, AggregationWithHeaders<AGG>> fetch(final K key) {
+    public KeyValueIterator<Windowed<K>, ValueWithHeaders<AGG>> fetch(final K key) {
         Objects.requireNonNull(key, "key cannot be null");
         return new MeteredSessionStoreWithHeadersIterator(
             wrapped().fetch(keyBytes(key, new RecordHeaders(), serdes))
         );
     }
 
     @Override
-    public KeyValueIterator<Windowed<K>, AggregationWithHeaders<AGG>> backwardFetch(final K key) {
+    public KeyValueIterator<Windowed<K>, ValueWithHeaders<AGG>> backwardFetch(final K key) {
         Objects.requireNonNull(key, "key cannot be null");
         return new MeteredSessionStoreWithHeadersIterator(
             wrapped().backwardFetch(keyBytes(key, new RecordHeaders(), serdes))
         );
     }
 
     @Override
-    public KeyValueIterator<Windowed<K>, AggregationWithHeaders<AGG>> fetch(final K keyFrom,
+    public KeyValueIterator<Windowed<K>, ValueWithHeaders<AGG>> fetch(final K keyFrom,
                                                                             final K keyTo) {

Review Comment:
   @aliehsaeedii -- that's why the current formatting is bad... 
   
   We would need to fix the indention here, too. That's why I prefer to either make a method definition a single line:
   ```
   public KeyValueIterator<Windowed<K>, AggregationWithHeaders<AGG>> fetch(final K keyFrom, final K keyTo) {
   ```
   or, it the line becomes too long, move every parameter into it's own line:
   ```
   public KeyValueIterator<Windowed<K>, AggregationWithHeaders<AGG>> fetch(
       final K keyFrom,
       final K keyTo
   ) {
   ```
   to avoid all this indention non-sense.



##########
streams/integration-tests/src/test/java/org/apache/kafka/streams/integration/HeadersStoreUpgradeIntegrationTest.java:
##########
@@ -1830,21 +1830,21 @@ private void verifySessionValueWithEmptyHeaders(final String key,
                 if (processorRef.get() == null) {
                     return false;
                 }
-                final ReadOnlySessionStore<String, AggregationWithHeaders<String>> store = processorRef.get().store();
+                final ReadOnlySessionStore<String, ValueWithHeaders<String>> store = processorRef.get().store();

Review Comment:
   `AggregationWithHeaders` is a public class added via KIP-1271 -- do we really want to rename it?
   
   My original comment was about `AggregationWithHeadersSerializer`/`AggregationWithHeadersDeserializer`/`AggregationWithHeadersSerde` (I know that I did say "related classes"; sorry for the confusion)
   
   Would it be too odd to only rename the serde classes? Or would we want to update the KIP? Or disregard my original comment (might not have been a good suggestion after all...)
   
   \cc @aliehsaeedii @bbejeck 



-- 
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] MINOR: follow-up of KAFKA-20317 [kafka]

Posted by "frankvicky (via GitHub)" <gi...@apache.org>.
frankvicky commented on code in PR #21838:
URL: https://github.com/apache/kafka/pull/21838#discussion_r3044275472


##########
streams/integration-tests/src/test/java/org/apache/kafka/streams/integration/HeadersStoreUpgradeIntegrationTest.java:
##########
@@ -1830,21 +1830,21 @@ private void verifySessionValueWithEmptyHeaders(final String key,
                 if (processorRef.get() == null) {
                     return false;
                 }
-                final ReadOnlySessionStore<String, AggregationWithHeaders<String>> store = processorRef.get().store();
+                final ReadOnlySessionStore<String, ValueWithHeaders<String>> store = processorRef.get().store();

Review Comment:
   I think it would be confusing if we only rename the serde classes 🤔
   I am open to other options



-- 
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] MINOR: follow-up of KAFKA-20317 [kafka]

Posted by "DL1231 (via GitHub)" <gi...@apache.org>.
DL1231 commented on code in PR #21838:
URL: https://github.com/apache/kafka/pull/21838#discussion_r2972834306


##########
streams/src/test/java/org/apache/kafka/streams/state/internals/MeteredSessionStoreWithHeadersTest.java:
##########
@@ -158,16 +158,16 @@ private List<MetricName> storeMetrics() {
             .collect(Collectors.toList());
     }
 
-    private <AGG> Serde<AggregationWithHeaders<AGG>> createAggregationWithHeadersSerde(final Serde<AGG> aggSerde) {
+    private <AGG> Serde<AggregationWithHeaders<AGG>> createValueWithHeadersSerde(final Serde<AGG> aggSerde) {

Review Comment:
   AGG -> V



##########
streams/src/main/java/org/apache/kafka/streams/state/internals/ValueWithHeadersDeserializer.java:
##########
@@ -31,79 +31,79 @@
 import static org.apache.kafka.streams.state.internals.Utils.readBytes;
 
 /**
- * Deserializer for AggregationWithHeaders.
+ * Deserializer for values with headers.
  * Deserialization format (per KIP-1271):
- * [headersSize(varint)][headersBytes][aggregation]
+ * [headersSize(varint)][headersBytes][value]
  * <p>
  * Where:
  * - headersSize: Size of the headersBytes section in bytes, encoded as varint
  * - headersBytes:
  *   - For null/empty headers: headersSize = 0, headersBytes is omitted (0 bytes)
  *   - For non-empty headers: headersSize > 0, serialized headers in the format [count(varint)][header1][header2]... to be processed by HeadersDeserializer.
- * - aggregation: Serialized aggregation to be deserialized with the provided aggregation deserializer
+ * - value: Serialized value to be deserialized with the provided value deserializer
  * <p>
- * This is used by KIP-1271 to deserialize aggregations with headers from session state stores.
+ * This is used by KIP-1271 to deserialize values with headers from session and window state stores.
  */
-class AggregationWithHeadersDeserializer<AGG> implements WrappingNullableDeserializer<AggregationWithHeaders<AGG>, Void, AGG> {
+class ValueWithHeadersDeserializer<V> implements WrappingNullableDeserializer<AggregationWithHeaders<V>, Void, V> {
 
-    public final Deserializer<AGG> aggregationDeserializer;
+    public final Deserializer<V> valueDeserializer;
 
-    AggregationWithHeadersDeserializer(final Deserializer<AGG> aggregationDeserializer) {
-        Objects.requireNonNull(aggregationDeserializer);
-        this.aggregationDeserializer = aggregationDeserializer;
+    ValueWithHeadersDeserializer(final Deserializer<V> valueDeserializer) {
+        Objects.requireNonNull(valueDeserializer);
+        this.valueDeserializer = valueDeserializer;
     }
 
     @Override
     public void configure(final Map<String, ?> configs, final boolean isKey) {
-        aggregationDeserializer.configure(configs, isKey);
+        valueDeserializer.configure(configs, isKey);
     }
 
     @Override
-    public AggregationWithHeaders<AGG> deserialize(final String topic, final byte[] aggregationWithHeaders) {
-        if (aggregationWithHeaders == null) {
+    public AggregationWithHeaders<V> deserialize(final String topic, final byte[] valueWithHeaders) {
+        if (valueWithHeaders == null) {
             return null;
         }
 
-        final ByteBuffer buffer = ByteBuffer.wrap(aggregationWithHeaders);
+        final ByteBuffer buffer = ByteBuffer.wrap(valueWithHeaders);
         final Headers headers = readHeaders(buffer);
-        final byte[] rawAggregation = readBytes(buffer, buffer.remaining());
-        final AGG aggregation = aggregationDeserializer.deserialize(topic, headers, rawAggregation);
+        final byte[] rawValue = readBytes(buffer, buffer.remaining());
+        final V value = valueDeserializer.deserialize(topic, headers, rawValue);
 
-        return AggregationWithHeaders.makeAllowNullable(aggregation, headers);
+        return AggregationWithHeaders.makeAllowNullable(value, headers);
     }
 
     @Override
     public void close() {
-        aggregationDeserializer.close();
+        valueDeserializer.close();
     }
 
     @Override
     public void setIfUnset(final SerdeGetter getter) {
-        initNullableDeserializer(aggregationDeserializer, getter);
+        initNullableDeserializer(valueDeserializer, getter);
     }
 
     /**
-     * Extract headers from serialized AggregationWithHeaders.
+     * Extract headers from serialized value with headers.
      */
-    static Headers headers(final byte[] rawAggregationWithHeaders) {
-        if (rawAggregationWithHeaders == null) {
+    static Headers headers(final byte[] rawValueWithHeaders) {
+        if (rawValueWithHeaders == null) {
             return null;
         }
 
-        final ByteBuffer buffer = ByteBuffer.wrap(rawAggregationWithHeaders);
+        final ByteBuffer buffer = ByteBuffer.wrap(rawValueWithHeaders);
         return readHeaders(buffer);
     }
 
     /**
-     * Extract the raw aggregation bytes from serialized AggregationWithHeaders,
+     * Extract the raw value bytes from serialized value with headers,
      * stripping the headers prefix.
      */
-    static byte[] rawAggregation(final byte[] aggregationWithHeaders) {
-        if (aggregationWithHeaders == null) {
+    static byte[] rawAggregation(final byte[] valueWithHeaders) {

Review Comment:
   [KAFKA-20249](https://github.com/apache/kafka/pull/21706) moved `rawAggregation()` and `headers()` to `Utils.java` with performance improvements, please reuse them



##########
streams/src/test/java/org/apache/kafka/streams/state/internals/MeteredSessionStoreWithHeadersTest.java:
##########
@@ -158,16 +158,16 @@ private List<MetricName> storeMetrics() {
             .collect(Collectors.toList());
     }
 
-    private <AGG> Serde<AggregationWithHeaders<AGG>> createAggregationWithHeadersSerde(final Serde<AGG> aggSerde) {
+    private <AGG> Serde<AggregationWithHeaders<AGG>> createValueWithHeadersSerde(final Serde<AGG> aggSerde) {
         return new Serde<>() {
             @Override
             public Serializer<AggregationWithHeaders<AGG>> serializer() {

Review Comment:
   Ditto



##########
streams/src/main/java/org/apache/kafka/streams/state/internals/SessionToHeadersStoreAdapter.java:
##########
@@ -138,7 +138,7 @@ public void remove(final Windowed<Bytes> sessionKey) {
 
     @Override
     public void put(final Windowed<Bytes> sessionKey, final byte[] aggregateWithHeader) {
-        store.put(sessionKey, AggregationWithHeadersDeserializer.rawAggregation(aggregateWithHeader));
+        store.put(sessionKey, ValueWithHeadersDeserializer.rawAggregation(aggregateWithHeader));

Review Comment:
   Ditto



##########
streams/src/main/java/org/apache/kafka/streams/state/internals/ValueWithHeadersDeserializer.java:
##########
@@ -31,79 +31,79 @@
 import static org.apache.kafka.streams.state.internals.Utils.readBytes;
 
 /**
- * Deserializer for AggregationWithHeaders.
+ * Deserializer for values with headers.
  * Deserialization format (per KIP-1271):
- * [headersSize(varint)][headersBytes][aggregation]
+ * [headersSize(varint)][headersBytes][value]
  * <p>
  * Where:
  * - headersSize: Size of the headersBytes section in bytes, encoded as varint
  * - headersBytes:
  *   - For null/empty headers: headersSize = 0, headersBytes is omitted (0 bytes)
  *   - For non-empty headers: headersSize > 0, serialized headers in the format [count(varint)][header1][header2]... to be processed by HeadersDeserializer.
- * - aggregation: Serialized aggregation to be deserialized with the provided aggregation deserializer
+ * - value: Serialized value to be deserialized with the provided value deserializer
  * <p>
- * This is used by KIP-1271 to deserialize aggregations with headers from session state stores.
+ * This is used by KIP-1271 to deserialize values with headers from session and window state stores.
  */
-class AggregationWithHeadersDeserializer<AGG> implements WrappingNullableDeserializer<AggregationWithHeaders<AGG>, Void, AGG> {
+class ValueWithHeadersDeserializer<V> implements WrappingNullableDeserializer<AggregationWithHeaders<V>, Void, V> {
 
-    public final Deserializer<AGG> aggregationDeserializer;
+    public final Deserializer<V> valueDeserializer;
 
-    AggregationWithHeadersDeserializer(final Deserializer<AGG> aggregationDeserializer) {
-        Objects.requireNonNull(aggregationDeserializer);
-        this.aggregationDeserializer = aggregationDeserializer;
+    ValueWithHeadersDeserializer(final Deserializer<V> valueDeserializer) {
+        Objects.requireNonNull(valueDeserializer);
+        this.valueDeserializer = valueDeserializer;
     }
 
     @Override
     public void configure(final Map<String, ?> configs, final boolean isKey) {
-        aggregationDeserializer.configure(configs, isKey);
+        valueDeserializer.configure(configs, isKey);
     }
 
     @Override
-    public AggregationWithHeaders<AGG> deserialize(final String topic, final byte[] aggregationWithHeaders) {
-        if (aggregationWithHeaders == null) {
+    public AggregationWithHeaders<V> deserialize(final String topic, final byte[] valueWithHeaders) {
+        if (valueWithHeaders == null) {
             return null;
         }
 
-        final ByteBuffer buffer = ByteBuffer.wrap(aggregationWithHeaders);
+        final ByteBuffer buffer = ByteBuffer.wrap(valueWithHeaders);
         final Headers headers = readHeaders(buffer);
-        final byte[] rawAggregation = readBytes(buffer, buffer.remaining());
-        final AGG aggregation = aggregationDeserializer.deserialize(topic, headers, rawAggregation);
+        final byte[] rawValue = readBytes(buffer, buffer.remaining());
+        final V value = valueDeserializer.deserialize(topic, headers, rawValue);
 
-        return AggregationWithHeaders.makeAllowNullable(aggregation, headers);
+        return AggregationWithHeaders.makeAllowNullable(value, headers);
     }
 
     @Override
     public void close() {
-        aggregationDeserializer.close();
+        valueDeserializer.close();
     }
 
     @Override
     public void setIfUnset(final SerdeGetter getter) {
-        initNullableDeserializer(aggregationDeserializer, getter);
+        initNullableDeserializer(valueDeserializer, getter);
     }
 
     /**
-     * Extract headers from serialized AggregationWithHeaders.
+     * Extract headers from serialized value with headers.
      */
-    static Headers headers(final byte[] rawAggregationWithHeaders) {
-        if (rawAggregationWithHeaders == null) {
+    static Headers headers(final byte[] rawValueWithHeaders) {
+        if (rawValueWithHeaders == null) {
             return null;
         }
 
-        final ByteBuffer buffer = ByteBuffer.wrap(rawAggregationWithHeaders);
+        final ByteBuffer buffer = ByteBuffer.wrap(rawValueWithHeaders);
         return readHeaders(buffer);
     }
 
     /**
-     * Extract the raw aggregation bytes from serialized AggregationWithHeaders,
+     * Extract the raw value bytes from serialized value with headers,
      * stripping the headers prefix.
      */
-    static byte[] rawAggregation(final byte[] aggregationWithHeaders) {
-        if (aggregationWithHeaders == null) {
+    static byte[] rawAggregation(final byte[] valueWithHeaders) {

Review Comment:
   Nit: The class was renamed from `AggregationWithHeaders*` to `ValueWithHeaders*`, but the static method is still called `rawAggregation()`. Should this be `rawValue()` for consistency? 
   Same for the `AggregationWithHeaders` wrapper class itself, could be renamed to `ValueWithHeaders`.



-- 
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] MINOR: follow-up of KAFKA-20317 [kafka]

Posted by "frankvicky (via GitHub)" <gi...@apache.org>.
frankvicky closed pull request #21838: MINOR: follow-up of KAFKA-20317
URL: https://github.com/apache/kafka/pull/21838


-- 
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