Skip to content

Raise config error on duplicate process_name within a group - #1726

Open
bardia-key wants to merge 2 commits into
Supervisor:mainfrom
bardia-key:fix_duplicate_process_name_bug
Open

Raise config error on duplicate process_name within a group#1726
bardia-key wants to merge 2 commits into
Supervisor:mainfrom
bardia-key:fix_duplicate_process_name_bug

Conversation

@bardia-key

Copy link
Copy Markdown

Previously, when a [group:x] section combined two or more programs whose process_name expressions expanded to the same value (most commonly when numprocs > 1 is used and process_name does not include %(program_name)s), ProcessGroupBase.init silently dropped the collision: processes for a group are stored in a dict keyed by their expanded process name, so the later program's process configs simply overwrote the earlier program's entries in that dict. The result was that one of the programs never started, with no warning or error logged anywhere, making the failure very difficult to diagnose.

For example:

[group:a]
programs=b,c

[program:b]
process_name=%(process_num)02d
numprocs=10

[program:c]
process_name=%(process_num)02d
numprocs=10

expected 20 total processes but only started 10, because programs b and c both expand process_name to "00".. "09" and c's entries silently replaced b's.

This commit adds duplicate-name detection to
process_groups_from_parser() while building a heterogeneous group's process list: as each program's processes are added, we track which program produced each expanded process_name, and raise a ValueError identifying the colliding process name and both programs as soon as a collision is found. This matches the existing convention in this file of raising hard config-parse errors for other invalid process_name/ numprocs combinations (e.g. missing %(process_num)s) rather than allowing supervisord to start in a broken, partially-functioning state.

Also updates documentation and the changelog:

  • docs/configuration.rst: adds a note under the process_name option explaining the group-wide uniqueness requirement and the new startup error, and a matching note under [group:x]'s programs option, cross-referenced via a new groupx_section label.
  • CHANGES.rst: adds an entry under 4.4.0.dev0 describing the bug and the fix.

Adds three tests to supervisor/tests/test_options.py:

  • test_duplicate_process_name_in_heterogeneous_group: reproduces the numprocs/process_name collision above and asserts the ValueError mentions the colliding name and both program names.
  • test_duplicate_process_name_in_heterogeneous_group_default_process_name: covers a collision via identical fixed process_name strings (no numprocs involved).
  • test_no_duplicate_process_name_error_when_names_differ: regression guard confirming process_name expressions that differ per program (e.g. via %(program_name)s) still produce the full expected process count with no error.

Previously, when a [group:x] section combined two or more programs
whose process_name expressions expanded to the same value (most
commonly when numprocs > 1 is used and process_name does not include
%(program_name)s), ProcessGroupBase.__init__ silently dropped the
collision: processes for a group are stored in a dict keyed by their
expanded process name, so the later program's process configs simply
overwrote the earlier program's entries in that dict. The result was
that one of the programs never started, with no warning or error
logged anywhere, making the failure very difficult to diagnose.

For example:

    [group:a]
    programs=b,c

    [program:b]
    process_name=%(process_num)02d
    numprocs=10

    [program:c]
    process_name=%(process_num)02d
    numprocs=10

expected 20 total processes but only started 10, because programs b
and c both expand process_name to "00".. "09" and c's entries silently
replaced b's.

This commit adds duplicate-name detection to
process_groups_from_parser() while building a heterogeneous group's
process list: as each program's processes are added, we track which
program produced each expanded process_name, and raise a ValueError
identifying the colliding process name and both programs as soon as a
collision is found. This matches the existing convention in this file
of raising hard config-parse errors for other invalid process_name/
numprocs combinations (e.g. missing %(process_num)s) rather than
allowing supervisord to start in a broken, partially-functioning
state.

Also updates documentation and the changelog:

- docs/configuration.rst: adds a note under the process_name option
  explaining the group-wide uniqueness requirement and the new
  startup error, and a matching note under [group:x]'s programs
  option, cross-referenced via a new groupx_section label.
- CHANGES.rst: adds an entry under 4.4.0.dev0 describing the bug and
  the fix.

Adds three tests to supervisor/tests/test_options.py:

- test_duplicate_process_name_in_heterogeneous_group: reproduces the
  numprocs/process_name collision above and asserts the ValueError
  mentions the colliding name and both program names.
- test_duplicate_process_name_in_heterogeneous_group_default_process_name:
  covers a collision via identical fixed process_name strings (no
  numprocs involved).
- test_no_duplicate_process_name_error_when_names_differ: regression
  guard confirming process_name expressions that differ per program
  (e.g. via %(program_name)s) still produce the full expected process
  count with no error.
The previous commit referenced a :ref:`groupx_section` label from the
process_name documentation note, but the label itself was never
actually added next to the [group:x] Section Values heading, and the
:ref: target ended up written as 'group:x' instead. Sphinx's -W build
(used in CI) treats this as a fatal 'undefined label' error.

Since [group:x] doesn't have a stable anchor to link to, drop the
:ref: and just reference the section by its literal name in prose,
consistent with how [group:x] is referenced elsewhere in this file.

Verified with 'python -m sphinx -b html -W -E docs /tmp/out', which
now builds cleanly.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Development

Successfully merging this pull request may close these issues.

1 participant