Skip to content
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

[SYSTEMDS-3185] Worker-specific Multithreading of Operations #1535

Conversation

ywcb00
Copy link
Contributor

@ywcb00 ywcb00 commented Feb 5, 2022

Hi,
This PR adds the MultiThreadedOperator class as a superclass for all operators supporting multithreading. This allows us to replace the number of threads of the operator with a worker-specific amount after parsing the received instruction from the EXEC_INST request.
With this PR, we are setting the number of threads to the amount of processors available to the JVM of the federated worker whenever the operator allows for multithreading (i.e. a subclass of MultiThreadedOperator).

Thanks for review :)

@mboehm7
Copy link
Contributor

mboehm7 commented Feb 6, 2022

LGTM - thanks for the patch @ywcb00. This is a nice, generic way of handling degree of parallelism at the federated workers. Before merging it in, I'd like to understand why the Python tests are now failing in this and other PRs but not on main.

@Baunsgaard
Copy link
Contributor

@mboehm7

Python test failed because when reading in a file for some reason we to import Apache commons-compress and that is not available in our bin release.

The only place where we have automated the testing of our bin release is in Python. We should add a test of our normal system that use our bin release.

See 336ac8b for fix.

@mboehm7
Copy link
Contributor

mboehm7 commented Feb 7, 2022

ok thanks for the clarification.

…rs with a stored number of threads

chore(**Operator.java): make the operators with a number of threads to subclasses of the MultiThreadedOperator
	remove the separate members for the number of threads

feat(FederatedWorkerHandler.java): replace the number of threads of the multi threaded operator with the number of processors on the federated site
@ywcb00 ywcb00 force-pushed the feat/fed/multitenancy/optimize/instructionparallelization branch from f80eff1 to 31e144d Compare February 14, 2022 11:24
@mboehm7
Copy link
Contributor

mboehm7 commented Mar 3, 2022

LGTM again, and sorry for the delay.

@asfgit asfgit closed this in de7d9c3 Mar 3, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
3 participants