Opened 3 months ago

Closed 6 weeks ago

#2377 closed defect (fixed)

Start i2ptunnel from router state machine, or reduce delay

Reported by: zzz Owned by: zab
Priority: minor Milestone: 0.9.39
Component: router/general Version: 0.9.37
Keywords: Cc: zab
Parent Tickets:

Description

Clients are loaded very early in the startup process (before the router is running at all) in Router.runRouter() which runs StartupJob? which runs LoadClientAppsJob?. We want to load clients.config very early as it fires off the router console.

i2ptunnel is started from clients.config with a delay that's coded in clients.config, or default 2 minutes. 4 years ago we added a delay of 35 seconds in the file, so new installs since then have a shorter delay.

That checkin says:

Router: Add startup/shutdown state machine
ClientAppConfig: Start i2ptunnel sooner
  Since BuildRequestor won't use a zero-hop exploratory as a paired tunnel
  for client builds, it's now safe to start client tunnels
  before the expl. tunnels are ready. This will save up to 90 seconds.

If correct, this allows us to start i2ptunnel even earlier. However, there may be other side effects of starting i2ptunnel immediately. For example, client tunnel builds will eventually timeout if the router has trouble building exploratory tunnels.

We could special-case the i2ptunnel delay setting in LoadClientAppsJob? to ignore the value in the file, so all users would benefit from increased startup. We could additionally special-case it to start it from the router state machine when it gets to RUNNING, although this could mean a prolonged delay if the router can't build tunnels. Note that before i2ptunnel is started, the UI for it is unavailable, which could be problematic.

For starters, will test a value of 5 seconds and see how it goes. A very slow platform such as RPi needs to be a part of the testing.

Subtickets

Change History (9)

comment:1 Changed 3 months ago by zab

How about adding a boolean property to each client in clients.config whether it requires the state machine to be in RUNNING state before it can start? That way there's no need to add callbacks from the state machine and the LoadClientAppsJob can poll periodically, should be a smaller change.

comment:2 Changed 7 weeks ago by zab

Here is a patch that implements the approach proposed in comment 1:

#
# old_revision [302c0fcfe0bd2fe72b5392db98b0d5c40b202026]
#
# patch "apps/routerconsole/java/src/net/i2p/router/web/PluginStarter.java"
#  from [a64fb51597c657090225d02150fd70b1c714a9e8]
#    to [132b15dfa95066804081a7c85bff6bf82239213e]
# 
# patch "router/java/src/net/i2p/router/Router.java"
#  from [ee55a09bd36eeae5ab86b4b5ecb4361b2f74d18d]
#    to [963f3b77a3f2efb05f5b27a1773b0513aab3594a]
# 
# patch "router/java/src/net/i2p/router/startup/ClientAppConfig.java"
#  from [365224f427ae5da211a175403198e63178529568]
#    to [39aa313c23ed6a9dd366d21025dc8c602154b84e]
# 
# patch "router/java/src/net/i2p/router/startup/LoadClientAppsJob.java"
#  from [9e6a7df44740d5e20639e15de80950e8046fe972]
#    to [25076cac1495b50cfa04c9f2ad70852efc971864]
#
============================================================
--- apps/routerconsole/java/src/net/i2p/router/web/PluginStarter.java	a64fb51597c657090225d02150fd70b1c714a9e8
+++ apps/routerconsole/java/src/net/i2p/router/web/PluginStarter.java	132b15dfa95066804081a7c85bff6bf82239213e
@@ -918,7 +918,7 @@ public class PluginStarter implements Ru
         public TrackedDelayedClient(String pluginName,
                                     SimpleTimer2 pool, RouterContext enclosingContext, String className, String clientName,
                                     String args[], ThreadGroup threadGroup, ClassLoader cl) {
-            super(pool, enclosingContext, className, clientName, args, threadGroup, cl);
+            super(pool, enclosingContext, className, clientName, false, args, threadGroup, cl);
             _pluginName = pluginName;
             _pendingPluginClients.get(pluginName).add(this);
         }
