fix: preserve hostname/start_date in task_instance_history when orphan-reset discards a running attempt - #71150
Open
nagasrisai wants to merge 5 commits into
Open
Conversation
…han reset When adopt_or_reset_orphaned_tasks() resets a task instance it calls prepare_db_for_next_try(), which in turn calls TaskInstanceHistory.record_ti() to archive the current attempt before issuing the next try. The query in adopt_or_reset_orphaned_tasks() uses load_only() with only seven columns (id, dag_id, task_id, run_id, map_index, state, external_executor_id), so hostname, start_date, end_date, and duration are deferred. When TaskInstanceHistory.__init__ iterates every column of the history table it triggers a separate lazy-SELECT round-trip for each deferred attribute, and any task whose pod was killed before the task-sdk could call the execution API would have hostname='' and start_date=None in the DB — values that end up verbatim in the history row. An empty-string hostname in task_instance_history causes FileTaskHandler to build the served-log URL as http://:8793/log/..., which Python's urllib rejects with "No host supplied" — the error visible in the UI for each earlier failed retry attempt. Changes ------- * scheduler_job_runner.py: add hostname, start_date, end_date, duration, and try_number to the load_only() clause in adopt_or_reset_orphaned_tasks() so all fields needed by record_ti() are fetched in the same query as the row-lock, eliminating the per-attribute lazy-SELECT round-trips. * taskinstancehistory.py: in TaskInstanceHistory.__init__, convert hostname=='' to None before storing in the history row. TaskInstance initialises hostname to "" (not None), so a task that never contacted the execution API leaves an empty string in the DB. Storing NULL instead lets callers distinguish 'hostname was never reported' from a real hostname, and prevents the http://:8793 log URL construction. Tests ----- * test_adopt_or_reset_resettable_tasks_preserves_execution_metadata_in_history: verifies that a task in RUNNING state with a real hostname has that hostname preserved in the history row after an orphan reset. * test_adopt_or_reset_resettable_tasks_stores_null_hostname_when_never_reported: verifies that a task whose hostname was never set (pod killed before task-sdk reported back) produces a history row with hostname=NULL rather than hostname=''.
…) after normalization fix
This file contains hidden or 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
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.
Fixes #65366.
What happened
Under
KubernetesExecutor, failed retry attempts end up withhostname=''andstart_date=NULLintask_instance_history. The UI then builds a broken log URL:Root cause
adopt_or_reset_orphaned_tasks()fetches TIs withload_only()covering only 7 columns —hostname,start_date,end_date, anddurationare all deferred. Whenrecord_ti()archives the attempt it accesses every history column, firing a lazy-SELECT per deferred attribute. For pods killed before the task-sdk could reach the execution API, those SELECTs returnhostname=""(the Python-level default, not SQL NULL).TaskInstanceHistory.__init__copies that empty string verbatim into history, which is what the log URL builder turns intohttp://:8793.Fix
scheduler_job_runner.py— addhostname,start_date,end_date,duration, andtry_numberto theload_only()clause so all fields needed byrecord_ti()are fetched in the same locked query.taskinstancehistory.py— in__init__, converthostname == ""toNonebefore storing, so history records an explicit NULL ("never reported") instead of an empty string that breaks URL construction.Tests
test_adopt_or_reset_resettable_tasks_preserves_execution_metadata_in_history— real hostname is carried through correctly after an orphan reset.test_adopt_or_reset_resettable_tasks_stores_null_hostname_when_never_reported— a task whose pod died before the task-sdk reported back storeshostname=NULL, nothostname="".