Process ID should not be stored in configuration

Created on 30 August 2023, over 1 year ago

Problem/Motivation

  • The process ID (PID) is an environment dependent value while Drupal configuration is environment agnostic. Actually, this is the philosophy of Drupal content management: the way to deploy the same configuration on multiple environments.
  • Many sites are freezing the configuration in the production environment for security considerations. They are using modules such as https://www.drupal.org/project/config_readonly ā†’ to achieve this. In such cases the module will not work because it would be imposible to save the PID.

Proposed resolution

Store the PID as state variable as this is the way to store specific environment variables. See the system.cron_last for a similar example.

Remaining tasks

None.

User interface changes

None.

API changes

None.

Data model changes

PID will be stored in a state variable, rather than in configuration.

šŸ› Bug report
Status

Active

Version

2.0

Component

Code

Created by

šŸ‡·šŸ‡“Romania claudiu.cristea Arad šŸ‡·šŸ‡“

Live updates comments and jobs are added and updated live.
Sign in to follow issues

Comments & Activities

  • Issue created by @claudiu.cristea
  • @claudiucristea opened merge request.
  • Status changed to Needs review over 1 year ago
  • šŸ‡·šŸ‡“Romania claudiu.cristea Arad šŸ‡·šŸ‡“

    A little bit confused about used branches (2.x vs. 2.0.x). Finally I've based on 2.0.x.

  • Assigned to kylehuynh
  • šŸ‡§šŸ‡ŖBelgium kenowax Mons

    I confirm this is an issue.
    In my environments, I use multiple docker containers with a shared database.
    The process IDs are different on each container but the config stored in the database is shared.
    Sometimes, PIDs are overlapping - so when I stop the runner, it would kill an unrelated process in another container.

    Therefore, the PIDs should definitely not be stored in the Database - using the State API would not solve this issue.
    Also, we should probably check that the process we kill is indeed a runner and not another task.

    We should dynamically retrieve the process status and ID from the ps command.
    Similarly to statusByPid but generic.

    Maybe do a check on the running command itself ;
    [17] => 268 www-data 0:00 php /var/www/html/web/modules/contrib/advancedqueue_runner/src/Scripts/jobs.php

  • šŸ‡·šŸ‡“Romania claudiu.cristea Arad šŸ‡·šŸ‡“

    We should dynamically retrieve the process status and ID from the ps command

    If it's not stored, the functionality to restart after a crash will not work. I've created https://www.drupal.org/project/recurring_task ā†’ to make have a generic way to run tasks (not only queue) and there I've moved to state vars. I didn't think about database shared between many sites but we can work there to make that working (maybe such as prefixing with some site hash, or other way?)

  • šŸ‡§šŸ‡ŖBelgium kenowax Mons

    Yes, we need to make sure the processes we generate are the only ones we stop - storing in the database can be a way to check but should not be the way to identify.

    I'll look into your contrib. module soon. But for now, my only solution is to NEVER use the stop button as it may kill unwanted processes.
    Thanks for your work on this. :)

  • šŸ‡·šŸ‡“Romania claudiu.cristea Arad šŸ‡·šŸ‡“

    If that module is doing what you need to do Iā€™m willing to help to get the site-aware support. Please open a ticket there

  • šŸ‡ØšŸ‡¦Canada kylehuynh

    I'll look into this. we use the PID to determine if the runner is active or not. We don't have the use case of multiple containers shared the database, so that's not an issue from my end. I'll looking into alternative way.

Production build 0.71.5 2024