[AIRFLOW-2865] Call success_callback before updating task state

[evizitei]

This race condition bites us rather often in our success callbacks, hoping to get this included in an upcoming release.

Jira

• My PR addresses the following Airflow Jira issues and references them in the PR title.

Description

• Here are some details about my PR, including screenshots of any UI changes:

In cases where the success callback takes variable
time, it's possible for it to interrupted by the heartbeat process.
This is because the heartbeat process looks for tasks that are no
longer in the "running" state but are still executing and reaps them.

This commit reverses the order of callback invocation and state
updating so that the "SUCCESS" state for the task isn't committed
to the database until after the success callback has finished.

Tests

• My PR adds the following unit tests OR does not need testing for this extremely good reason:
  TaskInstanceTest.test_success_callbak_no_race_condition

Commits

• My commits all reference Jira issues in their subject lines, and I have squashed multiple commits if they address the same issue. In addition, my commits follow the guidelines from "How to write a good git commit message":
  1. Subject is separated from body by a blank line
  2. Subject is limited to 50 characters (not including Jira issue reference)
  3. Subject does not end with a period
  4. Subject uses the imperative mood ("add", not "adding")
  5. Body wraps at 72 characters
  6. Body explains "what" and "why", not "how"

Documentation

• In case of new functionality, my PR adds documentation that describes how to use it.
  • When adding new operators/hooks/sensors, the autoclass documentation generation needs to be added.

Code Quality

• Passes flake8

[codecov-io]

Codecov Report

Merging #4082 into master will increase coverage by <.01%.
The diff coverage is 100%.

@@            Coverage Diff             @@
##           master    #4082      +/-   ##
==========================================
+ Coverage   77.91%   77.91%   +<.01%     
==========================================
  Files         199      199              
  Lines       15958    15957       -1     
==========================================
  Hits        12433    12433              
+ Misses       3525     3524       -1

Impacted Files	Coverage Δ
airflow/models.py	92.2% <100%> (+0.03%)	⬆️

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 829086c...8a41998. Read the comment docs.

[evizitei]

@ashb or @Fokko hey y'all, I don't want to be pushy or anything, I just want to know what the right way is to get feedback on a PR; I've had this one open for about 10 days and because there's no comments on it I wonder if I've done something wrong to make it de-prioritized? Should I have been making noise in a mailing list somewhere or something like that? Thanks, ~Ethan

[ashb]

@evizitei No, nothing you've done wrong - it's a lack of time on the part of the (volunteer-based) committers. (We all only work on Airflow in spare moments or when we convince our employers to lets us carve out some time. :(

[ashb]

Thanks for the detailed description, it made this much much easier to review and reason about :D

[evizitei]

Sure, I understand, just wanted to make sure I hadn't accidentally kicked myself into a process black hole :) Thanks for accepting my change, this will make the next version of airflow a valuable upgrade for us