Skip to content

Fix the configuration calculation #132

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

Merged
merged 6 commits into from
Oct 16, 2022
Merged

Conversation

theofidry
Copy link
Collaborator

  • We previously we doing some calculation based on $numberOfProcesses and $numberOfProcessesDefined. In reality those calculation were not related directly to those values, but about the fact that child processes we spawned. Indeed when they are not, i.e. we do the processing in the main process, then the segment size and the number of segment becomes irrelevant. I think the logging should be updated to reflect this and better document when child processes are spawned, but this is out of scope of this PR. To address the direct issue though, we now pass directly bool $shouldSpawnChildProcesses
  • The calculation of the segment size and number of segments was incorrect, most likely due to some confusion as to when it is run in the main process or not.
  • The total number of batches was incorrect:
    • when no child processes are spawned, it is not the segment size that matters but the total number of items
    • when child processes are spawned, it may be that the last process does not need to run its maximum number of batches if there is not enough items

@theofidry theofidry enabled auto-merge (squash) October 16, 2022 19:45
@theofidry theofidry merged commit 948d258 into webmozarts:main Oct 16, 2022
@theofidry theofidry deleted the bugfix/config branch October 16, 2022 21:01
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant