I think the Smart Sensor should not be deprecated.

Based on the doc
https://airflow.apache.org/docs/apache-airflow/stable/concepts/deferring.html#deferrable-operators-triggers
 ,

The trigger is run until it fires, at which point its source task is
> re-scheduled
> The scheduler queues the task to resume on a worker node


deferrable operators/tasks still need to be scheduled by the schedulers.
Will this incur additional load to the scheduler?

When rescheduling those tasks, it will create bursty traffic to the worker,
which might cause issues. Also now, workers need to parse those dag files
again, which could be expensive.

A few lightweight long running smart sensors can help to reduce the burden
of the scheduler and workers.

Best wishes

Ping Zhang


On Thu, Dec 9, 2021 at 12:47 PM Jarek Potiuk <[email protected]> wrote:

> Love it ! Thanks Jed :).
>
> On Thu, Dec 9, 2021 at 9:35 PM Jed Cunningham <[email protected]>
> wrote:
> >
> > Thanks everyone for the feedback. Being more user-centric is certainly
> not a bad thing. (@Jarek, I didn't take your comments personally.)
> >
> > I think I've addressed everything discussed here and in the PR, so
> please take another look. High level, I've added a docs section on
> migrating and linked to it across the board.
> >
> >
> > On Thu, Dec 9, 2021 at 10:08 AM Kaxil Naik <[email protected]> wrote:
> >>
> >> In 100% agreement with the sentiments, Jarek and you have been doing
> great work at improving that recently.
> >>
> >> Looking forward to a single Updating guide from Jed too.
> >>
> >>
> >>
> >> On Thu, Dec 9, 2021 at 5:04 PM Jarek Potiuk <[email protected]> wrote:
> >>>
> >>> This is cool, Jed. Looking forward to it!
> >>>
> >>> One more comment. Apologies if my words were understood as "critique"
> >>> of anyone's job. Jed - if it came to you as such - it was never
> >>> intended. So - sincere apologies.
> >>>
> >>> I am as guilty as anyone of the "poor communication with our users".
> >>> So the "we have done quite a bad job in the past" applies to me in my
> >>> mind mostly (And I am personally embarrassed with some of those). I've
> >>> done a very poor job on that multiple times.
> >>>
> >>> My point is really - if there are - really small - things that we can
> >>> do now to improve what we have without introducing new ways (which
> >>> will take time I am sure) - why not?
> >>> Creating a small chapter on "Why no instructions?" and "What should I
> >>> do now?" - thinking from our poor users' perspective - who have no
> >>> time to read the docs - takes almost no time. Linking to it from the
> >>> error message - also simple.
> >>> This might prevent some questions from our users. So why not do it?
> >>>
> >>> J.
> >>>
> >>> On Thu, Dec 9, 2021 at 5:56 PM Jed Cunningham
> <[email protected]> wrote:
> >>> >
> >>> > As for UPDATING only being on github, I have a separate proposal in
> that area coming soon. It likely won't be an issue come time to release
> 2.3.0 👍.
> >>> >
> >>> > On Thu, Dec 9, 2021 at 9:34 AM Jarek Potiuk <[email protected]>
> wrote:
> >>> >>
> >>> >> And I agree with you :) (but with a twist).
> >>> >>
> >>> >> I do not say we should remove "UPDATING.md" information.  Not at
> all.
> >>> >>
> >>> >> Providing that:
> >>> >>
> >>> >> * UPDATING.md contains both
> >>> >> * It is available in our User-facing docs
> >>> >> * it has an anchor to this particular "piece of upgrading"
> >>> >> * the deprecated error message has a direct link to it to help to
> find it
> >>> >>
> >>> >> I (and our users I hope) would be perfectly happy.
> >>> >>
> >>> >> As far as I know. UPDATING.md is only in Github (I just checked and
> I
> >>> >> could not find in in airflow.apache.org. So by definition it's not
> a
> >>> >> User documentation. It's developer documentation only.
> >>> >>
> >>> >> J
> >>> >>
> >>> >>
> >>> >>
> >>> >>
> >>> >> On Thu, Dec 9, 2021 at 5:20 PM Kaxil Naik <[email protected]>
> wrote:
> >>> >> >
> >>> >> > Partially agree -- not completely.
> >>> >> >
> >>> >> > Firstly what I agree - (1) and (2) points from your email.
> >>> >> >
> >>> >> > Disagree the (3) point and the para after that.
> >>> >> >
> >>> >> > UPDATING.md is our source of breaking changes. Instead of users
> just having to rely and checking "deprecation" for 100s of commands, we
> should be helpful to users by also having a single page where we list all
> the deprecations.
> >>> >> >
> >>> >> > That is another way of being helpful in finding the "right"
> information and context quickly too. And "Guiding the users" in a different
> way.
> >>> >> >
> >>> >> > On Thu, Dec 9, 2021 at 4:13 PM Jarek Potiuk <[email protected]>
> wrote:
> >>> >> >>
> >>> >> >> I really think about a chapter (Which was missing):
> >>> >> >>
> >>> >> >> "How should I approach this migration?"
> >>> >> >>
> >>> >> >> 1) explain why there is no 1-1 migration instruction
> >>> >> >> 2) explain that for every smart sensor they need to use or write
> >>> >> >> deferrable operator
> >>> >> >> 3) link to this information from "deprecation message" they will
> see
> >>> >> >> in the logs when they use smart sensor (rather than relying on
> the
> >>> >> >> fact that they will look at UPDATING.md and find the right part
> >>> >> >>
> >>> >> >> That's it, Guiding the users. Being helpful in finding the right
> >>> >> >> information and context quickly (at the place where they hit the
> error
> >>> >> >> and not in one of the 100 pages of documentation that they will
> only
> >>> >> >> find by googling.
> >>> >> >>
> >>> >> >> J.
> >>> >> >>
> >>> >> >> On Thu, Dec 9, 2021 at 5:09 PM Kaxil Naik <[email protected]>
> wrote:
> >>> >> >> >
> >>> >> >> > Just as an FYI - the commit 18 hours ago on that PR already
> had added "deprecation" in the docs too.
> >>> >> >> >
> >>> >> >> > Not only docs, but UPDATING.md, even in the Scheduler logs, so
> kudos to Jed for taking care of it.
> >>> >> >> >
> >>> >> >> > So I don't agree with your comment or suggestion Jarek at
> least in the context of this discussion as it makes me (at least) read that
> the PR does not do those things.
> >>> >> >> >
> >>> >> >> > re: Tomek's question - it is a very valid question.
> Unfortunately, I don't see a like-by-like replacement for DAG Authors as
> different work needs to be done to write an Async operator and make a
> sensor "smart sensor compatible".
> >>> >> >> > However, agree that we try to be as clear as possible on what
> a user might need to do - I just don't know what that would be other than
> what I suggested in last email and would love the feedback on the PR of
> what else can be included.
> >>> >> >> >
> >>> >> >> > Thanks.
> >>> >> >> >
> >>> >> >> > Regards,
> >>> >> >> > Kaxil
> >>> >> >> >
> >>> >> >> > On Thu, Dec 9, 2021 at 4:00 PM Kaxil Naik <[email protected]>
> wrote:
> >>> >> >> >>
> >>> >> >> >> I don't think there is a 1-1 migration path. Async operators
> supersede what Smart sensors were written to achieve - Cost Savings.
> >>> >> >> >>
> >>> >> >> >> Smart Sensors were marked experimental feature for the same
> reason and there are currently just two Sensors that are Smart
> >>> >> >> >> sensors compatible.
> >>> >> >> >>
> >>> >> >> >> The only thing I can currently think of is writing an async
> version of the Smart Sensor Hook and Operator differs based on the
> underlying library that is used and
> https://airflow.apache.org/docs/apache-airflow/stable/concepts/deferring.html
> explains how you can write one. Also -
> https://airflow.apache.org/docs/apache-airflow/stable/concepts/deferring.html#smart-sensors
> >>> >> >> >>
> >>> >> >> >>
> >>> >> >> >>> I believe we have done quite a bad job in the past assuming
> that our
> >>> >> >> >>> users read all the discussions and AIPs we write. They
> don't. They
> >>> >> >> >>> need some guidance.
> >>> >> >> >>
> >>> >> >> >>
> >>> >> >> >> Which instances? I am just curious to know what are those bad
> instances where we "assumed" that our users read mailing list and not
> covered it in UPDATING.md or docs.
> >>> >> >> >>
> >>> >> >> >> Regards,
> >>> >> >> >> Kaxil
> >>> >> >> >>
> >>> >> >> >> On Thu, Dec 9, 2021 at 3:46 PM Jarek Potiuk <[email protected]>
> wrote:
> >>> >> >> >>>
> >>> >> >> >>> Extremely good point Tomek.
> >>> >> >> >>>
> >>> >> >> >>> Also as Ephraim pointed out in the PR - IMHO any time when
> we do
> >>> >> >> >>> deprecation we should have a note in our docs, explaining at
> the very
> >>> >> >> >>> least how the users should approach the migration as
> correctly pointed
> >>> >> >> >>> out by @turbaszek in the devlist.
> >>> >> >> >>> I think this should be a standard of any deprecation we do.
> >>> >> >> >>>
> >>> >> >> >>> I believe we have done quite a bad job in the past assuming
> that our
> >>> >> >> >>> users read all the discussions and AIPs we write. They
> don't. They
> >>> >> >> >>> need some guidance.
> >>> >> >> >>>
> >>> >> >> >>> J.
> >>> >> >> >>>
> >>> >> >> >>> On Wed, Dec 8, 2021 at 11:44 PM Tomasz Urbaszek <
> [email protected]> wrote:
> >>> >> >> >>> >
> >>> >> >> >>> > Do we have documentation about how to migrate from smart
> sensors to deferrable operators?
>

Reply via email to