[ 
https://issues.apache.org/jira/browse/KAFKA-12313?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17303843#comment-17303843
 ] 

Sagar Rao edited comment on KAFKA-12313 at 3/18/21, 4:08 AM:
-------------------------------------------------------------

Haha my bad i should have been more elaborate about "it" :D. Your suggestions 
make perfect sense to me. Summarising them one final time here(quoting your 
rules from the last comment) :
 # Deprecate inner class key/value configs. Replace with one deserialiser 
config (windowed.deserializer.inner.class) which basically represents a 
WindowedKey.
 # 
{noformat}
For the console-consumer, you have to pass in both parameters via the 
configs{noformat}
Console consumer to invoke the default constructor of TimeWindowedDeserializer 
and we will ensure that both parameters(Deserialiser and windowSize) configs 
are passed.

 # 
{noformat}
For any other plain consumer client, you can pass them in as configs OR pass 
the parameters to the TimeWindowedDeserializer construct, and then pass that 
object to the consumer. I guess it doesn't hurt if a user supplies BOTH the 
configs and an actual Deserializer, as long as the parameters match{noformat}
A normal consumer client can use either of the 2 constructors. We have to 
ensure that both the deserialiser and windowsize are set and there are no 
conflicts amongst them(similar to how windowSize is being validated).

 # 
{noformat}
For use in Kafka Streams (such as the DSL), you must supply the parameters by 
constructing a TimeWindowedSerde and passing that in as a parameter to any 
relevant DSL operators{noformat}
Here I believe the way the TemperatureDemo class creates the WindowSerde is the 
way that users can use it in the DSL. 
{code:java}
final Serde<Windowed<String>> windowedSerde = 
WindowedSerdes.timeWindowedSerdeFrom(String.class, 
TEMPERATURE_WINDOW_SIZE);{code}
So, this is already being taken care of so I don't expect any changes here.

If you can confirm these, then I can start with the next steps. BTW, the label 
on the ticket says `needs-kip`. If the summary above looks fine to you, then 
should I go ahead with the KIP creation?


was (Author: sagarrao):
Haha my bad i should have been more elaborate about "it" :D. Your suggestions 
make perfect sense to me. Summarising them one final time here(quoting your 
rules from the last comment) :
 # Deprecate inner class key/value configs. Replace with one deserialiser 
config (windowed.deserializer.inner.class) which basically represents a 
WindowedKey.
 # 
{noformat}
For the console-consumer, you have to pass in both parameters via the 
configs{noformat}
Console consumer to invoke the default constructor of TimeWindowedDeserializer 
and we will ensure that both parameters(Deserialiser and windowSize) configs 
are passed.
 # 
{noformat}
For any other plain consumer client, you can pass them in as configs OR pass 
the parameters to the TimeWindowedDeserializer construct, and then pass that 
object to the consumer. I guess it doesn't hurt if a user supplies BOTH the 
configs and an actual Deserializer, as long as the parameters match{noformat}
A normal consumer client can use either of the 2 constructors. We have to 
ensure that both the deserialiser and windowsize are set and there is no 
conflicts amongst them(similar to how windowSize is being validated).
 # 
{noformat}
For use in Kafka Streams (such as the DSL), you must supply the parameters by 
constructing a TimeWindowedSerde and passing that in as a parameter to any 
relevant DSL operators{noformat}
Here I believe the way the TemperatureDemo class creates the WindowSerde is the 
way that users can use it in the DSL. 
{code:java}
final Serde<Windowed<String>> windowedSerde = 
WindowedSerdes.timeWindowedSerdeFrom(String.class, 
TEMPERATURE_WINDOW_SIZE);{code}
So, this is already being taken care of so I don't expect any changes here.

If you can confirm these, then I can start with the next steps. BTW, the label 
on the ticket says `needs-kip`. If the summary above looks fine to you, then 
should I go ahead with the KIP creation?

> Consider deprecating the default.windowed.serde.inner.class configs
> -------------------------------------------------------------------
>
>                 Key: KAFKA-12313
>                 URL: https://issues.apache.org/jira/browse/KAFKA-12313
>             Project: Kafka
>          Issue Type: Improvement
>          Components: streams
>            Reporter: A. Sophie Blee-Goldman
>            Assignee: Sagar Rao
>            Priority: Major
>              Labels: needs-kip
>             Fix For: 3.0.0
>
>
> During the discussion of KIP-659 we discussed whether it made sense to have a 
> "default" class for the serdes of windowed inner classes across Streams. 
> Using these configs instead of specifying an actual Serde object can lead to 
> subtle bugs, since the WindowedDeserializer requires a windowSize in addition 
> to the inner class. If the default constructor is invoked, as it will be when 
> falling back on the config, this windowSize defaults to MAX_VALUE. 
> If the downstream program doesn't care about the window end time in the 
> output, then this can go unnoticed and technically there is no problem. But 
> if anything does depend on the end time, or the user just wants to manually 
> read the output for testing purposes, then the MAX_VALUE will result in a 
> garbage timestamp.
> We should consider whether the convenience of specifying a config instead of 
> instantiating a Serde in each operator is really worth the risk of a user 
> accidentally failing to specify a windowSize



--
This message was sent by Atlassian Jira
(v8.3.4#803005)

Reply via email to