The fix will definitely "facilitate" the source-connectors I have
written. It will make them work 100% correctly. Today they dont.
Fine for me to change "Acknowledged" to "Acked" in the method-naming.
Not sure I would like to give a Collection instead of a List as the
argument to offstesFlushedAndAck(nowledg)ed. poll() returns a List
(ordered records), the records are handled in that order and I would
like to hand the records back in that order as well. Handling back a
Collection may indicate that order does not matter. Besides that it is
likely to help the implementation of offstesFlushedAndAck(nowledg)ed
that you get records back in order.
Regarding adding stuff to the "rejected approaches" and "motivation"
sections of the KIP, I am not sure I will get the time anytime soon.
Please feel free to help adding this to the KIP. This way we also have
at least two persons who really understands what this is about. Some
times you only really understand what something is about, when you are
forced to write about it (at least that is my excuse ).
Regards, Per Steffensen
On 16/10/2018 05.57, Konstantine Karantasis wrote:
This is a significant improvement to the semantics of source connectors.
I'm expecting that it will facilitate source connector implementations and
even enrich the application uses cases that we see. I only have a few minor
suggestions at the moment.
I believe that Acked is a common abbreviation for Acknowledged and that we
could use it in this context. And this suggestion is coming from a big
proponent of complete words in variables and method names. Thus, feel free
to consider 'offsetsFlushedAndAcked' as well as 'recordSentAndAcked'. Since
this is a public interface, I'd also make the implementation specific
comment that a Collection<SourceRecord> might be more versatile than
List<SourceRecord> as argument in offsetsFlushedAndAcknowledged.
The rejected approaches section could use some of the material in the
original jira ticket, which is pretty insightful in order to understand how
we arrived to this KIP. For example, I think it'd be useful to state
briefly why the 'commit' method is not getting removed completely but it's
substituted with 'offsetsFlushedAndAcked'. Also useful I believe it would
be to include in the motivation section some info related to why and how a
source system could use these method calls to safely recycle data that have
been surely imported to Kafka. I see this type of use cases having an
increased importance as Kafka is used more and more as the source of truth
and persistence layer for an increasing number of applications.
These suggestions, although they are not strictly required in order to move
forward with this improvement, I believe can help a lot to understand the
context of this proposed changed, without having to read the complete
history in the jira ticket.
Thanks for the KIP Per!
-Konstantine
On Wed, Oct 10, 2018 at 6:50 AM Per Steffensen <perst...@gmail.com> wrote:
Please help make the proposed changes in KIP-381 become reality. Please
comment.
KIP:
https://cwiki.apache.org/confluence/display/KAFKA/KIP-381%3A+Connect%3A+Tell+about+records+that+had+their+offsets+flushed+in+callback
JIRA: https://issues.apache.org/jira/browse/KAFKA-5716
PR: https://github.com/apache/kafka/pull/3872
Thanks!
