Hi Vahid,
Thanks for taking a look at this KIP.
- The KIP is proposing a new interface because the existing "stop()" interface
isn't called at the end of the SourceTask's existence. It's a signal to the
task to stop, rather than a signal that it has actually stopped. Essentially, if
the task has resources to clean up, there's no clear point at which to do this
before
this KIP. I'm trying to make it a bit easier to write a connector with this
need.
- The "stop()" interface can be called multiple times which you can see by
setting
breakpoints. That could be a defect in the implementation but I think it's a bit
risky changing the timing of that call because some connector somewhere might
cease working. Unlikely, but compatibility is important. Also, it's important
that
the stop() signal is noticed and a SourceTask runs on multiple threads so it's
tricky.
The new method is called exactly once after everything has quiesced.
- I don't disagree that a verb sounds better but couldn't really think of a more
final alternative to "stop()". That's why I went with "stopped()". Could be
"cleanup()"
or "release()". Suggestions are welcome.
Thanks.
Andrew
On 03/05/2019, 06:16, "Vahid Hashemian" <vahid.hashem...@gmail.com> wrote:
Hi Andrew,
Thanks for the KIP. I'm not too familiar with the internals of KC so I hope
you can clarify a couple of things:
- It seems the KIP is proposing a new interface because the existing
"stop()" interface doesn't fully perform what it should ideally be doing.
Is that a fair statement?
- You mentioned the "stop()" interface can be called multiple times.
Would the same thing be true for the proposed interface? Does it matter?
Or
there is a guard against that?
- I also agree with Ryan that using a verb sounds more intuitive for an
interface that's supposed to trigger some action.
Regards,
--Vahid
On Thu, Jan 24, 2019 at 9:23 AM Ryanne Dolan <ryannedo...@gmail.com> wrote:
> Ah, I'm sorta wrong -- in the current implementation, restartTask()
> stops the task and starts a *new* task instance with the same task ID.
> (I'm not certain that is clear from the documentation or interfaces,
> or if that may change in the future.)
>
> Ryanne
>
> On Thu, Jan 24, 2019 at 10:25 AM Ryanne Dolan <ryannedo...@gmail.com>
> wrote:
> >
> > Andrew, I believe the task can be started again with start() during the
> stopping and stopped states in your diagram.
> >
> > Ryanne
> >
> > On Thu, Jan 24, 2019, 10:20 AM Andrew Schofield <
> andrew_schofi...@live.com wrote:
> >>
> >> I've now added a diagram to illustrate the states of a SourceTask. The
> KIP is essentially trying to give a clear signal to SourceTask when all
> work has stopped. In particular, if a SourceTask has a session to the
> source system that it uses in poll() and commit(), it now has a safe way
to
> release this.
> >>
> >> Andrew Schofield
> >> IBM Event Streams
> >>
> >> On 21/01/2019, 10:13, "Andrew Schofield" <andrew_schofi...@live.com>
> wrote:
> >>
> >> Ryanne,
> >> Thanks for your comments. I think my overarching point is that the
> various states of a SourceTask and the transitions between them seem a bit
> loose and that makes it difficult to figure out when the resources held by
> a SourceTask can be safely released. Your "I can't tell from the
> documentation" comment is key here __ Neither could I.
> >>
> >> The problem is that stop() is a signal to stop polling. It's
> basically a request from the framework to the task and it doesn't tell the
> task that it's actually finished. One of the purposes of the KC framework
> is to make life easy for a connector developer and a nice clean "all done
> now" method would help.
> >>
> >> I think I'll add a diagram to illustrate to the KIP.
> >>
> >> Andrew Schofield
> >> IBM Event Streams
> >>
> >> On 18/01/2019, 19:02, "Ryanne Dolan" <ryannedo...@gmail.com> wrote:
> >>
> >> Andrew, do we know whether the SourceTask may be start()ed
> again? If this
> >> is the last call to a SourceTask I suggest we call it close().
> I can't tell
> >> from the documentation.
> >>
> >> Also, do we need this if a SourceTask can keep track of whether
> it was
> >> start()ed since the last stop()?
> >>
> >> Ryanne
> >>
> >>
> >> On Fri, Jan 18, 2019, 12:02 PM Andrew Schofield <
> andrew_schofi...@live.com
> >> wrote:
> >>
> >> > Hi,
> >> > I’ve created a new KIP to enhance the SourceTask interface in
> Kafka
> >> > Connect.
> >> >
> >> >
> >> >
>
https://nam02.safelinks.protection.outlook.com/?url=https%3A%2F%2Fcwiki.apache.org%2Fconfluence%2Fdisplay%2FKAFKA%2FKIP-419%3A%2BSafely%2Bnotify%2BKafka%2BConnect%2BSourceTask%2Bis%2Bstopped&data=02%7C01%7C%7C01ae755e310a423ca1f808d6cf8680af%7C84df9e7fe9f640afb435aaaaaaaaaaaa%7C1%7C0%7C636924573921246068&sdata=qgbFXAfB0ih1%2BQ%2B2KX4dyk6BnYE61H0vvhITAVMUyfw%3D&reserved=0
> >> >
> >> > Comments welcome.
> >> >
> >> > Andrew Schofield
> >> > IBM Event Streams
> >> >
> >> >
> >>
> >>
> >>
> >>
>
--
Thanks!
--Vahid
