Fix webserver exiting when gunicorn master crashes. Closes #13469

[drago-f5a]

As described in #13469, when gunicorn master processes exits, the webserver process continues to run indefinitely, periodically logging an error message.

The method _spawn_new_workers() attempts to spawn new workers, and if that fails the webserver is meant to exit. However, this does not currently work due to a logical mistake when computing number of workers to spawn, and a typo (passing current number of workers instead of the count of workers to spawn).

Btw, I believe the same issue is present in the 2.0 branch.

[boring-cyborg]

Congratulations on your first Pull Request and welcome to the Apache Airflow community! If you have any issues or are unsure about any anything please check our Contribution Guide (https://github.com/apache/airflow/blob/master/CONTRIBUTING.rst)
Here are some useful points:

• Pay attention to the quality of your code (flake8, pylint and type annotations). Our pre-commits will help you with that.
• In case of a new feature add useful documentation (in docstrings or in docs/ directory). Adding a new operator? Check this short guide Consider adding an example DAG that shows how users should use it.
• Consider using Breeze environment for testing locally, it’s a heavy docker but it ships with a working Airflow and a lot of integrations.
• Be patient and persistent. It might take some time to get a review or get the final approval from Committers.
• Please follow ASF Code of Conduct for all communication including (but not limited to) comments on Pull Requests, Mailing list and Slack.
• Be sure to read the Airflow Coding style.
  Apache Airflow is a community-driven project and together we are making it better 🚀.
  In case of doubts contact the developers at:
  Mailing List: [email protected]
  Slack: https://s.apache.org/airflow-slack

[mik-laj]

@drago-f5a can you create a new PR for Airflow 2.0? We do not accept new changes to Airflow 1.10, unless they have already been merged to Airflow 2.0 It would still be nice to add tests to prevent regression also.

airflow/tests/cli/commands/test_webserver_command.py

Line 37 in c2ead47

class TestGunicornMonitor(unittest.TestCase):

Thanks for finding this bug. I suspect it might not have been easy.

[drago-f5a]

@mik-laj I've created a PR for 2.0, and updated/added tests. See:
#13518

Note that you can not cherry-pick the fix into 1.10, due to the package reorganization.

[jhtimmins]

@drago-f5a @potiuk Is this still relevant or should this PR be closed?

[drago-f5a]

@drago-f5a @potiuk Is this still relevant or should this PR be closed?

@jhtimmins I believe the plan is to backport #13518 to 1.10.15. However, given the CLI reorganization that occurred between 1.10.x and 2.0.0, the commit from #13518 can not be cherry-picked into v1-10-stable branch. This PR here is essentially the backport, minus the tests.

If it seems like I am dancing around answering the question, it's only because I am not familiar with the backporting process you follow. Should this PR be used for the purpose of backporting? If yes, should we also backport the tests from #13518? Let me know if I can be of any help to move this forward.

[github-actions]

The PR most likely needs to run full matrix of tests because it modifies parts of the core of Airflow. However, committers might decide to merge it quickly and take the risk. If they don't merge it quickly - please rebase it to the latest master at your convenience, or amend the last commit of the PR, and push it with --force-with-lease.

[github-actions]

The Workflow run is cancelling this PR. Building images for the PR has failed. Follow the the workflow link to check the reason.