============================================================
--- router/java/src/net/i2p/router/Router.java	ee55a09bd36eeae5ab86b4b5ecb4361b2f74d18d
+++ router/java/src/net/i2p/router/Router.java	963f3b77a3f2efb05f5b27a1773b0513aab3594a
@@ -823,6 +823,15 @@ public class Router implements RouterClo
     }
 
     /**
+     * @return true if and only if state == RUNNING
+     */
+    public boolean isRunning() {
+        synchronized(_stateLock) {
+            return _state == State.RUNNING;
+        }
+    }
+
+    /**
      *  Only for Restarter, after soft restart is complete.
      *  Not for external use.
      *  @since 0.8.12
============================================================
--- router/java/src/net/i2p/router/startup/ClientAppConfig.java	365224f427ae5da211a175403198e63178529568
+++ router/java/src/net/i2p/router/startup/ClientAppConfig.java	39aa313c23ed6a9dd366d21025dc8c602154b84e
@@ -80,6 +80,7 @@ public class ClientAppConfig {
     public String args;
     public boolean disabled;
     public final long delay;
+    public final boolean routerRunning;
     /** @since 0.7.12 */
     public final String classpath;
     /** @since 0.7.12 */
@@ -88,16 +89,17 @@ public class ClientAppConfig {
     public final String uninstallargs;
 
     public ClientAppConfig(String cl, String client, String a, long d, boolean dis) {
-        this(cl, client, a, d, dis, null, null, null);
+        this(cl, client, a, d, dis, false, null, null, null);
     }
 
     /** @since 0.7.12 */
-    public ClientAppConfig(String cl, String client, String a, long d, boolean dis, String cp, String sa, String ua) {
+    public ClientAppConfig(String cl, String client, String a, long d, boolean dis, boolean rr, String cp, String sa, String ua) {
         className = cl;
         clientName = client;
         args = a;
         delay = d;
         disabled = dis;
+	routerRunning = rr;
         classpath = cp;
         stopargs = sa;
         uninstallargs = ua;
@@ -178,8 +180,10 @@ public class ClientAppConfig {
             String classpath = clientApps.getProperty(PREFIX + i + ".classpath");
             String stopargs = clientApps.getProperty(PREFIX + i + ".stopargs");
             String uninstallargs = clientApps.getProperty(PREFIX + i + ".uninstallargs");
+	    String routerRunning = clientApps.getProperty(PREFIX + i + ".routerRunning");
             i++;
             boolean dis = disabled != null && "false".equals(disabled);
+	    boolean rr = Boolean.valueOf(routerRunning);
 
             boolean onStartup = false;
             if (onBoot != null)
@@ -192,7 +196,7 @@ public class ClientAppConfig {
             if (delayStr != null && !onStartup)
                 try { delay = 1000*Integer.parseInt(delayStr); } catch (NumberFormatException nfe) {}
 
-            rv.add(new ClientAppConfig(className, clientName, args, delay, dis,
+            rv.add(new ClientAppConfig(className, clientName, args, delay, dis, rr,
                    classpath, stopargs, uninstallargs));
         }
         return rv;
@@ -213,6 +217,7 @@ public class ClientAppConfig {
                     buf.append(PREFIX).append(i).append(".args=").append(app.args).append("\n");
                 buf.append(PREFIX).append(i).append(".delay=").append(app.delay / 1000).append("\n");
                 buf.append(PREFIX).append(i).append(".startOnLoad=").append(!app.disabled).append("\n");
+		buf.append(PREFIX).append(i).append(".routerRunning=").append(app.routerRunning).append("\n");
             }
             fos.write(buf.toString().getBytes("UTF-8"));
         } catch (IOException ioe) {
============================================================
--- router/java/src/net/i2p/router/startup/LoadClientAppsJob.java	9e6a7df44740d5e20639e15de80950e8046fe972
+++ router/java/src/net/i2p/router/startup/LoadClientAppsJob.java	25076cac1495b50cfa04c9f2ad70852efc971864
@@ -56,14 +56,15 @@ public class LoadClientAppsJob extends J
                 continue;
             }
             String argVal[] = parseArgs(app.args);
-            if (app.delay <= 0) {
+            if (app.delay <= 0 && (!app.routerRunning)) {
                 // run this guy now
                 runClient(app.className, app.clientName, argVal, getContext(), _log);
             } else {
                 // wait before firing it up
                 DelayedRunClient drc = new DelayedRunClient(getContext().simpleTimer2(), getContext(), app.className,
-                                                            app.clientName, argVal);
-                drc.schedule(app.delay);
+                                                            app.clientName, app.routerRunning, argVal);
+		long delay = app.delay > 0 ? app.delay : 1000;
+                drc.schedule(delay);
             }
         }
     }
@@ -79,20 +80,22 @@ public class LoadClientAppsJob extends J
         private final Log _log;
         private final ThreadGroup _threadGroup;
         private final ClassLoader _cl;
+	private final boolean _routerRunning;
 
         /** caller MUST call schedule() */
         public DelayedRunClient(SimpleTimer2 pool, RouterContext enclosingContext, String className,
-                                String clientName, String args[]) {
-            this(pool, enclosingContext, className, clientName, args, null, null);
+                                String clientName, boolean routerRunning, String args[]) {
+            this(pool, enclosingContext, className, clientName, routerRunning, args, null, null);
         }
         
         /** caller MUST call schedule() */
         public DelayedRunClient(SimpleTimer2 pool, RouterContext enclosingContext, String className, String clientName,
-                                String args[], ThreadGroup threadGroup, ClassLoader cl) {
+                                boolean routerRunning, String args[], ThreadGroup threadGroup, ClassLoader cl) {
             super(pool);
             _ctx = enclosingContext;
             _className = className;
             _clientName = clientName;
+	    _routerRunning = routerRunning;
             _args = args;
             _log = enclosingContext.logManager().getLog(LoadClientAppsJob.class);
             _threadGroup = threadGroup;
@@ -100,6 +103,10 @@ public class LoadClientAppsJob extends J
         }
 
         public void timeReached() {
+	    if (_routerRunning && !_ctx.router().isRunning()) {
+                reschedule(1000);
+		return;
+	    }
             runClient(_className, _clientName, _args, _ctx, _log, _threadGroup, _cl);
         }
     }

comment:3 Changed 7 weeks ago by zzz

Interesting. You're on the right track here. Several thoughts, mostly related to how to get this change to everybody, as discussed in the OP:

  • By requiring a new config param, it would only be for new installs. We could fix that by checking for a classname of net.i2p.i2ptunnel.TunnelControllerGroup? and changing the default to true for it
  • Patch wouldn't have much effect because it waits for the specified delay AND for the router to be running. For it to be useful, wouldn't we have to ignore the delay if routerrunning is true?
  • Or maybe don't add a new param, and declare a wait value of -1 to be "wait for the router" ? This might be simpler... Probably should check the clients.config spec on the website to see what we claim the rules are (and we'd need to update that spec for any change... don't need to see a patch for that here though)
  • @since on isRunning(), and maybe make even more clear in the javadocs the difference between isAlive() and isRunning()
  • should isRunning also include the graceful shutdown state? (would need a new EnumSet?)
  • patch is a mix of spaces and tabs, please detab
  • need the change for installer/resources/clients.config in the patch

comment:4 Changed 7 weeks ago by zab

New patch, addressing all issues above except:

should isRunning also include the graceful shutdown state? (would need a new EnumSet??)

I think the answer is no, why would you want to start clients if you're shutting down?

#
# old_revision [870f1b65385a1931d76fc1f1e0886c6ccd9ac90c]
#
# patch "installer/resources/clients.config"
#  from [739da77f3f5b4609877d44a49af9b15216133576]
#    to [ece29e34a887fd4245792642e8a5dfb75b312f4d]
# 
# patch "router/java/src/net/i2p/router/Router.java"
#  from [ee55a09bd36eeae5ab86b4b5ecb4361b2f74d18d]
#    to [e34fb2db1add28eb15fd8e5c7db16f3132232414]
# 
# patch "router/java/src/net/i2p/router/startup/ClientAppConfig.java"
#  from [365224f427ae5da211a175403198e63178529568]
#    to [be20d92c8d80391fe8a4e861c9a9e7f96881df78]
# 
# patch "router/java/src/net/i2p/router/startup/LoadClientAppsJob.java"
#  from [9e6a7df44740d5e20639e15de80950e8046fe972]
#    to [21f8a8eabd4b8f053443ce5435d9173ff44d14d4]
#
============================================================
--- router/java/src/net/i2p/router/Router.java	ee55a09bd36eeae5ab86b4b5ecb4361b2f74d18d
+++ router/java/src/net/i2p/router/Router.java	e34fb2db1add28eb15fd8e5c7db16f3132232414
@@ -823,6 +823,16 @@ public class Router implements RouterClo
     }
 
     /**
+     * @return true if router is RUNNING, i.e NetDB and Expl. tunnels are ready.
+     * @since 0.9.39
+     */
+    public boolean isRunning() {
+        synchronized(_stateLock) {
+            return _state == State.RUNNING;
+        }
+    }
+
+    /**
      *  Only for Restarter, after soft restart is complete.
      *  Not for external use.
      *  @since 0.8.12
============================================================
--- router/java/src/net/i2p/router/startup/LoadClientAppsJob.java	9e6a7df44740d5e20639e15de80950e8046fe972
+++ router/java/src/net/i2p/router/startup/LoadClientAppsJob.java	21f8a8eabd4b8f053443ce5435d9173ff44d14d4
@@ -56,14 +56,18 @@ public class LoadClientAppsJob extends J
                 continue;
             }
             String argVal[] = parseArgs(app.args);
-            if (app.delay <= 0) {
+            if (app.delay == 0) {
                 // run this guy now
                 runClient(app.className, app.clientName, argVal, getContext(), _log);
-            } else {
+            } else if (app.delay > 0) {
                 // wait before firing it up
                 DelayedRunClient drc = new DelayedRunClient(getContext().simpleTimer2(), getContext(), app.className,
                                                             app.clientName, argVal);
                 drc.schedule(app.delay);
+            } else {
+                WaitForRunningClient wfrc = new WaitForRunningClient(getContext().simpleTimer2(), getContext(),
+                                                                app.className, app.clientName, argVal);
+                wfrc.schedule(1000);
             }
         }
     }
@@ -72,7 +76,7 @@ public class LoadClientAppsJob extends J
      *  Public for router console only, not for use by others, subject to change
      */
     public static class DelayedRunClient extends SimpleTimer2.TimedEvent {
-        private final RouterContext _ctx;
+        protected final RouterContext _ctx;
         private final String _className;
         private final String _clientName;
         private final String _args[];
@@ -103,6 +107,21 @@ public class LoadClientAppsJob extends J
             runClient(_className, _clientName, _args, _ctx, _log, _threadGroup, _cl);
         }
     }
