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.

Sign up for GitHub

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

Jump to bottom

Make sure backfill deadlocks raise errors #1290

Merged
merged 3 commits into from
Apr 5, 2016
Merged

Make sure backfill deadlocks raise errors #1290

merged 3 commits into from
Apr 5, 2016

Conversation

jlowin
Copy link
Member

@jlowin jlowin commented Apr 4, 2016

Closes #1289

Deadlocked backfills didn’t raise AirflowExceptions, so SubDagOperators didn’t recognize that their subdags were failing. This PR fixes that and adds a unit test for the situation.

It also contains improvements for running DAGs in unit tests

@landscape-bot
Copy link

Code Health
Repository health decreased by 0.02% when pulling acf3b63 on jlowin:subdag-backfill-status into 0f28090 on airbnb:master.

@jlowin jlowin force-pushed the subdag-backfill-status branch from acf3b63 to dc3a366 Compare April 4, 2016 02:06
@landscape-bot
Copy link

Code Health
Code quality remained the same when pulling dc3a366 on jlowin:subdag-backfill-status into 5ab2567 on airbnb:master.

@syvineckruyk
Copy link
Contributor

👍

@landscape-bot
Copy link

Code Health
Code quality remained the same when pulling 17959c8 on jlowin:subdag-backfill-status into 5ab2567 on airbnb:master.

@jlowin jlowin force-pushed the subdag-backfill-status branch from 17959c8 to e6d9897 Compare April 4, 2016 03:23
@landscape-bot
Copy link

Code Health
Code quality remained the same when pulling e6d9897 on jlowin:subdag-backfill-status into 5ab2567 on airbnb:master.

@jlowin jlowin force-pushed the subdag-backfill-status branch from e6d9897 to c85bc8d Compare April 4, 2016 19:31
@landscape-bot
Copy link

Code Health
Repository health decreased by 0.00% when pulling c85bc8d on jlowin:subdag-backfill-status into 0bae60f on airbnb:master.

@coveralls
Copy link

Coverage Status

Coverage decreased (-0.3%) to 66.753% when pulling c85bc8d4df7db8805a2a1bc408fd154adb44f228 on jlowin:subdag-backfill-status into 0bae60f on airbnb:master.

@landscape-bot
Copy link

Code Health
Code quality remained the same when pulling a4cda98 on jlowin:subdag-backfill-status into 0bae60f on airbnb:master.

@jlowin jlowin mentioned this pull request Apr 4, 2016
Closed
@coveralls
Copy link

Coverage Status

Coverage decreased (-0.0003%) to 67.058% when pulling a4cda9897b820071303c3e2b1faeba04ab57afac on jlowin:subdag-backfill-status into 0bae60f on airbnb:master.

@jlowin jlowin force-pushed the subdag-backfill-status branch from fcc928a to 8f7e5d4 Compare April 4, 2016 23:12
@landscape-bot
Copy link

Code Health
Code quality remained the same when pulling 8f7e5d4 on jlowin:subdag-backfill-status into 0bae60f on airbnb:master.

@coveralls
Copy link

Coverage Status

Coverage increased (+0.6%) to 67.631% when pulling fcc928aaae99c74a2d9c87a94ff44f1b34af51f2 on jlowin:subdag-backfill-status into 0bae60f on airbnb:master.

@coveralls
Copy link

Coverage Status

Coverage increased (+0.7%) to 67.76% when pulling 8f7e5d44b442b7185919866c43edccef313f0533 on jlowin:subdag-backfill-status into 0bae60f on airbnb:master.

@jlowin jlowin force-pushed the subdag-backfill-status branch from 8f7e5d4 to e05d1cb Compare April 4, 2016 23:58
@jlowin
Copy link
Member Author

jlowin commented Apr 4, 2016

@syvineckruyk there are a lot of fixes in this PR so I'm going to say it's "done" -- still thinking about one of the cases we discussed offline and will address it separately.

@landscape-bot
Copy link

Code Health
Code quality remained the same when pulling e05d1cb on jlowin:subdag-backfill-status into 0bae60f on airbnb:master.

@coveralls
Copy link

Coverage Status

Coverage increased (+0.7%) to 67.764% when pulling e05d1cb0c25c03a8d22daf5970deeb57eb5cc308 on jlowin:subdag-backfill-status into 0bae60f on airbnb:master.

@jlowin jlowin mentioned this pull request Apr 5, 2016
Merged
bolkedebruin
bolkedebruin reviewed Apr 5, 2016
View reviewed changes
tests/core.py
@@ -42,7 +42,7 @@
DEFAULT_DATE_ISO = DEFAULT_DATE.isoformat()
DEFAULT_DATE_DS = DEFAULT_DATE_ISO[:10]
TEST_DAG_ID = 'unit_tests'
configuration.test_mode()

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

configuration.test_mode() is used through out core.py. Should these all be replaced?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@bolkedebruin I deleted this one because it was already called on line 22 of tests/core.py -- but to be completely honest, I'm not sure if configuration.test_mode() actually does anything?

This is the entire function:

def test_mode():
    conf = ConfigParserWithDefaults(defaults)
    conf.read(TEST_CONFIG)

It creates a new conf object with the test parameters, but doesn't return it or overwrite the existing one. And it doesn't change values that were already read-in at the module level (like DAGS_FOLDER at the top of models.py)

@jlowin
Set DAG_FOLDER for unit tests
4763720 
When tests are running, the default DAG_FOLDER becomes
`airflow/tests/dags`. This makes it much easier to execute DAGs in unit
tests in a standardized manner.

Also exports DAGS_FOLDER as an env var for Travis
@jlowin jlowin force-pushed the subdag-backfill-status branch from e05d1cb to e3da897 Compare April 5, 2016 15:07
@landscape-bot
Copy link

Code Health
Code quality remained the same when pulling e3da897 on jlowin:subdag-backfill-status into 6581858 on airbnb:master.

@jlowin jlowin force-pushed the subdag-backfill-status branch from e3da897 to d1b5be9 Compare April 5, 2016 17:17
@landscape-bot
Copy link

Code Health
Code quality remained the same when pulling d1b5be9 on jlowin:subdag-backfill-status into 6581858 on airbnb:master.

@coveralls
Copy link

Coverage Status

Coverage increased (+0.7%) to 67.766% when pulling d1b5be939fde25c9e200f79e8b18592b42c13412 on jlowin:subdag-backfill-status into 6581858 on airbnb:master.

@jlowin jlowin mentioned this pull request Apr 5, 2016
Closed
@jlowin jlowin removed the In Progress label Apr 5, 2016
@jlowin jlowin self-assigned this Apr 5, 2016
jlowin added 2 commits April 5, 2016 14:51
@jlowin
Fix handling of deadlocked jobs
2e0421a 
- Raise an error when a backfill deadlocks

Deadlocked backfills didn’t raise AirflowExceptions, so
SubDagOperators didn’t recognize that their subdags
were failing.

- Fix bug with marking DagRuns as failed

- Let SchedulerJob mark DagRuns as deadlocked when there
are no TIs available; other deadlock metrics depend on TIs

- Adds unit tests.
@jlowin
Fix infinite retries with pools, with test
b2844af 
Addresses the issue raised in apache#1299
@jlowin jlowin force-pushed the subdag-backfill-status branch from d1b5be9 to b2844af Compare April 5, 2016 18:51
@landscape-bot
Copy link

Code Health
Code quality remained the same when pulling b2844af on jlowin:subdag-backfill-status into 58abca2 on airbnb:master.

@coveralls
Copy link

Coverage Status

Coverage increased (+0.5%) to 67.766% when pulling b2844af on jlowin:subdag-backfill-status into 58abca2 on airbnb:master.

@bolkedebruin bolkedebruin merged commit fd9388c into apache:master Apr 5, 2016
@jlowin jlowin deleted the subdag-backfill-status branch April 6, 2016 18:42
@jlowin jlowin mentioned this pull request Apr 15, 2016
Closed
@syvineckruyk syvineckruyk mentioned this pull request Sep 7, 2019
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees

@jlowin jlowin

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@jlowin @landscape-bot @syvineckruyk @coveralls @bolkedebruin