Hi Ash, Nice, thanks for the explanation. If it does not need to go back to a worker, it will be perfect.
Thanks, Ping Best wishes Ping Zhang On Fri, Dec 17, 2021 at 2:16 AM Ash Berlin-Taylor <[email protected]> wrote: > > A few lightweight long running smart sensors can help to reduce the > burden of the scheduler and workers. > > That is exactly what deferrable tasks are. When a task is deferred it gets > picked up by a triggerer process, and it runs an in an async IO loop until > the task is ready to continue. > > We could add an optimization for deferred task that don't have any further > steps and this don't need to go back to a worker (i.e. they also don't need > to run any further callbacks) > > Would that address your concern? > > -ash > > On Thu, Dec 16 2021 at 22:06:21 -0800, Ping Zhang <[email protected]> > wrote: > > 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? >> >
