Bug fix for KAFKA-13927

[Jordan Bull]

Hello,

I have a relatively simple fix for the bug
 reported in KAFKA-13927
 in which Connect will
not commit offsets for a message that a SinkTask retries and eventually
succeeds. Would appreciate eyes or feedback on.

Also the CI appears not to succeed for the trunk base (and other branches
right now); is that correct? I've added test assertions for this bug and
verified that the unit tests for the SinkTask do succeed. Curious what is
the course of action if tests do not pass in general for the base branch?

Thanks,
Jordan

Re: [Discuss] KIP-803: Add Task ID and Connector Name to Connect Task Context

[Jordan Bull]

Hi all,

Would like to chime in with my interest in seeing this change! At the risk
of getting a bit off topic, I had been planning to propose a similar KIP,
but for exposing the connector name and task ID in converters and
transformers as they run into the same issue of not being able to easily
denote the task any logging and telemetry they generate should be
associated with. Any change to expose this info to the task should also be
the method for exposing them to the converters and transformers (if
approved). The reason this becomes particularly relevant to this
conversation, is that the workaround that Chris mentioned in his second
bullet point cannot be performed by the connector for converters and
transformers in all cases since converters are sometimes instantiated using
the worker config and other times using the connector config. I don't want
to derail this KIP with a discussion around exposing these fields to
converters and transformers, but I think it's worth considering if
advocating the workaround would block these other use-cases.

Thanks for the KIP Sarah (and Ryanne in 438)!
Jordan

On Tue, Dec 7, 2021 at 2:27 PM Chris Egerton 
wrote:

> Hi Sarah,
>
> Thanks for the KIP! I have two major thoughts:
>
> 1. Adding new methods like this to the Connect API comes with the risk that
> connectors that invoke them become incompatible with older versions of
> Connect. For example, if I updated my connector to use the newly-proposed
> SourceTaskContext::connector method, it would fail with a NoSuchMethodError
> when run on Connect version 3.1 (which will not have this feature). We've
> been careful to document this limitation in any newly-introduced methods in
> the past (see KIP-610 [1] and KIP-618 [2], for example), but even then,
> there's still risk with anything like this and we may not want to expand
> the API if the benefits don't outweigh the costs.
>
> 2. It's already possible to implement this logic directly in a connector
> today, without any changes to the Connect framework. Every connector can
> learn its own name in Connector::start by reading the "name" configuration
> property, and can then choose to pass that information along to its tasks
> as part of the configs it generates in Connector::taskConfigs. And, in the
> same way, connectors can choose to provide IDs for each task in the task
> configs that they generate. If this isn't sufficient for your use cases, it
> should be documented as a rejected alternative.
>
> BTW, it looks like this aims to accomplish something very similar or even
> identical to KIP-438 [3]. Ryanne Dolan (the author of that KIP) may want to
> weigh in here; I've CC'd them.
>
> [1] -
>
> https://cwiki.apache.org/confluence/display/KAFKA/KIP-610%3A+Error+Reporting+in+Sink+Connectors#KIP610:ErrorReportinginSinkConnectors-Method
> (see javadocs for SinkTaskContext::errantRecordReporter, paragraph starting
> with "This method was added in Apache Kafka 2.6")
> [2] -
>
> https://cwiki.apache.org/confluence/display/KAFKA/KIP-618%3A+Exactly-Once+Support+for+Source+Connectors#KIP618:ExactlyOnceSupportforSourceConnectors-ConnectorAPIexpansions
> (see javadocs for SourceTaskContext::transactionContext, paragraph starting
> with "This method was added in Apache Kafka 3.0")
> [3] -
>
> https://cwiki.apache.org/confluence/display/KAFKA/KIP-438%3A+Expose+task%2C+connector+IDs+in+Connect+API
>
> Cheers,
>
> Chris
>
> On Tue, Dec 7, 2021 at 10:07 AM Sarah Story <
> sa...@sarahstoryengineering.com>
> wrote:
>
> > Hi all! Just wanted to bump this KIP for adding Task ID and Connector
> Name
> > to the task context. It's a small change, and I'd love some feedback on
> it!
> >
> > Thanks!
> > Sarah
> >
> > On Wed, Nov 24, 2021 at 10:26 AM Sarah Story <
> > sa...@sarahstoryengineering.com> wrote:
> >
> > > I have written a KIP for adding Task ID and Connector name to the
> Connect
> > > API task context.
> > >
> > > Here is the doc: https://cwiki.apache.org/confluence/x/4pKqCw
> > >
> > > Looking forward to hearing all of your thoughts!
> > >
> >
>

Re: [DISCUSS] KIP-767 Connect Latency Metrics

[Jordan Bull]

Hey Chris,

Thanks for jumping in! I have been using the consumer lag as an indicator
for some time, but when measured directly at the consumer, it will not
factor in the time that Connect actually spends transforming and sending
the messages. This is certainly useful for measuring if the connector is
keeping up, but doesn't really tell the story of how much delay Connect
introduces. We also perform a measurement of latency based off of the
commit times, but as you mentioned this is often dominated by the commit
interval. This limits our ability to provide SLOs for sub second latency as
this is used for realtime connecting. Both approaches do allow a
measurement of a connector's ability to keep up with a throughput, but
we've found neither allows us to measure real latency SLOs for Connect as a
realtime service.

- Jordan

On Tue, Sep 7, 2021 at 2:43 PM Chris Egerton 
wrote:

> Hi Jordan,
>
> Thanks for the KIP. I'm curious about a possible alternative where the
> consumer lag for the source connector can be monitored instead of the
> newly-proposed metric in the KIP. Although sink tasks can't directly report
> the successful write of a record to the sink system, they are responsible
> for indirectly monitoring and communicating this in the form of the offsets
> returned from the SinkTask::preCommit method. This should mean that, for
> any well-behaved connector that returns accurate offsets from its preCommit
> method (including connectors that perform synchronous writes in
> SinkTask::put, which in most cases will not override the default behavior
> of the preCommit method and will allow the most up-to-date offsets read
> from each topic to be committed to the consumer), the consumer lag for the
> connector should be a decent way to monitor latency. Of course, it'll be at
> the mercy of the commit interval for the connector and whether the
> connector can successfully commit offsets with its consumer, but since that
> often dictates where tasks will resume from if restarted, there's still
> plenty of value in this metric.
>
> Cheers,
>
> Chris
>
> On Thu, Sep 2, 2021 at 7:03 PM Ryanne Dolan  wrote:
>
> > Thanks Jordan, this is a major blindspot today.
> >
> > Ryanne
> >
> >
> > On Wed, Sep 1, 2021, 6:03 PM Jordan Bull  wrote:
> >
> > > Hi all,
> > >
> > > I would like to start the discussion for KIP-767 involving adding
> latency
> > > metrics to Connect. The KIP can be found at
> > >
> > >
> >
> https://cwiki.apache.org/confluence/display/KAFKA/KIP-767%3A+Connect+Latency+Metrics
> > >
> > > Thanks,
> > > Jordan
> > >
> >
>

[DISCUSS] KIP-767 Connect Latency Metrics

[Jordan Bull]

Hi all,

I would like to start the discussion for KIP-767 involving adding latency
metrics to Connect. The KIP can be found at
https://cwiki.apache.org/confluence/display/KAFKA/KIP-767%3A+Connect+Latency+Metrics

Thanks,
Jordan

Request Permissions to Contribute

[Jordan Bull]

Hi,

I'd like to request permissions to contribute to Kafka to propose a KIP.

Wiki ID: jbull
Jira ID: jbull

Thanks you,
Jordan