You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@airflow.apache.org by Daniel Standish <da...@astronomer.io.INVALID> on 2023/09/12 18:24:45 UTC

Re: [DISCUSS] Mechanism of SLA

Some questions for you Sung.

I tried looking to understand why we needed to remove behavior 3 discussed
in AIP:

*[remove]* Task-level SLA measured from DAG-run scheduled start time


I'm just a little concerned that removing this would be a mistake because,
in my mind, part of the essence of what an SLA is, is an agreement about,
in essence, when your data should "be there". (or the "thing" done, etc)

The behavior replacing it seems a lot like the existing execution_timeout
behavior:

*[add]* Finally, I propose that we implement a new Task-level SLA feature,
> that is contained within the bounds of the task's lifetime, and is measured
> within the task itself.


[1] Do I understand that correctly?  If so, the question I would have is,
if we can already rely on execution timeout to tell us when an individual
task is taking too long, why do we need an SLA to tell us the same thing?

[2] Can you also clarify how "dag slas" are defined?  (behavior 1 in the
AIP).  Is the duration to be applied relative to scheduled time or start
time?

[3] I saw some discussion of using the triggerer component somehow in
this.  Is that a thing?  Can you share notes on implementation?  I looked
on the AIP but did not find anything.

[4] The fact that task sla cannot be used with deferrable operators seems
problematic to me.  Users tend to expect (rightly or not) something like
"parity" between deferrable and non-deferrable tasks.  SLAs would seem to
fall into a category where this seems like a reasonable expectation.

Some discussion on solutions...

I gather that the justification for removing behavior 3 is that it's too
complex / expensive.  I feel like there's got to be a way.   Thinking about
how we could implement behavior 3, there's two problems that sort of push
us into this perceived need to query everything all the time, in a manner
that is prohibitive.  One is the problem that the dag may not get
scheduled.  We need a way to raise if the dag is not scheduled X time after
it's "schedule *at*" date.  For the moment let's call that the dag
scheduling timeout.  What if, for each dag with such a scheduling timeout,
we launch a trigger that sleeps until that timeout and then when that runs
out it runs a single query to determine whether the dag run was scheduled
-- if not, raise.  Then, for the task part of things.... well if the dag is
assumed to be reliably scheduled (and this is protected by the first part
here), then we can, at the time of dag run scheduling, launch triggers for
the individual tasks.  And they would behave the same way.  Run until
timeout and then query.
Perhaps better yet, perhaps when a task completes, it can delete its
associated "sla" triggers, so that its sla trigger would be cancelled, and
no query ever has to run.  Same thing could be done for an overall dag SLA
-- at dag run completion could kill any trigger that is monitoring it's
execution duration.

Anyway, I understand that the AIP is already accepted, but just hoping we
can have the discussion, in case we can get to a place where the existing
"expected" sla behavior can perhaps be preserved in a performant manner.

Thanks

Re: [DISCUSS] Mechanism of SLA

Posted by Sung Yun <sy...@cornell.edu>.
Thank you for the input Colin - noted.

Sent from my iPhone

> On Sep 20, 2023, at 4:53 PM, Collin McNulty <co...@astronomer.io.invalid> wrote:
> 
> I want to concur with Daniel and Damian, that the key behavior for SLA
> should be based on when a DAG or task _should_ have run, not when it
> actually started. I think that’s important from a semantic standpoint, but
> also I think it’s just an important behavior to have in Airflow, because
> that’s how organizations’ requirements are often stated (e.g. this report
> ready by 7 AM).

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@airflow.apache.org
For additional commands, e-mail: dev-help@airflow.apache.org


Re: [DISCUSS] Mechanism of SLA

Posted by Collin McNulty <co...@astronomer.io.INVALID>.
I want to concur with Daniel and Damian, that the key behavior for SLA
should be based on when a DAG or task _should_ have run, not when it
actually started. I think that’s important from a semantic standpoint, but
also I think it’s just an important behavior to have in Airflow, because
that’s how organizations’ requirements are often stated (e.g. this report
ready by 7 AM).

Re: [DISCUSS] Mechanism of SLA

Posted by Sung Yun <sy...@cornell.edu>.
Hi Damian, it's so great to hear from you - it's been a minute! I
personally haven't heard of OLA, but that's a very helpful context. So in a
sense I think OLAs are less-official SLAs that are used loosely in an
interdepartmental setting? I think the official terminology Google's SRE
team uses to distinguish a contractual vs non-contractual agreement are
actually SLA (Service-Level Agreement) vs SLA (Service-Level Objective), so
I find it interesting that there's yet another term for something very
similar!

On that note, I think nomenclature is underrated, but incredibly important
in engineering; because if we choose it wrong, we give tremendous power of
imagination of what the feature is 'supposed to do' to the users. In an
open community like this, that's maybe a recipe for the feature to quickly
degrade itself over time, if it results in its responsibility being poorly
defined. And I think that may be what might have happened with this feature
- there are vastly different ideas of what this 'SLA' feature is supposed
to be doing, at this point in time. So like you said, whatever feature we
end up building a consensus on this time around, it may be valuable to
define the name of this new feature with clarity that speaks for itself.

Maybe the next steps we need to take here is:

1. Decide with intentionality, what user requirements we are solving for in
the refactored feature (and which ones we are not) - I think Daniel has
already begun to take a stab at this properly, by asking the room of users
what kind of feature they actually need for a delay detecting feature to be
usable. I will also try to gather data points as well, beyond what I have
scraped so far online.
2. Evaluate at what component we would need to build the feature. In this
discussion, we will keep in mind the following topics: how much of the
scheduler / dag file processor's capacity we think is fine to take up for
evaluating the delays and their callbacks, the function signature /
interface of the callback functions
3. And lastly, we could decide on what the proper name should be, for this
reliability feature that ended up with
Iterate 1-3 over again if it is not satisfactory.

And yes Daniel I noticed that about the Keynote Panel as well - and I am
very excited to get some more traction on this discussion. Also, sorry
about the confusion start date vs expected start date - I think I made that
point clear in an earlier response that I am okay with either measure, so I
was a bit confused about what points you were referring to when you said
'this is just a timeout' :

Here's my original response: "[2] In the accepted version of the AIP,
the DAG SLA is defined from the start_date of the DAG. I had initially
considered for it to be evaluated from the scheduled start time
(data_interval_end), but was stuck on how we could avoid false alarms when
dag runs are cleared, which was discussed in the Google Doc that was
circulated
<https://docs.google.com/document/d/1drNaYmAy6GqC4WGGn4MNt6VqbOwVNm7jPfmr5Pc52AU/edit#heading=h.z45mqf5nt94>
before
the AIP was drafted. I've submitted a small change this week to introduce
the dag_run clear_number attribute
<https://github.com/apache/airflow/pull/34126> which would actually
mitigate this issue. So if we feel that using the data_interval_end over
the start_date for a SCHEDULED DagRunType is more appropriate, we can make
that change on the open DAG SLA PR
<https://github.com/apache/airflow/pull/33532>."

TLDR: There was an edge case that wasn't thought through that made the
expected start time based delay monitoring a poor option for scheduled dag
runs. Now that this change is in, I'm fine with going with either
definition, whichever we think fulfills our requirements better.

Sung

On Wed, Sep 20, 2023 at 1:16 PM Damian Shaw <ds...@striketechnologies.com>
wrote:

> I'm not sure where most of the conversation is going on, but I would like
> to add a little experience previously working in large enterprises where
> SLAs were a part of day to day life.
>
> I would agree that both use cases are common:
>         1) Some alert or action for when a DAG or task *should* have
> completed but hasn't
>         2) Some alert or action for when a DAG or task exceeded some time
> from when it started
>
> In my experience the organizations I have work with tended to refer to 1
> as an SLA (Service Level Agreement) and 2 as an OLA (Operational Level
> Agreement), the idea that a service as a whole should stick to agreed
> times, but also there should be operational agreements that only start once
> prerequisites have been completed. I am not sure if this is common in
> general or unique terminology within the organizations I have worked with.
>
> From my experience I would therefore agree that to many it would be
> unintuitive if the behavior is 2 but it is called SLA.
>
> Damian
>
> -----Original Message-----
> From: Daniel Standish <da...@astronomer.io.INVALID>
> Sent: Wednesday, September 20, 2023 11:29 AM
> To: dev@airflow.apache.org
> Subject: Re: [DISCUSS] Mechanism of SLA
>
> I don't think of it as really a question about accurate record keeping but
> more a question of what an SLA is, i.e. when do you want the warning, or
> what do you want the warning based on.  I think that the idea has been that
> it really means, "if task not done by X time each day then warn".  And the
> way this was defined is dag schedule + timedelta.  And, it does seem that
> this is sort of a desired feature.  Indeed it just came up again in one of
> the keynotes. But, it will be nice to talk about it tomorrow and see what
> others think.
>
> Thanks for the blog post.  Reading it was productive for me.  I hadn't
> really considered the fact that the existing way that SLA works could be
> counterintuitive.  I can see how it wcould be.  You set it as a timedelta
> param on a task, and then this timedelta is added to the dag "should start"
> date, instead of task duration.  Anyway, again, look forward to chatting
> about it.
> ________________________________
>  Strike Technologies, LLC (“Strike”) is part of the GTS family of
> companies. Strike is a technology solutions provider, and is not a broker
> or dealer and does not transact any securities related business directly
> whatsoever. This communication is the property of Strike and its
> affiliates, and does not constitute an offer to sell or the solicitation of
> an offer to buy any security in any jurisdiction. It is intended only for
> the person to whom it is addressed and may contain information that is
> privileged, confidential, or otherwise protected from disclosure.
> Distribution or copying of this communication, or the information contained
> herein, by anyone other than the intended recipient is prohibited. If you
> have received this communication in error, please immediately notify Strike
> at info@striketechnologies.com, and delete and destroy any copies hereof.
> ________________________________
>
> CONFIDENTIALITY / PRIVILEGE NOTICE: This transmission and any attachments
> are intended solely for the addressee. This transmission is covered by the
> Electronic Communications Privacy Act, 18 U.S.C ''2510-2521. The
> information contained in this transmission is confidential in nature and
> protected from further use or disclosure under U.S. Pub. L. 106-102, 113
> U.S. Stat. 1338 (1999), and may be subject to attorney-client or other
> legal privilege. Your use or disclosure of this information for any purpose
> other than that intended by its transmittal is strictly prohibited, and may
> subject you to fines and/or penalties under federal and state law. If you
> are not the intended recipient of this transmission, please DESTROY ALL
> COPIES RECEIVED and confirm destruction to the sender via return
> transmittal.
>


-- 
Sung Yun
Cornell Tech '20
Master of Engineering in Computer Science

RE: [DISCUSS] Mechanism of SLA

Posted by Damian Shaw <ds...@striketechnologies.com>.
I'm not sure where most of the conversation is going on, but I would like to add a little experience previously working in large enterprises where SLAs were a part of day to day life.

I would agree that both use cases are common:
        1) Some alert or action for when a DAG or task *should* have completed but hasn't
        2) Some alert or action for when a DAG or task exceeded some time from when it started

In my experience the organizations I have work with tended to refer to 1 as an SLA (Service Level Agreement) and 2 as an OLA (Operational Level Agreement), the idea that a service as a whole should stick to agreed times, but also there should be operational agreements that only start once prerequisites have been completed. I am not sure if this is common in general or unique terminology within the organizations I have worked with.

From my experience I would therefore agree that to many it would be unintuitive if the behavior is 2 but it is called SLA.

Damian

-----Original Message-----
From: Daniel Standish <da...@astronomer.io.INVALID>
Sent: Wednesday, September 20, 2023 11:29 AM
To: dev@airflow.apache.org
Subject: Re: [DISCUSS] Mechanism of SLA

I don't think of it as really a question about accurate record keeping but more a question of what an SLA is, i.e. when do you want the warning, or what do you want the warning based on.  I think that the idea has been that it really means, "if task not done by X time each day then warn".  And the way this was defined is dag schedule + timedelta.  And, it does seem that this is sort of a desired feature.  Indeed it just came up again in one of the keynotes. But, it will be nice to talk about it tomorrow and see what others think.

Thanks for the blog post.  Reading it was productive for me.  I hadn't really considered the fact that the existing way that SLA works could be counterintuitive.  I can see how it wcould be.  You set it as a timedelta param on a task, and then this timedelta is added to the dag "should start"
date, instead of task duration.  Anyway, again, look forward to chatting about it.
________________________________
 Strike Technologies, LLC (“Strike”) is part of the GTS family of companies. Strike is a technology solutions provider, and is not a broker or dealer and does not transact any securities related business directly whatsoever. This communication is the property of Strike and its affiliates, and does not constitute an offer to sell or the solicitation of an offer to buy any security in any jurisdiction. It is intended only for the person to whom it is addressed and may contain information that is privileged, confidential, or otherwise protected from disclosure. Distribution or copying of this communication, or the information contained herein, by anyone other than the intended recipient is prohibited. If you have received this communication in error, please immediately notify Strike at info@striketechnologies.com, and delete and destroy any copies hereof.
________________________________

CONFIDENTIALITY / PRIVILEGE NOTICE: This transmission and any attachments are intended solely for the addressee. This transmission is covered by the Electronic Communications Privacy Act, 18 U.S.C ''2510-2521. The information contained in this transmission is confidential in nature and protected from further use or disclosure under U.S. Pub. L. 106-102, 113 U.S. Stat. 1338 (1999), and may be subject to attorney-client or other legal privilege. Your use or disclosure of this information for any purpose other than that intended by its transmittal is strictly prohibited, and may subject you to fines and/or penalties under federal and state law. If you are not the intended recipient of this transmission, please DESTROY ALL COPIES RECEIVED and confirm destruction to the sender via return transmittal.

Re: [DISCUSS] Mechanism of SLA

Posted by Daniel Standish <da...@astronomer.io.INVALID>.
I don't think of it as really a question about accurate record keeping but
more a question of what an SLA is, i.e. when do you want the warning, or
what do you want the warning based on.  I think that the idea has been that
it really means, "if task not done by X time each day then warn".  And the
way this was defined is dag schedule + timedelta.  And, it does seem that
this is sort of a desired feature.  Indeed it just came up again in one of
the keynotes. But, it will be nice to talk about it tomorrow and see what
others think.

Thanks for the blog post.  Reading it was productive for me.  I hadn't
really considered the fact that the existing way that SLA works could be
counterintuitive.  I can see how it wcould be.  You set it as a timedelta
param on a task, and then this timedelta is added to the dag "should start"
date, instead of task duration.  Anyway, again, look forward to chatting
about it.

Re: [DISCUSS] Mechanism of SLA

Posted by Sung Yun <sy...@cornell.edu>.
Hi Daniel,

Thank you for following up with the assessment. That’s an incredibly valuable data point.

I know we may have some opportunity to talk about this topic more at the summit this week, but just for the sake of offering a reference of the other perspective, I would like to share this blog post where another user describes the SLA feature with the following words: ‘In Airflow’s context, SLA can be seen as “for how long your DAG can run before you need to do something about it”https://poatek.com/2022/10/19/how-to-fix-airflow-sla/

Which highlights the desire to use the SLA feature for the purpose of delay detection, as a soft timeout that does not kill the task and simply executes the defined callback.

If what you say is true, and there are indeed folks hoping to use the SLA defined on Airflow for bookkeeping the accurate count of SLA misses because they don’t want to do it outside of it, than I think it will be important for us to discuss at length and decide which of these two motivations we are prioritizing when finalizing the design of the next ‘SLA Feature’. Again, I feel that it is much more simple of an endeavor if we drop the sense of urgency if we are designing for accuracy, and vice versa. 

Or maybe we are better off having two separate implementations for the two - one that prioritizes urgency, and one that prioritizes accuracy in hindsight. And we can also continue to discuss at length what the difficulty is in trying to achieve both within a single feature as well.

Sent from my iPhone

> On Sep 19, 2023, at 1:19 AM, Daniel Standish <da...@astronomer.io.invalid> wrote:
> 
> I was able to chat with a couple folks about this. Small sample, but the
> sentiment was, "this is just a timeout".  In other words, if we're going to
> call this SLA, we really ought to evaluate against the "this thing should
> have run by" time and not the actual start time.  And, ideally, we should
> also have a way to enforce "this should have run by X time daily" (for
> example) even when it's a dataset-triggered or API-triggered dag with *no*
> schedule.
> 
> Like I said, it's a small number of folks I've talked to, so I don't have
> overwhelming confidence about this assessment.  But I do think it's more
> likely than not that this would be the prevailing assessment were somehow
> able to get better data on this.

Re: [DISCUSS] Mechanism of SLA

Posted by Ayaan Rayaan <ay...@gmail.com>.
mohammadirfanmemon687687@gmail.com
On Sep 19, 2023 10:19 AM, "Daniel Standish"
<da...@astronomer.io.invalid> wrote:

