potiuk commented on pull request #18068: URL: https://github.com/apache/airflow/pull/18068#issuecomment-915994036
Hm @ashb @jedcunningham .. The more I think about it, the more I believe the current behaviour of the image is actually CORRECT and we should only modify the chart, not the Dockerfile/image (and possibly ad documentation to the entrypoint detailing the signal processing). It think the dumb-init behaviour is really good and protective when you run stuff with `bash -c "xxxx"` command. By defaul, whatever usual process you run inside, it will just propagate the TERM to the whole process group and it will be killing stuff nicely. The case where we do not want to do it and send it to a single "master" process, is really few and far between. Celery is the only component we have I think that provides this nice handling of TERM signal and puts the whole process in "offline" mode. And even that, I think the correct behaviour should be that Celery should start their worker processes in a different process group (and I actually wonder actually why it is not happening currently - I tried to find it in the Celery docs but failed so far). This would nicely work also if celery works from terminal (Ctrl-C sends TERM signal to the whole process group by default). So I'd imagine Celery "could" be put in the mode that workers are spawned in separate process group. And even if not, then telling specifically in the chart to set `DUMB_INIT_SETSID=0` ONLY for celery workers in chart (and adding some explanation in image entrypoint documentation about it) might be the RIGHT solution. Or maybe we can enable separate process group for Celery workers if that's a possibility. It's backwards compatible (and the behaviour is perfectly reasonable), and it makes sense to expect that Celery needs "special treatment" in this case. WDYT? -- 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]
