-
Notifications
You must be signed in to change notification settings - Fork 14.3k
KAFKA-16260: Deprecate window.size.ms and window.inner.class.serde in StreamsConfig #18297
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
Changes from 2 commits
f0700e5
d0608e5
457ba7e
4283072
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 | ||||
---|---|---|---|---|---|---|
|
@@ -430,6 +430,9 @@ <h5><a id="upgrade_servers_400_notable" href="#upgrade_servers_400_notable">Nota | |||||
<li> See <a href="https://cwiki.apache.org/confluence/x/B40ODg">KIP-890</a> and | ||||||
<a href="https://cwiki.apache.org/confluence/x/8ItyEg">KIP-1050</a> for more details </li> | ||||||
</ul> | ||||||
<li> | ||||||
The <code>window.size.ms</code> and <code>window.inner.serde.class</code> in stream config are deprecated. | ||||||
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.
Suggested change
Something like that? Could also be the actual string constants. But it's good to make these notes actionable. |
||||||
</li> | ||||||
</ul> | ||||||
</li> | ||||||
</ul> | ||||||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -45,6 +45,10 @@ | |
import org.apache.kafka.streams.errors.StreamsException; | ||
import org.apache.kafka.streams.internals.StreamsConfigUtils; | ||
import org.apache.kafka.streams.internals.UpgradeFromValues; | ||
import org.apache.kafka.streams.kstream.SessionWindowedDeserializer; | ||
import org.apache.kafka.streams.kstream.SessionWindowedSerializer; | ||
import org.apache.kafka.streams.kstream.TimeWindowedDeserializer; | ||
import org.apache.kafka.streams.kstream.TimeWindowedSerializer; | ||
import org.apache.kafka.streams.processor.FailOnInvalidTimestamp; | ||
import org.apache.kafka.streams.processor.TimestampExtractor; | ||
import org.apache.kafka.streams.processor.assignment.TaskAssignor; | ||
|
@@ -823,13 +827,28 @@ public class StreamsConfig extends AbstractConfig { | |
+ CONFIG_ERROR_MSG | ||
+ "\"NO_OPTIMIZATION\" by default."; | ||
|
||
/** {@code windowed.inner.class.serde} */ | ||
/** | ||
* {@code windowed.inner.class.serde} | ||
* | ||
* @deprecated since 4.0.0. | ||
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. 4.1.0 |
||
* Use {@link TimeWindowedSerializer#WINDOWED_INNER_SERIALIZER_CLASS} for {@link TimeWindowedSerializer}. | ||
* Use {@link TimeWindowedDeserializer#WINDOWED_INNER_DESERIALIZER_CLASS} for {@link TimeWindowedDeserializer}. | ||
* Use {@link SessionWindowedSerializer#WINDOWED_INNER_SERIALIZER_CLASS} for {@link SessionWindowedSerializer}. | ||
* Use {@link SessionWindowedDeserializer#WINDOWED_INNER_DESERIALIZER_CLASS} for {@link SessionWindowedDeserializer}. | ||
*/ | ||
@Deprecated | ||
public static final String WINDOWED_INNER_CLASS_SERDE = "windowed.inner.class.serde"; | ||
private static final String WINDOWED_INNER_CLASS_SERDE_DOC = " Default serializer / deserializer for the inner class of a windowed record. Must implement the " + | ||
"<code>org.apache.kafka.common.serialization.Serde</code> interface. Note that setting this config in KafkaStreams application would result " + | ||
"in an error as it is meant to be used only from Plain consumer client."; | ||
|
||
/** {@code window.size.ms} */ | ||
/** | ||
* {@code window.size.ms} | ||
* | ||
* @deprecated since 4.0.0. | ||
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. 4.1.0 |
||
* Use {@link TimeWindowedDeserializer#WINDOW_SIZE_MS_CONFIG} for {@link TimeWindowedDeserializer}. | ||
*/ | ||
@Deprecated | ||
public static final String WINDOW_SIZE_MS_CONFIG = "window.size.ms"; | ||
private static final String WINDOW_SIZE_MS_DOC = "Sets window size for the deserializer in order to calculate window end times."; | ||
|
||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -23,10 +23,20 @@ | |
import org.apache.kafka.streams.StreamsConfig; | ||
import org.apache.kafka.streams.state.internals.SessionKeySchema; | ||
|
||
import org.slf4j.Logger; | ||
import org.slf4j.LoggerFactory; | ||
|
||
import java.util.Map; | ||
|
||
public class SessionWindowedDeserializer<T> implements Deserializer<Windowed<T>> { | ||
|
||
/** | ||
* Configuration key for the windowed inner deserializer class. | ||
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. Can we put any more information into this (and the other) java doc comments about the constants? Since this is a public string constant now, the use of the constant should ideally be clear from the javadoc comments. |
||
*/ | ||
public static final String WINDOWED_INNER_DESERIALIZER_CLASS = "windowed.inner.deserializer.class"; | ||
|
||
private final Logger log = LoggerFactory.getLogger(SessionWindowedDeserializer.class); | ||
|
||
private Deserializer<T> inner; | ||
|
||
// Default constructor needed by Kafka | ||
|
@@ -36,34 +46,43 @@ public SessionWindowedDeserializer(final Deserializer<T> inner) { | |
this.inner = inner; | ||
} | ||
|
||
@SuppressWarnings("unchecked") | ||
@SuppressWarnings({"deprecation", "unchecked"}) | ||
@Override | ||
public void configure(final Map<String, ?> configs, final boolean isKey) { | ||
final String windowedInnerClassSerdeConfig = (String) configs.get(StreamsConfig.WINDOWED_INNER_CLASS_SERDE); | ||
|
||
Serde<T> windowInnerClassSerde = null; | ||
String deserializerConfigFrom = WINDOWED_INNER_DESERIALIZER_CLASS; | ||
String windowedInnerDeserializerClassConfig = (String) configs.get(WINDOWED_INNER_DESERIALIZER_CLASS); | ||
if (windowedInnerDeserializerClassConfig == null) { | ||
final String windowedInnerClassSerdeConfig = (String) configs.get(StreamsConfig.WINDOWED_INNER_CLASS_SERDE); | ||
if (windowedInnerClassSerdeConfig != null) { | ||
deserializerConfigFrom = StreamsConfig.WINDOWED_INNER_CLASS_SERDE; | ||
windowedInnerDeserializerClassConfig = windowedInnerClassSerdeConfig; | ||
log.warn("Config {} is deprecated. Please use {} instead.", | ||
StreamsConfig.WINDOWED_INNER_CLASS_SERDE, WINDOWED_INNER_DESERIALIZER_CLASS); | ||
} | ||
} | ||
|
||
if (windowedInnerClassSerdeConfig != null) { | ||
Serde<T> windowedInnerDeserializerClass = null; | ||
if (windowedInnerDeserializerClassConfig != null) { | ||
try { | ||
windowInnerClassSerde = Utils.newInstance(windowedInnerClassSerdeConfig, Serde.class); | ||
windowedInnerDeserializerClass = Utils.newInstance(windowedInnerDeserializerClassConfig, Serde.class); | ||
} catch (final ClassNotFoundException e) { | ||
throw new ConfigException(StreamsConfig.WINDOWED_INNER_CLASS_SERDE, windowedInnerClassSerdeConfig, | ||
"Serde class " + windowedInnerClassSerdeConfig + " could not be found."); | ||
throw new ConfigException(deserializerConfigFrom, windowedInnerDeserializerClassConfig, | ||
"Serde class " + windowedInnerDeserializerClassConfig + " could not be found."); | ||
} | ||
} | ||
|
||
if (inner != null && windowedInnerClassSerdeConfig != null) { | ||
if (!inner.getClass().getName().equals(windowInnerClassSerde.deserializer().getClass().getName())) { | ||
if (inner != null && windowedInnerDeserializerClassConfig != null) { | ||
if (!inner.getClass().getName().equals(windowedInnerDeserializerClass.deserializer().getClass().getName())) { | ||
throw new IllegalArgumentException("Inner class deserializer set using constructor " | ||
+ "(" + inner.getClass().getName() + ")" + | ||
" is different from the one set in windowed.inner.class.serde config " + | ||
"(" + windowInnerClassSerde.deserializer().getClass().getName() + ")."); | ||
" is different from the one set in " + deserializerConfigFrom + " config " + | ||
"(" + windowedInnerDeserializerClass.deserializer().getClass().getName() + ")."); | ||
} | ||
} else if (inner == null && windowedInnerClassSerdeConfig == null) { | ||
} else if (inner == null && windowedInnerDeserializerClassConfig == null) { | ||
throw new IllegalArgumentException("Inner class deserializer should be set either via constructor " + | ||
"or via the windowed.inner.class.serde config"); | ||
"or via the " + WINDOWED_INNER_DESERIALIZER_CLASS + " config"); | ||
} else if (inner == null) | ||
inner = windowInnerClassSerde.deserializer(); | ||
inner = windowedInnerDeserializerClass.deserializer(); | ||
} | ||
|
||
@Override | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This should not be in the 4.0 section.