> I was able to chat with a couple folks about this. Small sample, but the
> sentiment was, "this is just a timeout".  In other words, if we're going to
> call this SLA, we really ought to evaluate against the "this thing should
> have run by" time and not the actual start time.  And, ideally, we should
> also have a way to enforce "this should have run by X time daily" (for
> example) even when it's a dataset-triggered or API-triggered dag with *no*
>  schedule.
>
> Like I said, it's a small number of folks I've talked to, so I don't have
> overwhelming confidence about this assessment.  But I do think it's more
> likely than not that this would be the prevailing assessment were somehow
> able to get better data on this.
>

Re: [DISCUSS] Mechanism of SLA

Posted by Daniel Standish <da...@astronomer.io.INVALID>.
I was able to chat with a couple folks about this. Small sample, but the
sentiment was, "this is just a timeout".  In other words, if we're going to
call this SLA, we really ought to evaluate against the "this thing should
have run by" time and not the actual start time.  And, ideally, we should
also have a way to enforce "this should have run by X time daily" (for
example) even when it's a dataset-triggered or API-triggered dag with *no*
 schedule.

Like I said, it's a small number of folks I've talked to, so I don't have
overwhelming confidence about this assessment.  But I do think it's more
likely than not that this would be the prevailing assessment were somehow
able to get better data on this.

Re: [DISCUSS] Mechanism of SLA

Posted by Sung Yun <sy...@cornell.edu>.
No problem! I very much appreciate your questions and critical thought
process as well. It's been pretty difficult for me to fully understand how
the SLA feature worked, given how overloaded and complicated the logic is
in its current state. So it really helps to have another invested party
thinking through the solution.

Yeah, I think that's a great point. If we are changing the definition of
'SLA' or 'soft_alert_timeout' to mean: checking if a running dag, or its
running tasks have just passed the expected completion time, maybe we could
put it into the scheduler. I think the complication of performance concerns
only arises when we are trying to ensure that SLA Misses for tasks have
been evaluated for all the tasks - instead of checking just the running
tasks.

Here are some running thoughts that I think are worth thinking through
before we come to that conclusion:

1. The evaluation of SLA of tasks that have 'yet to be scheduled' is not
too straightforward. Yes, we've just made a distinction between missing an
SLA due to DAG scheduling delays vs a promptly running one with the
reasoning that if there are scheduling delays in the cluster, and that we
should be paying attention to a different higher level alarm. I think the
problem of evaluating SLAs for tasks that are blocked by an upstream task
is generally a different one - here we don't have any issues with the
cluster's capacity itself. Instead, there is a specific task we want to
measure against an 'expected time of completion' for, that has not been
launched yet because its upstream task has yet to be completed. My
impression is, if a DAG has been launched - we should evaluate the
'expected time of completion' of all the tasks within. If you disagree, I'm
happy to continue this discussion to hash out the scope of the
responsibility of this feature. For now, assuming that we see these blocked
tasks as being worth tracking the 'expected time of completion' for, there
are interesting ways to address this issue, and here are two examples.
- Now / in the DagFileProcessingProcess:
https://github.com/apache/airflow/blob/main/airflow/dag_processing/processor.py#L439
- the existing implementation does a group by to get the maximum execution
of a task instance that has ever succeeded or skipped in the specific Dag
ID, and 'extends' them onto the next run of the Dag. This way, we aren't
able to evaluate the SLA for the first run of a given task, but we avoid
not being able to evaluate the SLA for tasks that have been blocked by an
upstream task. In my opinion, even though this query has been tweaked over
multiple times, and behaves reasonably well, it slows the Dag Parsing
process when the number of dags and tasks scales out to hundreds or
thousands. Another big downside of this logic in my humble opinion, is just
that it's so difficult to understand and maintain. I had to read this code
over a series of times to understand what exactly we were exactly doing
with these series of SQL operations.
- If Task SLAs were evaluated in the scheduler at update_state, similar to
how it is done in this proposal:
https://github.com/apache/airflow/pull/8545/files#diff-62c8e300ee91e0d59f81e0ea5d30834f04db71ae74f2e155a10b51056b00b59bR1772,
we would need to infer 'unscheduled_tis' that are more downstream than the
list of TIs that are currently "RUNNING". Maybe this isn't so bad?

2. The callback should not be called within the scheduler: I believe that
there is a reason why DagCallbackRequests are sent to the
DagFileProcessingProcess for it to be executed in a separate process. My
general understanding is, the DAG Data Model that the Scheduler has access
to, only has a subset of DAG attributes available to it. And same goes for
the Task Instance. If we want to execute the callback, we need to issue the
callback request to an external process for it to launch. Let's say that we
want to send a callback request to the DagFileProcessingProcess once an SLA
miss is detected, and have the DAG execute the task level SLA callback. The
issue with this is that the task_instance context that the DAG has access
to, is actually not the context of the task itself, but just the first task
instance that is arbitrarily selected to represent the DAG:
https://github.com/apache/airflow/blob/main/airflow/models/dag.py#L1408.
Hence, a Task level SLA callback cannot accurately be called in a
DagFileProcessorProcess. I believe this is the reason why folks over the
years have fitted so many additional arguments into the sla_miss_callback
that pulls it further away from having a consistent function signature with
other DAG and Task level callbacks.

On Wed, Sep 13, 2023 at 1:51 PM Daniel Standish
<da...@astronomer.io.invalid> wrote:

> OK so one difference here is, you're adding a new DAG SLA concept.  Which
> is useful.  One subtle difference from what I think is the existing
> "concept" of SLA is that you are evaluating it against when it started, as
> opposed to when it should have started, and evaluating it only in the
> course of running.
>
> Let's suppose for a moment that everyone is on board with this and thinks
> it's a necessary tradeoff.
>
> Well now let's look at individual task instance SLAs.  With that change in
> the concept of what an SLA is, do we still *need* to move to "soft timeout"
> for individual tasks?  I think maybe no.  Because, why could we not, at the
> same time as we evaluate the dag run SLA, also evaluate each task's SLA,
> and evaluate it against the same "start time" that the overall DAG SLA is
> evaluating against?  This would seem to be more *like* the existing SLA
> concept for individual tasks, the difference being it requires the dag to
> be running (which is already a requirement of your new task SLA concept).
> The other difference, again, is the start time vs should-have-started time
> distinction.  But this would also seem to remove the "doesn't work for
> deferrables" problem.
>


-- 
Sung Yun
Cornell Tech '20
Master of Engineering in Computer Science

Re: [DISCUSS] Mechanism of SLA

Posted by Daniel Standish <da...@astronomer.io.INVALID>.
OK so one difference here is, you're adding a new DAG SLA concept.  Which
is useful.  One subtle difference from what I think is the existing
"concept" of SLA is that you are evaluating it against when it started, as
opposed to when it should have started, and evaluating it only in the
course of running.

Let's suppose for a moment that everyone is on board with this and thinks
it's a necessary tradeoff.

Well now let's look at individual task instance SLAs.  With that change in
the concept of what an SLA is, do we still *need* to move to "soft timeout"
for individual tasks?  I think maybe no.  Because, why could we not, at the
same time as we evaluate the dag run SLA, also evaluate each task's SLA,
and evaluate it against the same "start time" that the overall DAG SLA is
evaluating against?  This would seem to be more *like* the existing SLA
concept for individual tasks, the difference being it requires the dag to
be running (which is already a requirement of your new task SLA concept).
The other difference, again, is the start time vs should-have-started time
distinction.  But this would also seem to remove the "doesn't work for
deferrables" problem.

Re: [DISCUSS] Mechanism of SLA

Posted by Daniel Standish <da...@astronomer.io.INVALID>.
First of all, thanks for being so charitable in engaging in this dialogue,
I appreciate it.

Yeah I think that the notion that maybe Airflow is making really
impractical promises with SLA, well that could be true.

One question for you, as I continue to let this percolate.

Can you help me understand why, from a technical perspective, you
determined that, calculating task sla against "when it should have started"
(i.e. "the old behavior", as I understand it) is so problematic that we
need to scrap that behavior?  I'm not saying it's not, but just want to
understand better what you found.  My sense is that you determined that it
involves too much polling and querying overy too great a pool of potential
tasks to be practical.  But if you could provide some details that would
help me understand.  If it's already been hashed out in a thread, I'm happy
to be directed there.  I looked at the AIP and did not find detailed
discussion of this.

Thank you

Re: [DISCUSS] Mechanism of SLA

Posted by Sung Yun <sy...@cornell.edu>.
Hi Daniel,

These are all really great points, and I'm going to attempt at answering
all of them in no particular order:

On Expectations / SLAs / Naming:
I think you hit the nail on the head here, and I agree with you that the
naming choice of SLA is very misleading. To my understanding,
SLA/Service-Level Agreement means something very specific in Reliability
Engineering, which refers to the contract that the application owner has
with the end-users that might have monetary consequences if it isn't met.
And I do think that this choice of word might generate unrealistic
expectations of what Airflow is supposed to keep track of. In my humble
opinion, given that Airflow is a workflow orchestration tool, I think it
would be best equipped with supporting features like 'execution_timeout'.
Similarly, it would be great at measuring and executing a callback for
something like a 'soft_alert_timeout' or 'expected_time_of_completion' -
when it happens. I think these are similar, but semantically different in
how people want to use them. When people talk about
'expected_time_of_completion', people are concerned about being alerted as
soon as the system can detect that something is running longer than
expected. When people talk about 'SLA's, we are generally interested in
checking how many times we honored or missed the SLA by checking the
delivery timestamp on the actual files or table updates in hindsight, to
check if we did hold up that agreement for some past period. And I would
argue that as a workflow orchestration tool, Airflow should be concerned
with checking for 'expected_time_of_completion'. Keeping track of 'SLA'
could also be done as a separate on-demand analysis by simply checking the
timestamps of the deliverables.

On Data Interval End:
I think data_interval_end is equivalent to the 'expected start_date' of
scheduled DagRuns. This same logic is used in generating the existing
schedule_delay metrics here:
https://github.com/apache/airflow/blob/main/airflow/jobs/scheduler_job_runner.py#L1359

On where the SLA Miss are evaluated, and where SLA callbacks are executed:
Now: SLA Miss are evaluated in DagFileProcessingProcessor, at every dag
parsing time. SLA callbacks are executed in the processor as well
AIP:
1. DAG SLA - SLA Miss are evaluated in the scheduler loop, when the
scheduler checks the DAG to update its state. The callbacks are executed in
the DagFileProcessingProcessor, just like the other DAG level callbacks
2. Task-duration SLA - SLA Miss is evaluated in a child process in the task
instance, and is evaluated within that child process.
DagFileProcessingProcessor is not involved in this at all. Since it needs
to be evaluated in the child process, this feature cannot be supported in
Deferrable Operators, because the task would have deferred itself in lieu
of a Trigger.
3. Custom Operator based user defined SLA - SLA Miss is evaluated in a
Deferrable Operator after it receives the event back from the Temporal
Trigger. The SLA callback is evaluated here.

On any concerns with making idea [3], config driven:
This is where I want to give more weight to the opinions of folks who have
been developing on Airflow core infrastructure for a longer time. I think
since [3] is such a lightweight solution, it would generally be fine to
mold this solution into a config driven approach. My only concern here,
would be that if we create the SLA Triggers and Deferrable Operators (which
need to be picked up by the worker to execute the SLA callback) behind the
scenes, instead of users actually adding them to the DAGs themselves, we
are hiding away the details of how many Trigger slots or worker slots the
SLA parameters are taking up. Yes, these are going to incur very little
resource on the cluster, but that's where my suggestions to having a
dedicated 'sla_pool' and limiting the execution_timeout of these callback
functions to be incredibly short comes from. If we are creating these
invisible triggers and deferrable operators to do the job, I would want to
make it disrupt the capacity of workers, triggers and existing
default_pools as little as possible, if at all.

On Tue, Sep 12, 2023 at 5:05 PM Daniel Standish
<da...@astronomer.io.invalid> wrote:

