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 >> >
