hkc-8010 commented on PR #65002: URL: https://github.com/apache/airflow/pull/65002#issuecomment-4862962380
Hi @Saddala — following up here since this has been sitting in draft since mid-May. Looking at the current diff, it looks like you already addressed @ashb's "don't close the log until we've drained the data" comment from May 9: `_deregister_processor_sockets()` now reads through each socket's registered handler (the same `(handler, on_close)` pair `_service_subprocess` uses) until it hits EOF or a read error, and only *then* calls `on_close`/closes the socket — and this whole drain happens before `logger_filehandle.close()` in both `terminate_orphan_processes` and `_kill_timed_out_processors`. That lines up with the shared-selector root cause in #64959 (the manager creates one `selector` in `prepare_process_context` and passes it to every spawned `DagFileProcessorProcess`, so a stale event from a just-killed processor can land in the same selector the manager keeps polling). That fix just never got surfaced back to the thread, so it's reading as unaddressed. A few asks to get this moving: 1. Could you reply to ashb's review comment confirming this is the intended fix (or correct me if I'm misreading the drain logic)? 2. The branch is quite far behind `main` now and shows merge conflicts — a rebase is needed before CI can run cleanly again. 3. Once rebased and the thread is resolved, flip it back to "Ready for review" so it re-enters the maintainer queue. We're seeing this crash independently as well (matches #64959/#64996 exactly), so happy to help review promptly once it's back up. Thanks for the work here! -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