> >
> > [1] Yes, that's correct. I believe that containing the SLA evaluation
> > within the lifetime of a task as a duration-based sla will still have a
> > purpose. It's technically implemented like an execution_timeout, but the
> > goal of the SLA check is to execute a callback without killing the task:
> so
> > that the accountable owner can be alerted, or a record of the SLA miss
> can
> > be made without killing the task and throwing away the work that's been
> > done. Measuring the SLA as a duration of a task is actually a feature
> that
> > has been requested before, by multiple folks in previous discussions and
> > PRs.
>
>
> Good point yeah I see what you mean -- alert but don't kill
>
> [2] In the accepted version of the AIP, the DAG SLA is defined from the
> > start_date of the DAG. I had initially considered for it to be evaluated
> > from the scheduled start time (data_interval_end), but was stuck on how
> we
> > could avoid false alarms when dag runs are cleared, which was discussed
> in
> > the Google Doc that was circulated
> > <
> >
> https://docs.google.com/document/d/1drNaYmAy6GqC4WGGn4MNt6VqbOwVNm7jPfmr5Pc52AU/edit#heading=h.z45mqf5nt94
> > >
> > before the AIP was drafted. I've submitted a small change this week to
> > introduce
> > the dag_run clear_number attribute
> > <https://github.com/apache/airflow/pull/34126> which would actually
> > mitigate this issue. So if we feel that using the data_interval_end over
> > the start_date for a SCHEDULED DagRunType is more appropriate, we can
> make
> > that change on the open DAG SLA PR
> > <https://github.com/apache/airflow/pull/33532>.
>
>
> Using "data interval" doesn't sound right to me.  Data interval could be
> completely disconnected from "when should the dag finish".  When you say
> start_time you mean "when the dag run starts" not the dag attribute,
> correct?  So this would catch a long-running dag, but would not catch when,
> for some reason, there's a scheduling delay.  It's starting to feel like
> there are multiple kinds of SLAs.  And forgive me if this already
> well-trodden ground but I'm just now looking into it closely.  But so it
> sounds like what you're adding is perhaps a subtype, DurationSLA, as
> distinguished from maybe a different subtype, call it SchedulingSLA.  Maybe
> we can offer both?
>
> [3] An example of using Deferrable Operators to create Custom Operators
> > that monitor the SLA of a target task is discussed in the "Are there any
> > downsides to this change?" section as a workaround that will allow users
> to
> > measure SLA[3]: Task-level SLA measured from DAG-run scheduled start
> time.
> > I agree that this isn't an optimal message to say that we are removing an
> > existing feature. But perhaps the fact that users can implement this on
> > their own in a scalable manner makes the feature deprecation easier to
> > swallow...
>
>
> It's not just that it's not "optimal messaging".  Why would it not be
> optimal?  Because people want the feature, use the feature, expect the
> feature. That's the real issue, not the messaging.
> If we accept that this is a valid and desirable use cases, I think forcing
> users to write extra tasks to do this kind of monitoring, while it's a good
> attempt at a mitigation, just is not gonna work.  Not work in the sense of
> not good enough / acceptable.  To paint with an overly broad brush, no one,
> practically speaking, is going to do that :)
>
> [continuing with 3] I know Pierre Jeambrun brought up the idea of
> > implementing this
>
> way of detecting SLAs in a systematic way, so that instead of users
> > attaching CustomOperators to their DAGs, Airflow will add Triggers that
> > invoke sla callbacks at the end of the Trigger through a configuration
> > parameter on the task that we want to check the SLA on. If you think
> making
> > this approach of measuring SLAs on the Trigger, a first class idea on
> > Airflow is a good idea - I'm happy to discuss this further and also learn
>
> more from you on how we could make this a reality.
>
> Some running ideas that
> > come to my mind - new SLA feature requires a Triggerer process; we create
> > SLA watching temporal triggers if sla is defined on the task; the
> > sla_miss_callback uses 'context' argument, but the deferred operator that
> > is executed upon the completion of the temporal trigger has a strict
> > execution_timeout of 10 seconds (or other arbitrary short duration), to
> > ensure that the worker slots are not taken up by slas for a significant
> > duration; a separate pool for SLA triggers and workers (sla_pool?) so
> that
> > it doesn't use the default pool?
>
>
> You lost me a bit here with some of those details.  Happy to chat about
> it.  But yeah i think having the new SLA behavior rely on triggerer, I
> don't see a problem with that.  And I don't see a problem with using
> triggerer for more things.  Can't speak for others though.
>
> But separately, my original question here was, how is this AIP going to be
> implemented?  Does the code still run on the dag processor?  Thanks
>
> [4] I whole-heartedly agree with you on this - Deferrable Operators are my
> > favorite, and it feels bad to me that we'd be introducing a feature that
> > can't be extended to deferrables at all. And since the act of missing an
> > SLA isn't a final state, I'm not sure exactly how we would be able to
> > invoke an SLA callback on a Deferred Operator (or its corresponding
> > Trigger) in the current workflow.
>
>
> Help me understand what is the problem with running SLA callback.  Where /
> how do they run now / in the AIP?  And how is this more problematic with
> deferrables?
>
> On the idea of dags not being scheduled at a certain time - this is a
> > pretty popular edge case that obviously falls under the subject of
> > measuring SLAs, but I'm curious if it will be helpful to separate out the
> > concern of maintaining a scheduler cluster with enough bandwidth, vs the
> > concern of the DAG and the contained tasks running its job within its
> > expected SLA, provided that the cluster is in a good state. There are
> > existing reliability metrics that help us detect when the DAG runs
> > themselves are being scheduled late, or if we are reaching the limit of
> > worker parallelism. These are issues that will impact the delivery times
> of
> > all of the DAGs and the Tasks, if not resolved immediately. So to me, as
> > long as there are existing ways of detecting these more critical
> > infrastructure issues (which there are), I am not too concerned that my
> SLA
> > measuring might be impacted by a late scheduled DAG Run.
>
>
> Yeah I think this is a legitimate point.... essentially it's like "if your
> dags are running behind, you have bigger problems".  And maybe scheduling
> delay is not really an issue, and so we shouldn't really worry about it --
> I honestly don't know.  I saw you recently added a scheduling delay
> metric... maybe you know.  Nonetheless, it does feel like that's sort of
> core to what an SLA is.  From business stakeholder perspective, business
> user does not care why the task is delayed -- slow scheduling vs slow
> running of *this* task vs slow running of *upstream* tasks.  A business
> user wants their data in accordance with the SLA.  A business user doesn't
> care that X task in the pipeline took X time -- they care that their data
> is refreshed when expected.  Maybe the terminology SLA is throwing me off a
> bit for this reason, cus it really implies an "agreement" i.e. with someone
> else i.e. external stakeholder who... probably doesn't give a hoot about
> individual task duration.
>
> It feels like what we're dealing with is various kinds of "expectations".
> Not that I'm proposing a rename :) But when my dag finishes, I can set an
> "expectation" that the next dag run will be running by X time.  When the
> dag starts running, I can set an expectation that the dag will be *done* by
> X time (and that the tasks should be done by X times).  We can have
> expectations about how long certain things will take to happen.  It seems
> trigger would support this very easily.  Whether we should do it or not is
> another question 😬
>
> But yeah so, perhaps in a dag with an SLA, it emits events that are
> relevant to SLA logic.  And then perhaps something consumes those events
> and runs triggers that enforce those expectations.  And when events are
> emitted which mean trigger not needed anymore, then that "something"
> processes that event and in response kills the relevant trigger.  Thinking
> out loud here.  It would put a lot more tasks on the triggerer, but all
> they would be doing is sleeping.
>
>
>
>
>
>
>
>
>
> On Tue, Sep 12, 2023 at 12:48 PM Sung Yun <sy...@cornell.edu> wrote:
>
> > Hi Daniel,
> >
> > Thank you for the review! I'm happy to keep having the discussion to make
> > sure we can introduce the right way of implementing these solutions into
> > Airflow.
> >
> > My general impression from the community in the discussions so far led me
> > to believe that deprecating the problematic feature would be welcomed at
> > large, as long as we can educate folks about how they can achieve a
> similar
> > outcome by using Deferrable Operators - which will be much less
> problematic
> > for the Airflow core infrastructure.
> > But as you say, if you think it is important to maintain the feature
> until
> > we can find a suitable, systematic replacement for it, I'm happy to hold
> > off on the implementations until we can make the necessary edits to the
> > AIP.
> >
> > Here are my answers to your points:
> >
> > [1] Yes, that's correct. I believe that containing the SLA evaluation
> > within the lifetime of a task as a duration-based sla will still have a
> > purpose. It's technically implemented like an execution_timeout, but the
> > goal of the SLA check is to execute a callback without killing the task:
> so
> > that the accountable owner can be alerted, or a record of the SLA miss
> can
> > be made without killing the task and throwing away the work that's been
> > done. Measuring the SLA as a duration of a task is actually a feature
> that
> > has been requested before, by multiple folks in previous discussions and
> > PRs.
> >
> > [2] In the accepted version of the AIP, the DAG SLA is defined from the
> > start_date of the DAG. I had initially considered for it to be evaluated
> > from the scheduled start time (data_interval_end), but was stuck on how
> we
> > could avoid false alarms when dag runs are cleared, which was discussed
> in
> > the Google Doc that was circulated
> > <
> >
> https://docs.google.com/document/d/1drNaYmAy6GqC4WGGn4MNt6VqbOwVNm7jPfmr5Pc52AU/edit#heading=h.z45mqf5nt94
> > >
> > before the AIP was drafted. I've submitted a small change this week to
> > introduce
> > the dag_run clear_number attribute
> > <https://github.com/apache/airflow/pull/34126> which would actually
> > mitigate this issue. So if we feel that using the data_interval_end over
> > the start_date for a SCHEDULED DagRunType is more appropriate, we can
> make
> > that change on the open DAG SLA PR
> > <https://github.com/apache/airflow/pull/33532>.
> >
> > [3] An example of using Deferrable Operators to create Custom Operators
> > that monitor the SLA of a target task is discussed in the "Are there any
> > downsides to this change?" section as a workaround that will allow users
> to
> > measure SLA[3]: Task-level SLA measured from DAG-run scheduled start
> time.
> > I agree that this isn't an optimal message to say that we are removing an
> > existing feature. But perhaps the fact that users can implement this on
> > their own in a scalable manner makes the feature deprecation easier to
> > swallow... I know Pierre Jeambrun brought up the idea of implementing
> this
> > way of detecting SLAs in a systematic way, so that instead of users
> > attaching CustomOperators to their DAGs, Airflow will add Triggers that
> > invoke sla callbacks at the end of the Trigger through a configuration
> > parameter on the task that we want to check the SLA on. If you think
> making
> > this approach of measuring SLAs on the Trigger, a first class idea on
> > Airflow is a good idea - I'm happy to discuss this further and also learn
> > more from you on how we could make this a reality. Some running ideas
> that
> > come to my mind - new SLA feature requires a Triggerer process; we create
> > SLA watching temporal triggers if sla is defined on the task; the
> > sla_miss_callback uses 'context' argument, but the deferred operator that
> > is executed upon the completion of the temporal trigger has a strict
> > execution_timeout of 10 seconds (or other arbitrary short duration), to
> > ensure that the worker slots are not taken up by slas for a significant
> > duration; a separate pool for SLA triggers and workers (sla_pool?) so
> that
> > it doesn't use the default pool?
> >
> > [4] I whole-heartedly agree with you on this - Deferrable Operators are
> my
> > favorite, and it feels bad to me that we'd be introducing a feature that
> > can't be extended to deferrables at all. And since the act of missing an
> > SLA isn't a final state, I'm not sure exactly how we would be able to
> > invoke an SLA callback on a Deferred Operator (or its corresponding
> > Trigger) in the current workflow.
> >
> > On the idea of dags not being scheduled at a certain time - this is a
> > pretty popular edge case that obviously falls under the subject of
> > measuring SLAs, but I'm curious if it will be helpful to separate out the
> > concern of maintaining a scheduler cluster with enough bandwidth, vs the
> > concern of the DAG and the contained tasks running its job within its
> > expected SLA, provided that the cluster is in a good state. There are
> > existing reliability metrics that help us detect when the DAG runs
> > themselves are being scheduled late, or if we are reaching the limit of
> > worker parallelism. These are issues that will impact the delivery times
> of
> > all of the DAGs and the Tasks, if not resolved immediately. So to me, as
> > long as there are existing ways of detecting these more critical
> > infrastructure issues (which there are), I am not too concerned that my
> SLA
> > measuring might be impacted by a late scheduled DAG Run.
> >
> > On the idea of killing the running sla triggers, if the task succeeds - I
> > think that's a great idea, it makes sure that the Trigger is removed when
> > it is no longer needed.
> >
> > Please let me know if you think idea [3] is worth exploring further and
> > materializing as a formal proposal. I'd be happy to find time to discuss
> it
> > more in depth :)
> >
> > Sung
> >
> > On Tue, Sep 12, 2023 at 2:25 PM Daniel Standish
> > <da...@astronomer.io.invalid> wrote:
> >
> > > Some questions for you Sung.
> > >
> > > I tried looking to understand why we needed to remove behavior 3
> > discussed
> > > in AIP:
> > >
> > > *[remove]* Task-level SLA measured from DAG-run scheduled start time
> > >
> > >
> > > I'm just a little concerned that removing this would be a mistake
> > because,
> > > in my mind, part of the essence of what an SLA is, is an agreement
> about,
> > > in essence, when your data should "be there". (or the "thing" done,
> etc)
> > >
> > > The behavior replacing it seems a lot like the existing
> execution_timeout
> > > behavior:
> > >
> > > *[add]* Finally, I propose that we implement a new Task-level SLA
> > feature,
> > > > that is contained within the bounds of the task's lifetime, and is
> > > measured
> > > > within the task itself.
> > >
> > >
> > > [1] Do I understand that correctly?  If so, the question I would have
> is,
> > > if we can already rely on execution timeout to tell us when an
> individual
> > > task is taking too long, why do we need an SLA to tell us the same
> thing?
> > >
> > > [2] Can you also clarify how "dag slas" are defined?  (behavior 1 in
> the
> > > AIP).  Is the duration to be applied relative to scheduled time or
> start
> > > time?
> > >
> > > [3] I saw some discussion of using the triggerer component somehow in
> > > this.  Is that a thing?  Can you share notes on implementation?  I
> looked
> > > on the AIP but did not find anything.
> > >
> > > [4] The fact that task sla cannot be used with deferrable operators
> seems
> > > problematic to me.  Users tend to expect (rightly or not) something
> like
> > > "parity" between deferrable and non-deferrable tasks.  SLAs would seem
> to
> > > fall into a category where this seems like a reasonable expectation.
> > >
> > > Some discussion on solutions...
> > >
> > > I gather that the justification for removing behavior 3 is that it's
> too
> > > complex / expensive.  I feel like there's got to be a way.   Thinking
> > about
> > > how we could implement behavior 3, there's two problems that sort of
> push
> > > us into this perceived need to query everything all the time, in a
> manner
> > > that is prohibitive.  One is the problem that the dag may not get
> > > scheduled.  We need a way to raise if the dag is not scheduled X time
> > after
> > > it's "schedule *at*" date.  For the moment let's call that the dag
> > > scheduling timeout.  What if, for each dag with such a scheduling
> > timeout,
> > > we launch a trigger that sleeps until that timeout and then when that
> > runs
> > > out it runs a single query to determine whether the dag run was
> scheduled
> > > -- if not, raise.  Then, for the task part of things.... well if the
> dag
> > is
> > > assumed to be reliably scheduled (and this is protected by the first
> part
> > > here), then we can, at the time of dag run scheduling, launch triggers
> > for
> > > the individual tasks.  And they would behave the same way.  Run until
> > > timeout and then query.
> > > Perhaps better yet, perhaps when a task completes, it can delete its
> > > associated "sla" triggers, so that its sla trigger would be cancelled,
> > and
> > > no query ever has to run.  Same thing could be done for an overall dag
> > SLA
> > > -- at dag run completion could kill any trigger that is monitoring it's
> > > execution duration.
> > >
> > > Anyway, I understand that the AIP is already accepted, but just hoping
> we
> > > can have the discussion, in case we can get to a place where the
> existing
> > > "expected" sla behavior can perhaps be preserved in a performant
> manner.
> > >
> > > Thanks
> > >
> >
> >
> > --
> > Sung Yun
> > Cornell Tech '20
> > Master of Engineering in Computer Science
> >
>


-- 
Sung Yun
Cornell Tech '20
Master of Engineering in Computer Science

Re: [DISCUSS] Mechanism of SLA

Posted by Daniel Standish <da...@astronomer.io.INVALID>.
>
> [1] Yes, that's correct. I believe that containing the SLA evaluation
> within the lifetime of a task as a duration-based sla will still have a
> purpose. It's technically implemented like an execution_timeout, but the
> goal of the SLA check is to execute a callback without killing the task: so
> that the accountable owner can be alerted, or a record of the SLA miss can
> be made without killing the task and throwing away the work that's been
> done. Measuring the SLA as a duration of a task is actually a feature that
> has been requested before, by multiple folks in previous discussions and
> PRs.


Good point yeah I see what you mean -- alert but don't kill

[2] In the accepted version of the AIP, the DAG SLA is defined from the
> start_date of the DAG. I had initially considered for it to be evaluated
> from the scheduled start time (data_interval_end), but was stuck on how we
> could avoid false alarms when dag runs are cleared, which was discussed in
> the Google Doc that was circulated
> <
> https://docs.google.com/document/d/1drNaYmAy6GqC4WGGn4MNt6VqbOwVNm7jPfmr5Pc52AU/edit#heading=h.z45mqf5nt94
> >
> before the AIP was drafted. I've submitted a small change this week to
> introduce
> the dag_run clear_number attribute
> <https://github.com/apache/airflow/pull/34126> which would actually
> mitigate this issue. So if we feel that using the data_interval_end over
> the start_date for a SCHEDULED DagRunType is more appropriate, we can make
> that change on the open DAG SLA PR
> <https://github.com/apache/airflow/pull/33532>.


Using "data interval" doesn't sound right to me.  Data interval could be
completely disconnected from "when should the dag finish".  When you say
start_time you mean "when the dag run starts" not the dag attribute,
correct?  So this would catch a long-running dag, but would not catch when,
for some reason, there's a scheduling delay.  It's starting to feel like
there are multiple kinds of SLAs.  And forgive me if this already
well-trodden ground but I'm just now looking into it closely.  But so it
sounds like what you're adding is perhaps a subtype, DurationSLA, as
distinguished from maybe a different subtype, call it SchedulingSLA.  Maybe
we can offer both?

[3] An example of using Deferrable Operators to create Custom Operators
> that monitor the SLA of a target task is discussed in the "Are there any
> downsides to this change?" section as a workaround that will allow users to
> measure SLA[3]: Task-level SLA measured from DAG-run scheduled start time.
> I agree that this isn't an optimal message to say that we are removing an
> existing feature. But perhaps the fact that users can implement this on
> their own in a scalable manner makes the feature deprecation easier to
> swallow...


It's not just that it's not "optimal messaging".  Why would it not be
optimal?  Because people want the feature, use the feature, expect the
feature. That's the real issue, not the messaging.
If we accept that this is a valid and desirable use cases, I think forcing
users to write extra tasks to do this kind of monitoring, while it's a good
attempt at a mitigation, just is not gonna work.  Not work in the sense of
not good enough / acceptable.  To paint with an overly broad brush, no one,
practically speaking, is going to do that :)

[continuing with 3] I know Pierre Jeambrun brought up the idea of
> implementing this

way of detecting SLAs in a systematic way, so that instead of users
> attaching CustomOperators to their DAGs, Airflow will add Triggers that
> invoke sla callbacks at the end of the Trigger through a configuration
> parameter on the task that we want to check the SLA on. If you think making
> this approach of measuring SLAs on the Trigger, a first class idea on
> Airflow is a good idea - I'm happy to discuss this further and also learn

more from you on how we could make this a reality.

Some running ideas that
> come to my mind - new SLA feature requires a Triggerer process; we create
> SLA watching temporal triggers if sla is defined on the task; the
> sla_miss_callback uses 'context' argument, but the deferred operator that
> is executed upon the completion of the temporal trigger has a strict
> execution_timeout of 10 seconds (or other arbitrary short duration), to
> ensure that the worker slots are not taken up by slas for a significant
> duration; a separate pool for SLA triggers and workers (sla_pool?) so that
> it doesn't use the default pool?


You lost me a bit here with some of those details.  Happy to chat about
it.  But yeah i think having the new SLA behavior rely on triggerer, I
don't see a problem with that.  And I don't see a problem with using
triggerer for more things.  Can't speak for others though.

But separately, my original question here was, how is this AIP going to be
implemented?  Does the code still run on the dag processor?  Thanks

[4] I whole-heartedly agree with you on this - Deferrable Operators are my
> favorite, and it feels bad to me that we'd be introducing a feature that
> can't be extended to deferrables at all. And since the act of missing an
> SLA isn't a final state, I'm not sure exactly how we would be able to
> invoke an SLA callback on a Deferred Operator (or its corresponding
> Trigger) in the current workflow.


Help me understand what is the problem with running SLA callback.  Where /
how do they run now / in the AIP?  And how is this more problematic with
deferrables?

On the idea of dags not being scheduled at a certain time - this is a
> pretty popular edge case that obviously falls under the subject of
> measuring SLAs, but I'm curious if it will be helpful to separate out the
> concern of maintaining a scheduler cluster with enough bandwidth, vs the
> concern of the DAG and the contained tasks running its job within its
> expected SLA, provided that the cluster is in a good state. There are
> existing reliability metrics that help us detect when the DAG runs
> themselves are being scheduled late, or if we are reaching the limit of
> worker parallelism. These are issues that will impact the delivery times of
> all of the DAGs and the Tasks, if not resolved immediately. So to me, as
> long as there are existing ways of detecting these more critical
> infrastructure issues (which there are), I am not too concerned that my SLA
> measuring might be impacted by a late scheduled DAG Run.


Yeah I think this is a legitimate point.... essentially it's like "if your
dags are running behind, you have bigger problems".  And maybe scheduling
delay is not really an issue, and so we shouldn't really worry about it --
I honestly don't know.  I saw you recently added a scheduling delay
metric... maybe you know.  Nonetheless, it does feel like that's sort of
core to what an SLA is.  From business stakeholder perspective, business
user does not care why the task is delayed -- slow scheduling vs slow
running of *this* task vs slow running of *upstream* tasks.  A business
user wants their data in accordance with the SLA.  A business user doesn't
care that X task in the pipeline took X time -- they care that their data
is refreshed when expected.  Maybe the terminology SLA is throwing me off a
bit for this reason, cus it really implies an "agreement" i.e. with someone
else i.e. external stakeholder who... probably doesn't give a hoot about
individual task duration.

