Thanks Andrew, that answers my questions. If we don't have any other questions by the end of the week we should start a VOTE.
Regards, Kaxil On Tue, Apr 20, 2021 at 2:24 AM Andrew Godwin <andrew.god...@astronomer.io.invalid> wrote: > Thanks Kaxil - notes on those two things: > > - A timeout is probably a reasonable thing to have in most situations, as > it gives you that little bit of self-healing ability (I put a created > timestamp into the Trigger schema partially out of this kind of caution). > I'll update the AIP to mention that triggers will come with an optional > timeout. > > - Correct, users who don't want the new process shouldn't be affected at > all. The main bad failure case here is that if you don't have the process > and then try to use a deferred operator, it would silently hang forever; my > plan was to take a cue from the code in the webserver that warns you the > scheduler isn't running, and do something similar for triggers, somewhere > (it can't be an immediate error when you try to run the task, as the > triggerer process may just be down for a few seconds for a redeploy, or > similar). > > Andrew > > On Mon, Apr 19, 2021 at 6:10 PM Kaxil Naik <kaxiln...@gmail.com> wrote: > >> +1 >> >> This is awesome Andrew, this is going to be a huge cost saver. >> >> The AIP is quite detailed and the Draft PR certainly helps. >> >> Just minor comments: >> >> >> 1. Should we have a timeout on publishing a Trigger / Task to the >> triggerer similar to >> >> https://airflow.apache.org/docs/apache-airflow/stable/configurations-ref.html#operation-timeout >> 2. In How are users affected by the change? >> >> <https://cwiki.apache.org/confluence/pages/viewpage.action?pageId=177050929#AIP40:Deferrable(%22Async%22)Operators-Howareusersaffectedbythechange?(e.g.DBupgraderequired?)> >> section, do the users who don't want to use the new process affected by >> it. >> Just confirming that if a user does not opt-in they are unaffected. >> >> <https://cwiki.apache.org/confluence/pages/viewpage.action?pageId=177050929#AIP40:Deferrable(%22Async%22)Operators-Howareusersaffectedbythechange?(e.g.DBupgraderequired?)> >> >> Regards, >> Kaxil >> >> On Thu, Apr 15, 2021 at 9:34 PM Andrew Godwin >> <andrew.god...@astronomer.io.invalid> wrote: >> >>> Hi all, >>> >>> After a period of designing and prototyping, I've completed a first >>> draft of AIP-40 that I'd love to get some feedback on from the community: >>> https://cwiki.apache.org/confluence/pages/viewpage.action?pageId=177050929 >>> >>> This AIP proposes a way of adding what I'm calling "deferrable" >>> Operators into Airflow - essentially, taking the goal of Smart Sensors and >>> making it much more generalisable, where any Sensor or Operator can choose >>> to "defer" its execution based on an asynchronous trigger, and where all >>> the triggers run in one (or more) processes for efficiency. >>> >>> It also means that any Operator can be made deferrable in a >>> backwards-compatible way should we wish to in future, though I'm not >>> proposing that for a first release. >>> >>> It comes with a working prototype, too, should you wish to see what kind >>> of code footprint this would have: >>> https://github.com/apache/airflow/pull/15389 >>> >>> I personally think this would be a huge improvement for >>> Airflow's efficiency - I suspect we might be able to reduce the amount of >>> resources some Airflow installs use by over 50% if all their idling >>> operators were ported to be deferrable - but I would welcome further >>> opinions. >>> >>> Thanks, >>> Andrew >>> >>
