-
Notifications
You must be signed in to change notification settings - Fork 9.3k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Parallelised execution of static content deploy is broken on 2.3-develop #22563
Comments
Hi @hostep. Thank you for your report.
Please make sure that the issue is reproducible on the vanilla Magento instance following Steps to reproduce. To deploy vanilla Magento instance on our environment, please, add a comment to the issue:
For more details, please, review the Magento Contributor Assistant documentation. @hostep do you confirm that you was able to reproduce the issue on vanilla Magento instance following steps to reproduce?
|
Reverting that
But it gives a slightly different error message:
Notice the difference in |
Git bisecting reveals the following commit to be the cause: 147d11d And indeed, removing the line But this doesn't feel right, I think there is still some deeper problem here, making the PHP execution more stricter is just revealing the problem more clearly, I believe there is still an actual issue here: passing Will do some more digging. |
I can get it more or less working again with the following code changes: diff --git a/app/code/Magento/Deploy/Process/Queue.php b/app/code/Magento/Deploy/Process/Queue.php
index fd7aad44e0a..383b67b5d3c 100644
--- a/app/code/Magento/Deploy/Process/Queue.php
+++ b/app/code/Magento/Deploy/Process/Queue.php
@@ -340,7 +340,7 @@ class Queue
if ($package->getState() === null) {
// phpcs:ignore Magento2.Functions.DiscouragedFunction
$pid = pcntl_waitpid($this->getPid($package), $status, WNOHANG);
- if ($pid === $this->getPid($package)) {
+ if ($this->getPid($package) !== 0 && $pid === $this->getPid($package)) {
$package->setState(Package::STATE_COMPLETED);
unset($this->inProgress[$package->getPath()]);
@@ -354,14 +354,14 @@ class Queue
}
/**
- * Returns process ID or null if not found.
+ * Returns process ID or 0 if not found.
*
* @param Package $package
- * @return int|null
+ * @return int
*/
private function getPid(Package $package)
{
- return isset($this->processIds[$package->getPath()]) ?? null;
+ return $this->processIds[$package->getPath()] ?? 0;
}
/** But the command still seems to run endlessly on my local machine. Never quits and doesn't seem to execute anything. According to a discussion on Slack yesterday, there was at least one person who got it actually executing fine using Docker, but I don't have that setup here right now. So maybe somebody who is able to run this parallelised SCD can test on 2.3.1 and then also on 2.3-develop after applying the above changes and see if that restores the functionality again? Thanks! |
✅ Confirmed by @maheshWebkul721 |
Hi @maheshWebkul721. Thank you for working on this issue.
|
Hi @davidalger. Thank you for working on this issue.
|
@hostep I wish I'd seen this issue before I spent hours figuring out why I was getting the infamous I have a patch which I've been working on locally fixing the random failures resulting in #21852 on 2.2.x, and which adds better error handling around both points where In my Concourse CI pipeline I'm testing, about 50% of builds were failing with parallel jobs with the RuntimeException noted in #21852, which is what set me down the path of fixing this. I'm working on building a fast CI/CD pipeline using build artifacts for a project with over a dozen themes (takes over 10 minutes for SCD without multiple threads being used). Update coming soon. |
Reverts inadvertent change in 7421dfb causing the improper type to be returned as noted on magento#22563
….3-develop and issue where SCD hangs ending in RuntimeException on 2.2-develop Resolves magento#22563 and magento#21852
@davidalger: nice! So based on #21852 it looks like this problem has been happening since at least 2.2.7? So then it probably makes sense that it's not working properly over here on my local environment using Magento 2.3.1. The reason why I changed it from Looking forwards to a fix, I've never really played with the |
I thought that as well, until I added proper error checking around that. If you pass
Not sure when on 2.2.x it was introduced, but at least since 2.2.6 since that's the version I found the issue throwing the |
… in Parallel SCD Execution Relates to magento#22607 magento#21852 magento#22563
Hi @hostep. Thank you for your report.
The fix will be available with the upcoming 2.3.3 release. |
Hi @hostep. Thank you for your report.
The fix will be available with the upcoming 2.2.10 release. |
Preconditions (*)
2.3-develop
branch (works fine on version 2.3.1 btw)pcntl
extension enabledSteps to reproduce (*)
2.3-develop
branch from Magento 2 repo (I've used commit 93a8162)composer install
bin/magento setup:install ...
bin/magento setup:static-content:deploy -f --jobs=2
Expected result (*)
Actual result (*)
Discussion
Issue was probably introduced in 7421dfb#diff-415a71577d8921cc45d695471db3a36fL340 (not verified yet)
The text was updated successfully, but these errors were encountered: