esmero / strawberryfield

A Field of strawberries
GNU Lesser General Public License v3.0
10 stars 5 forks source link

The PID we store for the Hydroponics Service is the one of the Parent process, killing it leaves children #239

Open DiegoPino opened 1 year ago

DiegoPino commented 1 year ago

What?

@giancarlobi @patdunlavey @aksm this code here:

https://github.com/esmero/strawberryfield/blob/911f8cbbfa6429636846ddc75cd5316103a46cc4/src/StrawberryfieldHydroponicsService.php#L170-L213

Seems to be getting the PID of the sh that calls the command itself, but not the child process that is started. When we kill it, we are leaving a Child behind that eventually dies but gives this error

php-fpm "oops, unknown child exited with 0".

I think we should try with proc_open instead of exec? Or, save not only the PID of the Parent Process but also of the child one. Then when we need to kill it, we try first a SIGTERM? If that does not kill both (parent and child) we try with the Child first, then the parent?

This is not a very big deal but I would love to see those errors gone, might end generating a slow down eventually (and a lot of php-fpm processes) on heavy load.

DiegoPino commented 1 year ago

There are some documented bugs here regarding this https://bugs.php.net/bug.php?id=73342

Seems like we could do this (like we do on SBR) Because the parent process detaches eventually we could close it... but this needs testing

$command = 'command to execute...';
$pipes = array();
$process = proc_open(
  $command,
  array(
     0 => array('pipe', 'r'),
     1 => array('pipe', 'w'),
     2 => array('pipe', 'w'),
  ),
  $pipes
);

stream_set_blocking($pipes[1], true);
stream_set_blocking($pipes[2], true);

if (
  is_resource($process)
) {

  $output = stream_get_contents($pipes[1]);
  $error = stream_get_contents($pipes[2]);

  fclose($pipes[1]);
  fclose($pipes[2]);

  proc_close($process);

}
DiegoPino commented 1 year ago

https://github.com/esmero/strawberry_runners/blob/dbac9cf07d910dcef3f26d115ced1d5bd774e377/src/Plugin/StrawberryRunnersPostProcessorPluginBase.php#L197-L232

patdunlavey commented 1 year ago

This may be related to ISSUE-75 (issue, pull-request). We found that on large background ocr queues we were getting multiple processes running the same ghostscript and tesseract commands, and this could possibly be the result of \Drupal\strawberry_runners\Plugin\StrawberryRunnersPostProcessorPluginBase::proc_execute failing to close processes due to having incorrect process ids.

DiegoPino commented 1 year ago

@patdunlavey this is not what I observed today. What I observed is you were running Hydroponics via Drush multiple times and they competing and using all resources. Is this a new issue I did not see?

patdunlavey commented 1 year ago

I didn't notice that this issue was being reported for the strawberryfield module, not strawberry_runners, and that it proposes to solve it using an approach implemented in strawberry_runners. Apologies for the confusion!

DiegoPino commented 1 month ago

Still pending.