Properly check the existence of missing mapped TIs #25788
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
ashb
reviewed
Aug 18, 2022
ashb
reviewed
Aug 18, 2022
ashb
reviewed
Aug 18, 2022
ashb
changed the title
ashb
reviewed
Aug 18, 2022
ephraimbuddy
force-pushed
the
fix-unnecessary-calls
branch
2 times, most recently
from
db598ad to
51af409
Compare
ashb
reviewed
Aug 18, 2022
ephraimbuddy
force-pushed
the
fix-unnecessary-calls
branch
4 times, most recently
from
904bcb9 to
bb06806
Compare
uranusjr
reviewed
Aug 19, 2022
ephraimbuddy
force-pushed
the
fix-unnecessary-calls
branch
from
c536b31 to
8516d24
Compare
uranusjr
reviewed
Aug 19, 2022
uranusjr
reviewed
Aug 19, 2022
ephraimbuddy
force-pushed
the
fix-unnecessary-calls
branch
from
8516d24 to
07249ad
Compare
uranusjr
reviewed
Aug 25, 2022
uranusjr
reviewed
Aug 25, 2022
Comment on lines
+1066
to
+1071
| total_length = ( | ||
| task.parse_time_mapped_ti_count | ||
| or task.run_time_mapped_ti_count(self.run_id, session=session) | ||
| or 0 | ||
| ) |
There was a problem hiding this comment.
This induces an unnecessary query of parse_time_mapped_ti_count is 0. Reading how run_time_mapped_ti_count is used, some refactoring might be a good idea. I’ll do it after this PR is done.
uranusjr
reviewed
Aug 25, 2022
ephraimbuddy
force-pushed
the
fix-unnecessary-calls
branch
from
219ccfb to
c2dc5e6
Compare
uranusjr
approved these changes
Aug 25, 2022
ephraimbuddy
force-pushed
the
fix-unnecessary-calls
branch
from
cc38cb2 to
da6f608
Compare
The previous implementation of missing indexes was not right. Missing indexes were being checked everytime that `task_instance_scheduling_decision` was called. The missing tasks should only be revised after expanding of last resort for mapped tasks have been done. If we find that a task is in schedulable state and has already been expanded, we revise their indexes and ensure they are complete. Missing indexes are marked as removed. This implementation allows the revision to be done in one place
Co-authored-by: Tzu-ping Chung <[email protected]>
Co-authored-by: Tzu-ping Chung <[email protected]>
ephraimbuddy
force-pushed
the
fix-unnecessary-calls
branch
from
da6f608 to
23a546e
Compare
anja-istenic
pushed a commit
to anja-istenic/airflow
that referenced
this pull request
Aug 29, 2022
Also a chapter was added to recommend taking a backup before the migration. Based on discussions and user input from apache#25866, apache#24526 Closes: apache#24526 Improve cleanup of temporary files in CI (apache#25957) After recent change in Paralell execution, we start to have infrequent "no space left on device" message - likely caused by the /tmp/ generated files clogging the filesystem from multiple runs. We could fix it by simply running cleanup after parallel job always, but this is not good due to diagnostics needed when debugging parallel runs locally so we need to have a way to skip /tmp files deletion. This PR fixes the problem twofold: * cleanup breeze instructions which is run at the beginning of every job cleans also /tmp file * the parallel jobs cleans after themselvs unless skipped. Properly check the existence of missing mapped TIs (apache#25788) The previous implementation of missing indexes was not correct. Missing indexes were being checked every time that `task_instance_scheduling_decision` was called. The missing tasks should only be revised after expanding of last resort for mapped tasks have been done. If we find that a task is in schedulable state and has already been expanded, we revise its indexes and ensure they are complete. Missing indexes are marked as removed. This implementation allows the revision to be done in one place Co-authored-by: Tzu-ping Chung <[email protected]> Fix dataset_event_manager resolution (apache#25943) Appears `__init__` is not invoked as part of `_run_raw_task` due to the way TI is refreshed from db. Centralize dataset manager instantiation instead. Fix unhashable issue with secrets.backend_kwargs and caching (apache#25970) Resolves apache#25968 Fix response schema for list-mapped-task-instance (apache#25965) update areActiveRuns, fix states (apache#25962)
2 tasks
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
The previous implementation of missing indexes was not right. Missing indexes
were being checked everytime that
task_instance_scheduling_decisionwas called.The missing tasks should only be revised after expanding of last resort for mapped tasks have been done. If we find that a task is in schedulable state and has already been expanded, we revise their indexes and ensure they are complete. Missing indexes are marked as removed.
This implementation allows the revision to be done in one place
Closes: #25681