It feels like what we're dealing with is various kinds of "expectations".
Not that I'm proposing a rename :) But when my dag finishes, I can set an
"expectation" that the next dag run will be running by X time.  When the
dag starts running, I can set an expectation that the dag will be *done* by
X time (and that the tasks should be done by X times).  We can have
expectations about how long certain things will take to happen.  It seems
trigger would support this very easily.  Whether we should do it or not is
another question 😬

But yeah so, perhaps in a dag with an SLA, it emits events that are
relevant to SLA logic.  And then perhaps something consumes those events
and runs triggers that enforce those expectations.  And when events are
emitted which mean trigger not needed anymore, then that "something"
processes that event and in response kills the relevant trigger.  Thinking
out loud here.  It would put a lot more tasks on the triggerer, but all
they would be doing is sleeping.









On Tue, Sep 12, 2023 at 12:48 PM Sung Yun <sy...@cornell.edu> wrote:

> Hi Daniel,
>
> Thank you for the review! I'm happy to keep having the discussion to make
> sure we can introduce the right way of implementing these solutions into
> Airflow.
>
> My general impression from the community in the discussions so far led me
> to believe that deprecating the problematic feature would be welcomed at
> large, as long as we can educate folks about how they can achieve a similar
> outcome by using Deferrable Operators - which will be much less problematic
> for the Airflow core infrastructure.
> But as you say, if you think it is important to maintain the feature until
> we can find a suitable, systematic replacement for it, I'm happy to hold
> off on the implementations until we can make the necessary edits to the
> AIP.
>
> Here are my answers to your points:
>
> [1] Yes, that's correct. I believe that containing the SLA evaluation
> within the lifetime of a task as a duration-based sla will still have a
> purpose. It's technically implemented like an execution_timeout, but the
> goal of the SLA check is to execute a callback without killing the task: so
> that the accountable owner can be alerted, or a record of the SLA miss can
> be made without killing the task and throwing away the work that's been
> done. Measuring the SLA as a duration of a task is actually a feature that
> has been requested before, by multiple folks in previous discussions and
> PRs.
>
> [2] In the accepted version of the AIP, the DAG SLA is defined from the
> start_date of the DAG. I had initially considered for it to be evaluated
> from the scheduled start time (data_interval_end), but was stuck on how we
> could avoid false alarms when dag runs are cleared, which was discussed in
> the Google Doc that was circulated
> <
> https://docs.google.com/document/d/1drNaYmAy6GqC4WGGn4MNt6VqbOwVNm7jPfmr5Pc52AU/edit#heading=h.z45mqf5nt94
> >
> before the AIP was drafted. I've submitted a small change this week to
> introduce
> the dag_run clear_number attribute
> <https://github.com/apache/airflow/pull/34126> which would actually
> mitigate this issue. So if we feel that using the data_interval_end over
> the start_date for a SCHEDULED DagRunType is more appropriate, we can make
> that change on the open DAG SLA PR
> <https://github.com/apache/airflow/pull/33532>.
>
> [3] An example of using Deferrable Operators to create Custom Operators
> that monitor the SLA of a target task is discussed in the "Are there any
> downsides to this change?" section as a workaround that will allow users to
> measure SLA[3]: Task-level SLA measured from DAG-run scheduled start time.
> I agree that this isn't an optimal message to say that we are removing an
> existing feature. But perhaps the fact that users can implement this on
> their own in a scalable manner makes the feature deprecation easier to
> swallow... I know Pierre Jeambrun brought up the idea of implementing this
> way of detecting SLAs in a systematic way, so that instead of users
> attaching CustomOperators to their DAGs, Airflow will add Triggers that
> invoke sla callbacks at the end of the Trigger through a configuration
> parameter on the task that we want to check the SLA on. If you think making
> this approach of measuring SLAs on the Trigger, a first class idea on
> Airflow is a good idea - I'm happy to discuss this further and also learn
> more from you on how we could make this a reality. Some running ideas that
> come to my mind - new SLA feature requires a Triggerer process; we create
> SLA watching temporal triggers if sla is defined on the task; the
> sla_miss_callback uses 'context' argument, but the deferred operator that
> is executed upon the completion of the temporal trigger has a strict
> execution_timeout of 10 seconds (or other arbitrary short duration), to
> ensure that the worker slots are not taken up by slas for a significant
> duration; a separate pool for SLA triggers and workers (sla_pool?) so that
> it doesn't use the default pool?
>
> [4] I whole-heartedly agree with you on this - Deferrable Operators are my
> favorite, and it feels bad to me that we'd be introducing a feature that
> can't be extended to deferrables at all. And since the act of missing an
> SLA isn't a final state, I'm not sure exactly how we would be able to
> invoke an SLA callback on a Deferred Operator (or its corresponding
> Trigger) in the current workflow.
>
> On the idea of dags not being scheduled at a certain time - this is a
> pretty popular edge case that obviously falls under the subject of
> measuring SLAs, but I'm curious if it will be helpful to separate out the
> concern of maintaining a scheduler cluster with enough bandwidth, vs the
> concern of the DAG and the contained tasks running its job within its
> expected SLA, provided that the cluster is in a good state. There are
> existing reliability metrics that help us detect when the DAG runs
> themselves are being scheduled late, or if we are reaching the limit of
> worker parallelism. These are issues that will impact the delivery times of
> all of the DAGs and the Tasks, if not resolved immediately. So to me, as
> long as there are existing ways of detecting these more critical
> infrastructure issues (which there are), I am not too concerned that my SLA
> measuring might be impacted by a late scheduled DAG Run.
>
> On the idea of killing the running sla triggers, if the task succeeds - I
> think that's a great idea, it makes sure that the Trigger is removed when
> it is no longer needed.
>
> Please let me know if you think idea [3] is worth exploring further and
> materializing as a formal proposal. I'd be happy to find time to discuss it
> more in depth :)
>
> Sung
>
> On Tue, Sep 12, 2023 at 2:25 PM Daniel Standish
> <da...@astronomer.io.invalid> wrote:
>
> > Some questions for you Sung.
> >
> > I tried looking to understand why we needed to remove behavior 3
> discussed
> > in AIP:
> >
> > *[remove]* Task-level SLA measured from DAG-run scheduled start time
> >
> >
> > I'm just a little concerned that removing this would be a mistake
> because,
> > in my mind, part of the essence of what an SLA is, is an agreement about,
> > in essence, when your data should "be there". (or the "thing" done, etc)
> >
> > The behavior replacing it seems a lot like the existing execution_timeout
> > behavior:
> >
> > *[add]* Finally, I propose that we implement a new Task-level SLA
> feature,
> > > that is contained within the bounds of the task's lifetime, and is
> > measured
> > > within the task itself.
> >
> >
> > [1] Do I understand that correctly?  If so, the question I would have is,
> > if we can already rely on execution timeout to tell us when an individual
> > task is taking too long, why do we need an SLA to tell us the same thing?
> >
> > [2] Can you also clarify how "dag slas" are defined?  (behavior 1 in the
> > AIP).  Is the duration to be applied relative to scheduled time or start
> > time?
> >
> > [3] I saw some discussion of using the triggerer component somehow in
> > this.  Is that a thing?  Can you share notes on implementation?  I looked
> > on the AIP but did not find anything.
> >
> > [4] The fact that task sla cannot be used with deferrable operators seems
> > problematic to me.  Users tend to expect (rightly or not) something like
> > "parity" between deferrable and non-deferrable tasks.  SLAs would seem to
> > fall into a category where this seems like a reasonable expectation.
> >
> > Some discussion on solutions...
> >
> > I gather that the justification for removing behavior 3 is that it's too
> > complex / expensive.  I feel like there's got to be a way.   Thinking
> about
> > how we could implement behavior 3, there's two problems that sort of push
> > us into this perceived need to query everything all the time, in a manner
> > that is prohibitive.  One is the problem that the dag may not get
> > scheduled.  We need a way to raise if the dag is not scheduled X time
> after
> > it's "schedule *at*" date.  For the moment let's call that the dag
> > scheduling timeout.  What if, for each dag with such a scheduling
> timeout,
> > we launch a trigger that sleeps until that timeout and then when that
> runs
> > out it runs a single query to determine whether the dag run was scheduled
> > -- if not, raise.  Then, for the task part of things.... well if the dag
> is
> > assumed to be reliably scheduled (and this is protected by the first part
> > here), then we can, at the time of dag run scheduling, launch triggers
> for
> > the individual tasks.  And they would behave the same way.  Run until
> > timeout and then query.
> > Perhaps better yet, perhaps when a task completes, it can delete its
> > associated "sla" triggers, so that its sla trigger would be cancelled,
> and
> > no query ever has to run.  Same thing could be done for an overall dag
> SLA
> > -- at dag run completion could kill any trigger that is monitoring it's
> > execution duration.
> >
> > Anyway, I understand that the AIP is already accepted, but just hoping we
> > can have the discussion, in case we can get to a place where the existing
> > "expected" sla behavior can perhaps be preserved in a performant manner.
> >
> > Thanks
> >
>
>
> --
> Sung Yun
> Cornell Tech '20
> Master of Engineering in Computer Science
>

