Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[AIRFLOW-5811] add metric for externally killed task count #6466

Merged
merged 1 commit into from
Nov 6, 2019

Conversation

houqp
Copy link
Member

@houqp houqp commented Oct 30, 2019

Make sure you have checked all steps below.

Jira

  • My PR addresses the following Airflow Jira issues and references them in the PR title. For example, "[AIRFLOW-XXX] My Airflow PR"
    • https://issues.apache.org/jira/browse/AIRFLOW-XXX
    • In case you are fixing a typo in the documentation you can prepend your commit with [AIRFLOW-XXX], code changes always need a Jira issue.
    • In case you are proposing a fundamental code change, you need to create an Airflow Improvement Proposal (AIP).
    • In case you are adding a dependency, check if the license complies with the ASF 3rd Party License Policy.

Description

We found this to be a very good signal to alert for any init container related issue. Our worker cluster was affected by a bug in EKS CNI plugin. This metric would have caught the issue automatically.

Tests

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

Looks like we don't test for stats?

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.
    • All the public functions and the classes in the PR contain docstrings that explain what it does
    • If you implement backwards incompatible changes, please leave a note in the Updating.md so we can assign it to a appropriate release

@houqp houqp force-pushed the stats_task_externally_killed branch 3 times, most recently from 10d53ad to 5efe9ea Compare October 30, 2019 19:00
@codecov-io
Copy link

codecov-io commented Oct 30, 2019

Codecov Report

Merging #6466 into master will decrease coverage by 0.36%.
The diff coverage is 100%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #6466      +/-   ##
==========================================
- Coverage   83.95%   83.58%   -0.37%     
==========================================
  Files         635      635              
  Lines       36657    36662       +5     
==========================================
- Hits        30774    30645     -129     
- Misses       5883     6017     +134
Impacted Files Coverage Δ
airflow/jobs/scheduler_job.py 74.96% <100%> (+0.03%) ⬆️
airflow/operators/postgres_operator.py 0% <0%> (-100%) ⬇️
airflow/kubernetes/volume_mount.py 44.44% <0%> (-55.56%) ⬇️
airflow/kubernetes/volume.py 52.94% <0%> (-47.06%) ⬇️
airflow/kubernetes/pod_launcher.py 45.25% <0%> (-46.72%) ⬇️
airflow/kubernetes/kube_client.py 33.33% <0%> (-41.67%) ⬇️
...rflow/contrib/operators/kubernetes_pod_operator.py 70.14% <0%> (-28.36%) ⬇️
airflow/hooks/postgres_hook.py 94.73% <0%> (-1.76%) ⬇️
airflow/utils/sqlalchemy.py 91.52% <0%> (-1.7%) ⬇️
airflow/hooks/dbapi_hook.py 88.98% <0%> (-1.7%) ⬇️
... and 4 more

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 f68c186...e4f5125. Read the comment docs.

airflow/jobs/scheduler_job.py Outdated Show resolved Hide resolved
@houqp houqp force-pushed the stats_task_externally_killed branch from 5efe9ea to 25be564 Compare November 1, 2019 18:34
@houqp houqp requested a review from ashb November 1, 2019 18:35
@houqp houqp force-pushed the stats_task_externally_killed branch from 25be564 to 049744f Compare November 1, 2019 19:30
@houqp houqp force-pushed the stats_task_externally_killed branch from 049744f to e4f5125 Compare November 1, 2019 21:08
@feng-tao feng-tao merged commit 6bcbd48 into apache:master Nov 6, 2019
@houqp houqp deleted the stats_task_externally_killed branch November 6, 2019 20:51
kaxil pushed a commit that referenced this pull request Dec 17, 2019
ashb pushed a commit that referenced this pull request Dec 17, 2019
ashb pushed a commit that referenced this pull request Dec 19, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants