sebwrede commented on a change in pull request #1047:
URL: https://github.com/apache/systemds/pull/1047#discussion_r492015888
##########
File path:
src/test/java/org/apache/sysds/test/functions/privacy/FederatedL2SVMTest.java
##########
@@ -314,7 +314,7 @@ private void federatedL2SVM(Types.ExecMode execMode,
Map<String, PrivacyConstrai
if(rtplatform == Types.ExecMode.SPARK) {
DMLScript.USE_LOCAL_SPARK_CONFIG = true;
}
- Process t1 = null, t2 = null;
+ Process[] ts = new Process[0];
Review comment:
This has been changed to use threads since the use of processes failed
in GitHub Actions.
##########
File path: src/test/java/org/apache/sysds/test/AutomatedTestBase.java
##########
@@ -1283,44 +1288,89 @@ public static int getRandomAvailablePort() {
return availableSocket.getLocalPort();
}
catch(IOException e) {
- // If no port was found just use 9999
+ // If no port was found just use 9990
return 9990;
}
}
+ private static void waitForFederatedWorker(int port) {
+ long start = System.currentTimeMillis();
+ while(System.currentTimeMillis() - start <=
FED_WORKER_CONNECTION_WAIT) {
+ FederatedResponse response = null;
+ try {
+ response = executeFederatedOperation(new
InetSocketAddress("localhost", port),
+ new
FederatedRequest(FederatedRequest.RequestType.CLEAR)).get();
+ }
+ catch(Exception e) {
+ // failed, try again if fed worker wait has not
passed yet
+ }
+ if(response != null) {
+ if(!(response.isSuccessful()))
+ LOG.error("Ping failed: " +
response.getErrorMessage());
+ else
+ break;
+ }
+ }
+ }
Review comment:
If the worker does not respond and the time limit has been reached then
the execution would just continue as usual from here. This means that it will
fail later when it tries to send a request to the worker. This will make it
harder to debug compared to handling it when trying to create the worker.
##########
File path:
src/test/java/org/apache/sysds/test/functions/privacy/FederatedWorkerHandlerTest.java
##########
@@ -199,7 +199,7 @@ public void federatedSum(Types.ExecMode execMode,
PrivacyLevel privacyLevel, Cla
assert(checkedPrivacyConstraintsContains(privacyLevel));
- TestUtils.shutdownThread(t);
+ TestUtils.shutdownProcess(t);
Review comment:
This has also been changed to threads.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
[email protected]
