Bug 560968: [RJ-Servi] Improve generation of node ids and start of
multiple nodes in parallel
- Add random id for factory
- Use nanoSeconds and counter for node id
- Use NIO to create working directory for better error reporting
- Reduce synchronized code
Change-Id: I649fc1b027987b2cd307179941e639ceebff7d52
diff --git a/servi/org.eclipse.statet.rj.servi/src/org/eclipse/statet/internal/rj/servi/LocalNodeFactory.java b/servi/org.eclipse.statet.rj.servi/src/org/eclipse/statet/internal/rj/servi/LocalNodeFactory.java
index 25c289b..cd81506 100644
--- a/servi/org.eclipse.statet.rj.servi/src/org/eclipse/statet/internal/rj/servi/LocalNodeFactory.java
+++ b/servi/org.eclipse.statet.rj.servi/src/org/eclipse/statet/internal/rj/servi/LocalNodeFactory.java
@@ -23,6 +23,7 @@
import java.io.IOException;
import java.io.InputStream;
import java.io.InputStreamReader;
+import java.nio.file.Files;
import java.nio.file.Path;
import java.nio.file.Paths;
import java.rmi.NotBoundException;
@@ -32,6 +33,7 @@
import java.util.Map;
import java.util.Map.Entry;
import java.util.Set;
+import java.util.concurrent.ThreadLocalRandom;
import java.util.concurrent.TimeUnit;
import org.eclipse.statet.jcommons.collections.CollectionUtils;
@@ -104,6 +106,7 @@
private final String poolId;
+ private final String factoryId;
private @Nullable RServiNodeConfig baseConfig;
private final RJContext context;
private final ImList<BundleSpec> libSpecs;
@@ -120,6 +123,8 @@
private final List<String> sslPropertyArgs;
+ private int nodeCounter;
+
public LocalNodeFactory(final String poolId,
final RJContext context, final List<BundleSpec> libSpecs) {
@@ -130,6 +135,7 @@
throw new NullPointerException("context");
}
this.poolId= poolId;
+ this.factoryId= String.format("%1$s-%2$08X", poolId, ThreadLocalRandom.current().nextInt());
this.context= context;
this.libSpecs= ImCollections.toList(libSpecs);
@@ -414,6 +420,7 @@
final ProcessConfig p;
final RMIRegistry registry;
+ final int counter;
synchronized (this) {
p= this.processConfig;
if (p == null) {
@@ -426,26 +433,28 @@
throw new RjInvalidConfigurationException("Missing registry configuration.");
}
timeout= this.timeoutNanos;
+ counter= ++this.nodeCounter;
}
+
+ try {
+ prepareNode(handler, p.baseWd, counter, registry);
+ }
+ catch (final RjException e) {
+ try { // retry
+ Thread.sleep(100);
+ prepareNode(handler, p.baseWd, counter, registry);
+ Utils.logWarning("Preparing R node required a second attempt.", e);
+ }
+ catch (final InterruptedException | RjException e2) {
+ throw new RjException("Error preparing R node.", e);
+ }
+ }
+
ProcessBuilder pBuilder;
- String id;
List<String> command= null;
try {
- synchronized (this) {
- for (int i= 0; ; i++) {
- id= this.poolId + '-' + System.currentTimeMillis();
- handler.dir= new File(p.baseWd + File.separatorChar + id);
- if (!handler.dir.exists() && handler.dir.mkdirs()) {
- break;
- }
- if (i >= 20) {
- throw new RjException("Failed to create working directory (parent="+p.baseWd+").");
- }
- }
- }
command= new ArrayList<>(p.command.size() + 2);
command.addAll(p.command);
- handler.address= new RMIAddress(registry.getAddress(), id);
command.set(p.nameCommandIdx, handler.address.toString());
if (this.verbose) {
command.add("-verbose");
@@ -460,15 +469,16 @@
pBuilder.redirectErrorStream(true);
}
catch (final Exception e) {
- throw new RjException("Error preparing R node.", e);
+ throw new RjException("Error preparing process for R node.", e);
}
+
Process process= null;
try {
process= pBuilder.start();
for (int i= 1; i < Integer.MAX_VALUE; i++) {
try {
- final Server server= (Server) registry.getRegistry().lookup(id);
+ final Server server= (Server) registry.getRegistry().lookup(handler.nodeId);
final ServerLogin login= server.createLogin(Server.C_RSERVI_NODECONTROL);
final RServiNode node= (RServiNode) server.execute(Server.C_RSERVI_NODECONTROL, null, login);
@@ -504,7 +514,7 @@
throw e;
}
- handler.init(node, process);
+ handler.init2(node, process);
return;
}
catch (final NotBoundException e) {
@@ -600,6 +610,23 @@
}
}
+ protected void prepareNode(final NodeHandler handler,
+ final String baseWd, final int counter, final RMIRegistry registry) throws RjException {
+ final String id= String.format("%1$s-%2$016X-%3$08X",
+ this.factoryId, System.nanoTime(), counter );
+ final Path path= Paths.get(baseWd, id);
+ final RMIAddress rmiAddress= new RMIAddress(registry.getAddress(), id);
+ try {
+ handler.init1(id, rmiAddress,
+ Files.createDirectory(path) );
+ }
+ catch (final IOException e) {
+ throw new RjException(
+ String.format("Failed to create working directory '%1$s'.", path),
+ e );
+ }
+ }
+
@Override
public void stopNode(final NodeHandler handler) {
final long t= System.nanoTime();
@@ -642,7 +669,7 @@
&& handler.dir.exists() && handler.dir.isDirectory() ) {
for (int i= 0; i < 20; i++) {
try {
- Thread.sleep(250);
+ Thread.sleep(200);
}
catch (final InterruptedException e) {
}
diff --git a/servi/org.eclipse.statet.rj.servi/src/org/eclipse/statet/internal/rj/servi/NodeHandler.java b/servi/org.eclipse.statet.rj.servi/src/org/eclipse/statet/internal/rj/servi/NodeHandler.java
index b2bd8e0..f6e3e08 100644
--- a/servi/org.eclipse.statet.rj.servi/src/org/eclipse/statet/internal/rj/servi/NodeHandler.java
+++ b/servi/org.eclipse.statet.rj.servi/src/org/eclipse/statet/internal/rj/servi/NodeHandler.java
@@ -14,7 +14,10 @@
package org.eclipse.statet.internal.rj.servi;
+import static org.eclipse.statet.jcommons.lang.ObjectUtils.nonNullAssert;
+
import java.io.File;
+import java.nio.file.Path;
import java.rmi.RemoteException;
import org.eclipse.statet.jcommons.lang.Nullable;
@@ -30,7 +33,9 @@
protected RServiNode node;
+ String nodeId;
RMIAddress address;
+ Path dirPath;
File dir;
Process process;
@@ -72,7 +77,14 @@
}
- void init(final RServiNode node, final Process process) {
+ void init1(final String id, final RMIAddress address, final Path dirPath) {
+ this.nodeId= nonNullAssert(id);
+ this.address= nonNullAssert(address);
+ this.dirPath= nonNullAssert(dirPath);
+ this.dir= dirPath.toFile();
+ }
+
+ void init2(final RServiNode node, final Process process) {
this.node= node;
this.process= process;
}