+
+    private static class WaitForRunningClient extends DelayedRunClient {
+        WaitForRunningClient(SimpleTimer2 pool, RouterContext enclosingContext, String className, String clientName,
+                             String args[]) {
+            super(pool, enclosingContext, className, clientName, args, null, null);
+        }
+
+        public void timeReached() {
+            if (!_ctx.router().isRunning()) {
+                 reschedule(1000);
+                 return;
+            }
+            super.timeReached();
+        }
+    }
     
     /**
      *  Parse arg string into an array of args.
============================================================
--- installer/resources/clients.config	739da77f3f5b4609877d44a49af9b15216133576
+++ installer/resources/clients.config	ece29e34a887fd4245792642e8a5dfb75b312f4d
@@ -42,7 +42,7 @@ clientApp.2.args=i2ptunnel.config
 clientApp.2.main=net.i2p.i2ptunnel.TunnelControllerGroup
 clientApp.2.name=Application tunnels
 clientApp.2.args=i2ptunnel.config
-clientApp.2.delay=35
+clientApp.2.delay=-1
 clientApp.2.startOnLoad=true
 
 # run our own eepsite with a seperate jetty instance
============================================================
--- router/java/src/net/i2p/router/startup/ClientAppConfig.java	365224f427ae5da211a175403198e63178529568
+++ router/java/src/net/i2p/router/startup/ClientAppConfig.java	be20d92c8d80391fe8a4e861c9a9e7f96881df78
@@ -67,7 +67,7 @@ public class ClientAppConfig {
     /** wait 2 minutes before starting up client apps */
     private final static long DEFAULT_STARTUP_DELAY = 2*60*1000;
     /** speed up i2ptunnel without rewriting clients.config */
