Uh oh!
There was an error while loading. Please reload this page.
- Notifications
You must be signed in to change notification settings - Fork 17.7k
Fix dag processor callback cleanup for versioned bundle files#66484
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
Merged
ephraimbuddy
merged 5 commits into
apache:main
from
hkc-8010:fix/dag-processor-versioned-callback-orphaningMay 13, 2026
Uh oh!
There was an error while loading. Please reload this page.
Merged
Changes from all commits
Commits
Show all changes
5 commits
Select commit
Hold shift + click to select a range
3d23c79
Fix callback orphan cleanup for versioned bundle files
hkc-8010 fda9cb3
Remove newsfragment for callback orphan cleanup fix
hkc-8010 ec11c0a
Fix versioned dag file presence checks
hkc-8010 a328aae
Preserve public signatures for versioned dag file checks
hkc-8010 9990b66
Preserve manager cleanup extension points
hkc-8010 File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Uh oh!
There was an error while loading. Please reload this page.
Jump to
Jump to file
Failed to load files.
Loading
Uh oh!
There was an error while loading. Please reload this page.
Diff view
Diff view
There are no files selected for viewing
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
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -129,6 +129,11 @@ def absolute_path(self) -> Path: | ||
| raise ValueError("bundle_path not set") | ||
| return self.bundle_path / self.rel_path | ||
| @property | ||
| def presence_key(self) -> tuple[str, Path]: | ||
| """Return the stable file identity used for presence checks.""" | ||
| return self.bundle_name, self.rel_path | ||
| def _config_int_factory(section: str, key: str): | ||
| return functools.partial(conf.getint, section, key) | ||
| @@ -1014,20 +1019,22 @@ def handle_removed_files(self, known_files: dict[str, set[DagFileInfo]]): | ||
| def purge_removed_files_from_queue(self, present: set[DagFileInfo]): | ||
| """Remove from queue any files no longer observed locally.""" | ||
| self._file_queue = deque(x for x in self._file_queue if x in present) | ||
| present_keys = {file.presence_key for file in present} | ||
| self._file_queue = deque(x for x in self._file_queue if x.presence_key in present_keys) | ||
| stats.gauge("dag_processing.file_path_queue_size", len(self._file_queue)) | ||
| def remove_orphaned_file_stats(self, present: set[DagFileInfo]): | ||
| """Remove the stats for any dag files that don't exist anymore.""" | ||
| # todo: store stats by bundle also? | ||
| stats_to_remove = set(self._file_stats).difference(present) | ||
| present_keys = {file.presence_key for file in present} | ||
| stats_to_remove = {file for file in self._file_stats if file.presence_key not in present_keys} | ||
| for file in stats_to_remove: | ||
| del self._file_stats[file] | ||
| def terminate_orphan_processes(self, present: set[DagFileInfo]): | ||
| """Stop processors that are working on deleted files.""" | ||
| present_keys = {file.presence_key for file in present} | ||
| for file in list(self._processors.keys()): | ||
| if file not in present: | ||
| if file.presence_key not in present_keys: | ||
| processor = self._processors.pop(file, None) | ||
| if not processor: | ||
| continue | ||
| @@ -1261,11 +1268,14 @@ def _add_new_files_to_queue(self, known_files: dict[str, set[DagFileInfo]]): | ||
| A "new" file is a file that has not been processed yet and is not currently being processed. | ||
| """ | ||
| new_files = [] | ||
| tracked_presence_keys = {file.presence_key for file in self._file_queue} | ||
| tracked_presence_keys.update(file.presence_key for file in self._file_stats) | ||
| tracked_presence_keys.update(file.presence_key for file in self._processors) | ||
| for files in known_files.values(): | ||
| for file in files: | ||
| # todo: store stats by bundle also? | ||
| if file not in self._file_stats and file not in self._processors: | ||
| if file.presence_key not in tracked_presence_keys: | ||
| new_files.append(file) | ||
| tracked_presence_keys.add(file.presence_key) | ||
| if new_files: | ||
| self.log.info("Adding %d new files to the front of the queue", len(new_files)) | ||
| @@ -1290,27 +1300,43 @@ def _resort_file_queue(self): | ||
| self._file_queue = deque(callback_files + sorted_regular_files) | ||
| def _sort_by_mtime(self, files: Iterable[DagFileInfo]): | ||
| file_stats_by_presence_key = {file.presence_key: stat for file, stat in self._file_stats.items()} | ||
| files_with_mtime: dict[DagFileInfo, float] = {} | ||
| changed_recently = set() | ||
| for file in files: | ||
| try: | ||
| modified_timestamp = os.path.getmtime(file.absolute_path) | ||
| modified_datetime = datetime.fromtimestamp(modified_timestamp, tz=timezone.utc) | ||
| files_with_mtime[file] = modified_timestamp | ||
| last_time = self._file_stats[file].last_finish_time | ||
| stat = file_stats_by_presence_key.get(file.presence_key) | ||
| last_time = stat.last_finish_time if stat else None | ||
| if not last_time: | ||
| continue | ||
| if modified_datetime > last_time: | ||
| changed_recently.add(file) | ||
| except FileNotFoundError: | ||
| self.log.warning("Skipping processing of missing file: %s", file) | ||
| self._file_stats.pop(file, None) | ||
| stats_to_remove = [ | ||
| tracked_file | ||
| for tracked_file in self._file_stats | ||
| if tracked_file.presence_key == file.presence_key | ||
| ] | ||
| for tracked_file in stats_to_remove: | ||
| self._file_stats.pop(tracked_file, None) | ||
| continue | ||
| file_infos = [info for info, ts in sorted(files_with_mtime.items(), key=itemgetter(1), reverse=True)] | ||
| return file_infos, changed_recently | ||
| def processed_recently(self, now, file): | ||
| last_time = self._file_stats[file].last_finish_time | ||
| stat = next( | ||
| ( | ||
| stat | ||
| for tracked_file, stat in self._file_stats.items() | ||
| if tracked_file.presence_key == file.presence_key | ||
| ), | ||
| None, | ||
| ) | ||
| last_time = stat.last_finish_time if stat else None | ||
| if not last_time: | ||
| return False | ||
| elapsed_ss = (now - last_time).total_seconds() | ||
| @@ -1335,7 +1361,8 @@ def prepare_file_queue(self, known_files: dict[str, set[DagFileInfo]]): | ||
| # If the file path is already being processed, or if a file was | ||
| # processed recently, wait until the next batch | ||
| in_progress = set(self._processors) | ||
| in_progress_keys = {file.presence_key for file in self._processors} | ||
| file_stats_by_presence_key = {file.presence_key: stat for file, stat in self._file_stats.items()} | ||
hkc-8010 marked this conversation as resolved.
Uh oh!There was an error while loading. Please reload this page. | ||
| now = timezone.utcnow() | ||
| # Sort the file paths by the parsing order mode | ||
| @@ -1345,7 +1372,9 @@ def prepare_file_queue(self, known_files: dict[str, set[DagFileInfo]]): | ||
| for bundle_files in known_files.values(): | ||
| for file in bundle_files: | ||
| files.append(file) | ||
| if self.processed_recently(now, file): | ||
| stat = file_stats_by_presence_key.get(file.presence_key) | ||
| last_time = stat.last_finish_time if stat else None | ||
| if last_time and (now - last_time).total_seconds() < self._file_process_interval: | ||
| recently_processed.add(file) | ||
| changed_recently: set[DagFileInfo] = set() | ||
| @@ -1358,15 +1387,19 @@ def prepare_file_queue(self, known_files: dict[str, set[DagFileInfo]]): | ||
| # set of files. Since we set the seed, the sort order will remain same per host | ||
| random.Random(get_hostname()).shuffle(files) | ||
| at_run_limit = [info for info, stat in self._file_stats.items() if stat.run_count == self.max_runs] | ||
| to_exclude = in_progress.union(at_run_limit) | ||
| at_run_limit_keys = { | ||
| presence_key | ||
| for presence_key, stat in file_stats_by_presence_key.items() | ||
| if stat.run_count == self.max_runs | ||
| } | ||
| to_exclude = in_progress_keys.union(at_run_limit_keys) | ||
| # exclude recently processed unless changed recently | ||
| to_exclude |= recently_processed - changed_recently | ||
| to_exclude |= {file.presence_key for file in recently_processed - changed_recently} | ||
| # Do not convert the following list to set as set does not preserve the order | ||
| # and we need to maintain the order of files for `[dag_processor] file_parsing_sort_mode` | ||
| to_queue = [x for x in files if x not in to_exclude] | ||
| to_queue = [x for x in files if x.presence_key not in to_exclude] | ||
| if self.log.isEnabledFor(logging.DEBUG): | ||
| for path, processor in self._processors.items(): | ||
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
Oops, something went wrong.
Uh oh!
There was an error while loading. Please reload this page.
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.
Uh oh!
There was an error while loading. Please reload this page.