KAFKA-12749: Changelog topic config on suppressed KTable lost #10664
Conversation
|
@ableegoldman Would appreciate it if you can take a look at this PR when you get a chance. |
|
cc any of @cadonna @vvcephei @lct45 @wcarlson5 @mjsax @guozhangwang to review this |
| @@ -46,13 +48,13 @@ public void bufferBuilderShouldBeConsistent() { | |||
| assertThat( | |||
| "keys alone should be set", | |||
| maxRecords(2L), | |||
| is(new EagerBufferConfigImpl(2L, MAX_VALUE)) | |||
| is(new EagerBufferConfigImpl(2L, MAX_VALUE, Collections.emptyMap())) | |||
There was a problem hiding this comment.
@vichu Can you add some tests that don't just use an empty map?
There was a problem hiding this comment.
Sure.
Added a few tests to verify the logConfig map as well.
…art of the equals, hashCode & toString methods.
…ion is still set after the calls to withMaxBytes & withMaxRecords are applied.
|
Sorry, this got lost in my inbox. @vichu if you can rebase I think its good to go. do you agree @ableegoldman ? |
# Conflicts: # streams/streams-scala/src/main/scala/org/apache/kafka/streams/scala/kstream/Suppressed.scala # streams/streams-scala/src/test/scala/org/apache/kafka/streams/scala/kstream/SuppressedTest.scala
...src/main/java/org/apache/kafka/streams/kstream/internals/suppress/EagerBufferConfigImpl.java
Outdated
Show resolved
Hide resolved
…ls/suppress/EagerBufferConfigImpl.java
|
@vichu I took the liberty to commit the fix to the nit directly. I hope that is fine with you. |
|
I will merge as soon as the checks are done. |
|
The 7th build was green on the same commit. I do not know why a 8th build was started. |
Refactored
logConfigto be passed appropriately when usingshutDownWhenFulloremitEarlyWhenFull. Removed the constructor that doesn't accept alogConfigparameter so you're forced to specify it explicitly, whether it's empty/unspecified or not.Ticket: https://issues.apache.org/jira/browse/KAFKA-12749
Committer Checklist (excluded from commit message)