-    private final static long I2PTUNNEL_STARTUP_DELAY = 35*1000;
+    private final static long I2PTUNNEL_STARTUP_DELAY = -1000;
     
     private static final String PROP_CLIENT_CONFIG_FILENAME = "router.clientConfigFile";
     private static final String DEFAULT_CLIENT_CONFIG_FILENAME = "clients.config";
Last edited 7 weeks ago by zab (previous) (diff)

comment:5 Changed 7 weeks ago by zab

Note that the following statements from the spec on the website are currently incorrect:

If the delay is less than zero, the client is run immediately, in the same thread, so that exceptions may be propagated to the console. In this case, the client should either throw an exception, return quickly, or spawn its own thread.

If the delay is greater than or equal to zero, it will be run in a new thread, and exceptions will be logged but not propagated to the console.

comment:6 Changed 7 weeks ago by zab

Another note - the patch from comment 4 is not sufficient to make I2PTunnel start earlier because of the following logic in ClientAppConfig.java:

// speed up the start of i2ptunnel for everybody without rewriting clients.config
long delay = onStartup ? 0 :
(className.equals("net.i2p.i2ptunnel.TunnelControllerGroup") ?
I2PTUNNEL_STARTUP_DELAY : DEFAULT_STARTUP_DELAY);
if (delayStr != null && !onStartup)
    try { delay = 1000*Integer.parseInt(delayStr); } catch (NumberFormatException nfe) {}

