KAFKA-5567: Connect sink worker should commit offsets of original topic partitions #3499
Conversation
kkonstantine
changed the title
kkonstantine
force-pushed
the
KAFKA-5567-With-transformations-that-mutate-the-topic-partition-committing-offsets-should-to-refer-to-the-original-topic-partition
branch
from
b9eeeb1
to
7c6f223
Compare
|
Refer to this link for build results (access rights to CI server needed): |
|
Refer to this link for build results (access rights to CI server needed): |
| if (record != null) { | ||
| messageBatch.add(record); | ||
| SinkRecord transRecord = transformationChain.apply(origRecord); | ||
| if (transRecord != null) { |
There was a problem hiding this comment.
Even if we filter it out, wouldn't we want to include its offset? We did process it, the transformation just chose not to bother passing it to the connector, so it seems safe to commit the offset.
There was a problem hiding this comment.
I considered this as an optimization. In any case, it was already there since the initial commit.
I'm inclined to commit all the offsets.
kkonstantine
force-pushed
the
KAFKA-5567-With-transformations-that-mutate-the-topic-partition-committing-offsets-should-to-refer-to-the-original-topic-partition
branch
from
7c6f223
to
cf7718e
Compare
|
Refer to this link for build results (access rights to CI server needed): |
|
Refer to this link for build results (access rights to CI server needed): |
|
@kkonstantine, what's the status of this? @gwenshap would like this backported for the 0.11.0.1 release after it is merged to trunk. |
|
Refer to this link for build results (access rights to CI server needed): |
|
The JDK 8 PR builds have been failing lately. See INFRA-14882 |
|
Refer to this link for build results (access rights to CI server needed): |
kkonstantine
force-pushed
the
KAFKA-5567-With-transformations-that-mutate-the-topic-partition-committing-offsets-should-to-refer-to-the-original-topic-partition
branch
from
cf7718e
to
bf9dc95
Compare
kkonstantine
changed the title
|
@ewencp ready for final review. |
|
Refer to this link for build results (access rights to CI server needed): |
|
Refer to this link for build results (access rights to CI server needed): |
kkonstantine
force-pushed
the
KAFKA-5567-With-transformations-that-mutate-the-topic-partition-committing-offsets-should-to-refer-to-the-original-topic-partition
branch
from
bf9dc95
to
2b506cd
Compare
|
Great, thanks! The one on JDK8 failed with the known issue (Proxy Error ... Reason: Error reading from remote server). Waiting for the JDK7 one. |
|
Refer to this link for build results (access rights to CI server needed): |
|
Refer to this link for build results (access rights to CI server needed): |
…ic partitions Author: Konstantine Karantasis <konstantine@confluent.io> Reviewers: Randall Hauch <rhauch@gmail.com>, Ewen Cheslack-Postava <ewen@confluent.io> Closes #3499 from kkonstantine/KAFKA-5567-With-transformations-that-mutate-the-topic-partition-committing-offsets-should-to-refer-to-the-original-topic-partition (cherry picked from commit 72eacbe) Signed-off-by: Ewen Cheslack-Postava <me@ewencp.org>
|
What happened to this PR, why was it closed? This warn log shows up a lot so if this is a solution, would love to have it merged! |
|
Ah, I got a bit confused by the merge policy here that it did in fact get merged with the commit 72eacbe but this PR shows as closed not merged. Got it now, thanks. |
No description provided.