Re: [DISCUSS] Mechanism of SLA

Posted by Sung Yun <sy...@cornell.edu>.
Hi Daniel,

Thank you for the review! I'm happy to keep having the discussion to make
sure we can introduce the right way of implementing these solutions into
Airflow.

My general impression from the community in the discussions so far led me
to believe that deprecating the problematic feature would be welcomed at
large, as long as we can educate folks about how they can achieve a similar
outcome by using Deferrable Operators - which will be much less problematic
for the Airflow core infrastructure.
But as you say, if you think it is important to maintain the feature until
we can find a suitable, systematic replacement for it, I'm happy to hold
off on the implementations until we can make the necessary edits to the AIP.

Here are my answers to your points:

[1] Yes, that's correct. I believe that containing the SLA evaluation
within the lifetime of a task as a duration-based sla will still have a
purpose. It's technically implemented like an execution_timeout, but the
goal of the SLA check is to execute a callback without killing the task: so
that the accountable owner can be alerted, or a record of the SLA miss can
be made without killing the task and throwing away the work that's been
done. Measuring the SLA as a duration of a task is actually a feature that
has been requested before, by multiple folks in previous discussions and
PRs.

[2] In the accepted version of the AIP, the DAG SLA is defined from the
start_date of the DAG. I had initially considered for it to be evaluated
from the scheduled start time (data_interval_end), but was stuck on how we
could avoid false alarms when dag runs are cleared, which was discussed in
the Google Doc that was circulated
<https://docs.google.com/document/d/1drNaYmAy6GqC4WGGn4MNt6VqbOwVNm7jPfmr5Pc52AU/edit#heading=h.z45mqf5nt94>
before the AIP was drafted. I've submitted a small change this week to
introduce
the dag_run clear_number attribute
<https://github.com/apache/airflow/pull/34126> which would actually
mitigate this issue. So if we feel that using the data_interval_end over
the start_date for a SCHEDULED DagRunType is more appropriate, we can make
that change on the open DAG SLA PR
<https://github.com/apache/airflow/pull/33532>.

[3] An example of using Deferrable Operators to create Custom Operators
that monitor the SLA of a target task is discussed in the "Are there any
downsides to this change?" section as a workaround that will allow users to
measure SLA[3]: Task-level SLA measured from DAG-run scheduled start time.
I agree that this isn't an optimal message to say that we are removing an
existing feature. But perhaps the fact that users can implement this on
their own in a scalable manner makes the feature deprecation easier to
swallow... I know Pierre Jeambrun brought up the idea of implementing this
way of detecting SLAs in a systematic way, so that instead of users
attaching CustomOperators to their DAGs, Airflow will add Triggers that
invoke sla callbacks at the end of the Trigger through a configuration
parameter on the task that we want to check the SLA on. If you think making
this approach of measuring SLAs on the Trigger, a first class idea on
Airflow is a good idea - I'm happy to discuss this further and also learn
more from you on how we could make this a reality. Some running ideas that
come to my mind - new SLA feature requires a Triggerer process; we create
SLA watching temporal triggers if sla is defined on the task; the
sla_miss_callback uses 'context' argument, but the deferred operator that
is executed upon the completion of the temporal trigger has a strict
execution_timeout of 10 seconds (or other arbitrary short duration), to
ensure that the worker slots are not taken up by slas for a significant
duration; a separate pool for SLA triggers and workers (sla_pool?) so that
it doesn't use the default pool?

[4] I whole-heartedly agree with you on this - Deferrable Operators are my
favorite, and it feels bad to me that we'd be introducing a feature that
can't be extended to deferrables at all. And since the act of missing an
SLA isn't a final state, I'm not sure exactly how we would be able to
invoke an SLA callback on a Deferred Operator (or its corresponding
Trigger) in the current workflow.

On the idea of dags not being scheduled at a certain time - this is a
pretty popular edge case that obviously falls under the subject of
measuring SLAs, but I'm curious if it will be helpful to separate out the
concern of maintaining a scheduler cluster with enough bandwidth, vs the
concern of the DAG and the contained tasks running its job within its
expected SLA, provided that the cluster is in a good state. There are
existing reliability metrics that help us detect when the DAG runs
themselves are being scheduled late, or if we are reaching the limit of
worker parallelism. These are issues that will impact the delivery times of
all of the DAGs and the Tasks, if not resolved immediately. So to me, as
long as there are existing ways of detecting these more critical
infrastructure issues (which there are), I am not too concerned that my SLA
measuring might be impacted by a late scheduled DAG Run.

On the idea of killing the running sla triggers, if the task succeeds - I
think that's a great idea, it makes sure that the Trigger is removed when
it is no longer needed.

Please let me know if you think idea [3] is worth exploring further and
materializing as a formal proposal. I'd be happy to find time to discuss it
more in depth :)

Sung

On Tue, Sep 12, 2023 at 2:25 PM Daniel Standish
<da...@astronomer.io.invalid> wrote:

> Some questions for you Sung.
>
> I tried looking to understand why we needed to remove behavior 3 discussed
> in AIP:
>
> *[remove]* Task-level SLA measured from DAG-run scheduled start time
>
>
> I'm just a little concerned that removing this would be a mistake because,
> in my mind, part of the essence of what an SLA is, is an agreement about,
> in essence, when your data should "be there". (or the "thing" done, etc)
>
> The behavior replacing it seems a lot like the existing execution_timeout
> behavior:
>
> *[add]* Finally, I propose that we implement a new Task-level SLA feature,
> > that is contained within the bounds of the task's lifetime, and is
> measured
> > within the task itself.
>
>
> [1] Do I understand that correctly?  If so, the question I would have is,
> if we can already rely on execution timeout to tell us when an individual
> task is taking too long, why do we need an SLA to tell us the same thing?
>
> [2] Can you also clarify how "dag slas" are defined?  (behavior 1 in the
> AIP).  Is the duration to be applied relative to scheduled time or start
> time?
>
> [3] I saw some discussion of using the triggerer component somehow in
> this.  Is that a thing?  Can you share notes on implementation?  I looked
> on the AIP but did not find anything.
>
> [4] The fact that task sla cannot be used with deferrable operators seems
> problematic to me.  Users tend to expect (rightly or not) something like
> "parity" between deferrable and non-deferrable tasks.  SLAs would seem to
> fall into a category where this seems like a reasonable expectation.
>
> Some discussion on solutions...
>
> I gather that the justification for removing behavior 3 is that it's too
> complex / expensive.  I feel like there's got to be a way.   Thinking about
> how we could implement behavior 3, there's two problems that sort of push
> us into this perceived need to query everything all the time, in a manner
> that is prohibitive.  One is the problem that the dag may not get
> scheduled.  We need a way to raise if the dag is not scheduled X time after
> it's "schedule *at*" date.  For the moment let's call that the dag
> scheduling timeout.  What if, for each dag with such a scheduling timeout,
> we launch a trigger that sleeps until that timeout and then when that runs
> out it runs a single query to determine whether the dag run was scheduled
> -- if not, raise.  Then, for the task part of things.... well if the dag is
> assumed to be reliably scheduled (and this is protected by the first part
> here), then we can, at the time of dag run scheduling, launch triggers for
> the individual tasks.  And they would behave the same way.  Run until
> timeout and then query.
> Perhaps better yet, perhaps when a task completes, it can delete its
> associated "sla" triggers, so that its sla trigger would be cancelled, and
> no query ever has to run.  Same thing could be done for an overall dag SLA
> -- at dag run completion could kill any trigger that is monitoring it's
> execution duration.
>
> Anyway, I understand that the AIP is already accepted, but just hoping we
> can have the discussion, in case we can get to a place where the existing
> "expected" sla behavior can perhaps be preserved in a performant manner.
>
> Thanks
>


-- 
Sung Yun
Cornell Tech '20
Master of Engineering in Computer Science