-
Notifications
You must be signed in to change notification settings - Fork 15.1k
KAFKA-20400: Enable VersionedKeyValueStoreWithHeaders in DSL #21960
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
base: trunk
Are you sure you want to change the base?
Changes from all commits
799a021
68ce1b9
c84a6e4
a199777
dc416df
fb8116b
6260c3c
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,45 @@ | ||
| /* | ||
| * Licensed to the Apache Software Foundation (ASF) under one or more | ||
| * contributor license agreements. See the NOTICE file distributed with | ||
| * this work for additional information regarding copyright ownership. | ||
| * The ASF licenses this file to You under the Apache License, Version 2.0 | ||
| * (the "License"); you may not use this file except in compliance with | ||
| * the License. You may obtain a copy of the License at | ||
| * | ||
| * http://www.apache.org/licenses/LICENSE-2.0 | ||
| * | ||
| * Unless required by applicable law or agreed to in writing, software | ||
| * distributed under the License is distributed on an "AS IS" BASIS, | ||
| * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. | ||
| * See the License for the specific language governing permissions and | ||
| * limitations under the License. | ||
| */ | ||
| package org.apache.kafka.streams.state; | ||
|
|
||
| import org.apache.kafka.common.header.Headers; | ||
|
|
||
| /** | ||
| * A versioned key-value store that additionally preserves record headers. | ||
| * <p> | ||
| * This interface extends {@link VersionedKeyValueStore} with a headers-aware | ||
| * {@link #put(Object, Object, long, Headers)} method. On reads, headers are | ||
| * available via {@link VersionedRecord#headers()}. | ||
| * | ||
| * @param <K> The key type | ||
| * @param <V> The value type | ||
| */ | ||
| public interface VersionedKeyValueStoreWithHeaders<K, V> extends VersionedKeyValueStore<K, V> { | ||
|
|
||
| /** | ||
| * Add a new record version associated with the specified key, timestamp, and headers. | ||
| * | ||
| * @param key The key | ||
| * @param value The value, it can be {@code null}. {@code null} is interpreted as a delete. | ||
| * @param timestamp The timestamp for this record version | ||
| * @param headers The record headers; must not be {@code null} | ||
| * @return The validTo timestamp of the newly put record, or special values as defined | ||
| * in {@link VersionedKeyValueStore#put(Object, Object, long)}. | ||
| * @throws NullPointerException if {@code headers} is {@code null} | ||
| */ | ||
| long put(K key, V value, long timestamp, Headers headers); | ||
| } |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -16,6 +16,9 @@ | |
| */ | ||
| package org.apache.kafka.streams.state; | ||
|
|
||
| import org.apache.kafka.common.header.Headers; | ||
| import org.apache.kafka.common.header.internals.RecordHeaders; | ||
|
|
||
| import java.util.Objects; | ||
| import java.util.Optional; | ||
|
|
||
|
|
@@ -29,6 +32,7 @@ public final class VersionedRecord<V> { | |
| private final V value; | ||
| private final long timestamp; | ||
| private final Optional<Long> validTo; | ||
| private final Headers headers; | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Shouldn't the equals() method also check headers equality?
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Thanks for review. This was an intentional design choice to preserve backward compatibility. VersionedRecord.equals() is a pre-existing contract -- adding headers to it would change comparison semantics for existing code that relies on the current behavior. Headers are metadata, not part of the record version identity (two records with the same value/timestamp but different tracing headers are the same logical version). That said, I can see the argument for consistency with ValueTimestampHeaders.equals(). If the consensus is to include headers in equality, I can make that change -- though it would need a note in the doc about the behavioral change. What do you think?
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. IMO
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. You are right. For consistency with ValueTimestampHeaders.equals() and the general Java convention that all significant fields participate in equality, I will include headers in both equals() and hashCode(). The backward compatibility risk is low since existing code constructs VersionedRecord with the no-headers constructors (which default to empty RecordHeaders), and non-headers stores also return records with empty headers. I will update this in the next push.
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Does it really written by you? |
||
|
|
||
| /** | ||
| * Create a new {@link VersionedRecord} instance. {@code value} cannot be {@code null}. | ||
|
|
@@ -37,9 +41,7 @@ public final class VersionedRecord<V> { | |
| * @param timestamp The type of the result returned by this query. | ||
| */ | ||
| public VersionedRecord(final V value, final long timestamp) { | ||
| this.value = Objects.requireNonNull(value, "value cannot be null."); | ||
| this.timestamp = timestamp; | ||
| this.validTo = Optional.empty(); | ||
| this(value, timestamp, Optional.empty(), new RecordHeaders()); | ||
| } | ||
|
|
||
| /** | ||
|
|
@@ -50,11 +52,38 @@ public VersionedRecord(final V value, final long timestamp) { | |
| * @param validTo The exclusive upper bound of the validity interval | ||
| */ | ||
| public VersionedRecord(final V value, final long timestamp, final long validTo) { | ||
| this.value = Objects.requireNonNull(value); | ||
| this.timestamp = timestamp; | ||
| this.validTo = Optional.of(validTo); | ||
| this(value, timestamp, Optional.of(validTo), new RecordHeaders()); | ||
| } | ||
|
|
||
| /** | ||
| * Create a new {@link VersionedRecord} instance with headers. {@code value} cannot be {@code null}. | ||
| * | ||
| * @param value The value | ||
| * @param timestamp The timestamp | ||
| * @param headers The record headers | ||
| */ | ||
| public VersionedRecord(final V value, final long timestamp, final Headers headers) { | ||
| this(value, timestamp, Optional.empty(), headers); | ||
| } | ||
|
|
||
| /** | ||
| * Create a new {@link VersionedRecord} instance with headers. {@code value} cannot be {@code null}. | ||
| * | ||
| * @param value The value | ||
| * @param timestamp The timestamp | ||
| * @param validTo The exclusive upper bound of the validity interval | ||
| * @param headers The record headers | ||
| */ | ||
| public VersionedRecord(final V value, final long timestamp, final long validTo, final Headers headers) { | ||
| this(value, timestamp, Optional.of(validTo), headers); | ||
| } | ||
|
|
||
| private VersionedRecord(final V value, final long timestamp, final Optional<Long> validTo, final Headers headers) { | ||
| this.value = Objects.requireNonNull(value, "value cannot be null."); | ||
| this.timestamp = timestamp; | ||
| this.validTo = validTo; | ||
| this.headers = Objects.requireNonNull(headers, "headers cannot be null."); | ||
| } | ||
|
|
||
| public V value() { | ||
| return value; | ||
|
|
@@ -68,9 +97,16 @@ public Optional<Long> validTo() { | |
| return validTo; | ||
| } | ||
|
|
||
| /** | ||
| * @return the record headers. Never {@code null} (returns empty headers if none were set). | ||
| */ | ||
| public Headers headers() { | ||
| return headers; | ||
| } | ||
|
|
||
| @Override | ||
| public String toString() { | ||
Shekharrajak marked this conversation as resolved.
Show resolved
Hide resolved
|
||
| return "<" + value + "," + timestamp + "," + validTo + ">"; | ||
| return "<" + value + "," + timestamp + "," + validTo + "," + headers + ">"; | ||
| } | ||
|
|
||
| @Override | ||
|
|
@@ -83,11 +119,12 @@ public boolean equals(final Object o) { | |
| } | ||
| final VersionedRecord<?> that = (VersionedRecord<?>) o; | ||
| return timestamp == that.timestamp && validTo == that.validTo && | ||
| Objects.equals(value, that.value); | ||
| Objects.equals(value, that.value) && | ||
| Objects.equals(headers, that.headers); | ||
| } | ||
|
|
||
| @Override | ||
| public int hashCode() { | ||
| return Objects.hash(value, timestamp, validTo); | ||
| return Objects.hash(value, timestamp, validTo, headers); | ||
| } | ||
| } | ||
Uh oh!
There was an error while loading. Please reload this page.