But then in writeClientAppConfig(...) the new, overridden delay gets persisted. So any existing install would have an explicit clientApp.2.delay=35 in clients.config. I don't know how to get around that problem without making the patch significantly more involved.

comment:7 Changed 7 weeks ago by zzz

I like the new patch. Much simpler without a new config option. It's already too complex with onBoot, onStartup, and delay.

To address comment 6, maybe something like this? (untested)

--- router/java/src/net/i2p/router/startup/ClientAppConfig.java	365224f427ae5da211a175403198e63178529568
+++ router/java/src/net/i2p/router/startup/ClientAppConfig.java	5905691827f6b56ab67470c28d085f2ef7d9e818
@@ -67,7 +67,7 @@ public class ClientAppConfig {
     /** wait 2 minutes before starting up client apps */
     private final static long DEFAULT_STARTUP_DELAY = 2*60*1000;
     /** speed up i2ptunnel without rewriting clients.config */
-    private final static long I2PTUNNEL_STARTUP_DELAY = 35*1000;
+    private final static long I2PTUNNEL_STARTUP_DELAY = -1000;
     
     private static final String PROP_CLIENT_CONFIG_FILENAME = "router.clientConfigFile";
     private static final String DEFAULT_CLIENT_CONFIG_FILENAME = "clients.config";
@@ -185,13 +185,17 @@ public class ClientAppConfig {
             if (onBoot != null)
                 onStartup = "true".equals(onBoot) || "yes".equals(onBoot);
 
-            // speed up the start of i2ptunnel for everybody without rewriting clients.config
-            long delay = onStartup ? 0 :
-                                   (className.equals("net.i2p.i2ptunnel.TunnelControllerGroup") ?
-                                    I2PTUNNEL_STARTUP_DELAY : DEFAULT_STARTUP_DELAY);
-            if (delayStr != null && !onStartup)
-                try { delay = 1000*Integer.parseInt(delayStr); } catch (NumberFormatException nfe) {}
-
+            long delay;
+            if (onStartup) {
+                delay = 0;
+            } else if (className.equals("net.i2p.i2ptunnel.TunnelControllerGroup")) {
+                // speed up the start of i2ptunnel for everybody without rewriting clients.config
+                delay = I2PTUNNEL_STARTUP_DELAY;
+            } else {
+                delay = DEFAULT_STARTUP_DELAY;
+                if (delayStr != null)
+                    try { delay = 1000*Integer.parseInt(delayStr); } catch (NumberFormatException nfe) {}
+            }
             rv.add(new ClientAppConfig(className, clientName, args, delay, dis,
                    classpath, stopargs, uninstallargs));
         }

comment:8 Changed 6 weeks ago by zzz

  • Owner set to zab
  • Status changed from new to assigned

ACK, tested, looks good. I think we arrived at a clean, low-risk solution.

Please check in. Also please update the website clients.config spec at your convenience.

comment:9 Changed 6 weeks ago by zab

  • Resolution set to fixed
  • Status changed from assigned to closed

Checked in 03379069813ab8df18d30d5baefa397137ef02a7
Website updated in 1ce77c01e575fcc29c28e18828279b2f9d1e3811

Note: See TracTickets for help on using tickets.