FWD » Core Development » User Interface

These two timeout parameters of webClient

          <node class="integer" name="maxIdleTime">
            <node-attribute name="value" value="300000"/>
          </node>
          <node class="integer" name="maxHttpIdleTimeout">
            <node-attribute name="value" value="300000"/>
          </node>

Also "maxIdleTimeout" is erroneously (or intentionally) used by WebServer to set its http/https session idle timeout.

Sergey Ivanovskiy wrote:

Also "maxIdleTimeout" is erroneously (or intentionally) used by WebServer to set its http/https session idle timeout.

Does it make sense to set getHttpConfiguration().setIdleTimeout(0) for WebServer? It means infinite idle timeout for REST, SOAP and WebHandler?

maxHttpIdleTimeout is set to 0 (infinite timeout) if this setting is not defined in the directory under webClient container. But maxIdleTimeout is set to -1 (connection timeout) if this setting is not defined in the directory and it has consequences when debugging the large customer application.

Now I think that all these parameters should be present. It seems that it would be enough to fix the correct default value for maxIdleTimeout that should be 0 instead of -1. Are there another ideas?

There are the web server idle timeout and the embedded web server idle timeout. The first one is set for WebServer and the second one is set for EmbeddedWebServerImpl by WebGuiDriver.init. How can we simplify these settings? I found that if the web server idle timeout is set to connection timeout, then it can affect the embedded web server idle timeout for the large customer application during the web client debugging.

Could this task be reviewed? I rebased 7824a.

Is there any documentation about the connection settings (the full set, not just the ones changed here)?

Hynek: Please review.

Hynek, please review rev 14867 (7824a) that removed MAX_HTTP_IDLE_TIMEOUT and added CONNECTION_TIMEOUT and INFINITE_TIMEOUT. Now I am collecting these settings in one document to post here and to add to the documentation after the review.

These web client settings (under webClient node)

"webSocketTimeout" = WebClientDebugConfig.MAX_IDLE_TIME
"watchdogTimeout" = WebClientDebugConfig.WATCH_DOG_TIMEOUT
"maxIdleTime" = WebClientDebugConfig.MAX_IDLE_TIME
"maxHttpIdleTimeout" = WebClientDebugConfig.INFINITE_TIMEOUT
"delayBetweenTriesToConnect" = WebClientDebugConfig.DELAY_BETWEEN_CONNECT_TRIES
"pingPongInterval" = WebClientDebugConfig.PING_PONG_INTERVAL
"maxLostPings" = WebClientDebugConfig.MAX_LOST_PINGS
"delayBetweenPingTries" = WebClientDebugConfig.DELAY_BETWEEN_PING_TRIES
"enableDebugLogging" = WebClientDebugConfig.ENABLE_DEBUG_LOGGING

          <node class="string" name="jvmArgs">
            <node-attribute name="value" value="-Xmx1024m -Djava.awt.headless=true -Djava.compiler=NONE -Xdebug -Xrunjdwp:transport=dt_socket,server=y,suspend=n,address=9988"/>
          </node>

These settings are default

"webServerMaxIdleTimeout"  = HttpConfigurationConstants.CONNECTION_TIMEOUT

"embWebServerMaxIdleTimeout" = HttpConfigurationConstants.INFINITE_TIMEOUT

"webServerMaxIdleTimeout"  = HttpConfigurationConstants.INFINITE_TIMEOUT

Code review 7824a revisions 14862..14867.

I may be misinterpreting the changes, but I don't see how the debug mode is enabled when the client is directly given the -Xrunjdwp jvmArgs option in the directory. applyDebugProfile seems to be called only when the debug port is assigned implicitly by createResourceQueue from the range minAgentPort,maxAgentPort.

Hynek Cihlar wrote:

Code review 7824a revisions 14862..14867.

I may be misinterpreting the changes, but I don't see how the debug mode is enabled when the client is directly given the -Xrunjdwp jvmArgs option in the directory. applyDebugProfile seems to be called only when the debug port is assigned implicitly by createResourceQueue from the range minAgentPort,maxAgentPort.

Also applyDebugProfile should use the added string constants.

Hynek Cihlar wrote:

Code review 7824a revisions 14862..14867.

I may be misinterpreting the changes, but I don't see how the debug mode is enabled when the client is directly given the -Xrunjdwp jvmArgs option in the directory. applyDebugProfile seems to be called only when the debug port is assigned implicitly by createResourceQueue from the range minAgentPort,maxAgentPort

The agent port is assigned by WebClientManager that reads jvmArgs option in the directory. But the agent range must be configured too.

Sergey Ivanovskiy wrote:

The agent port is assigned by WebClientManager that reads jvmArgs option in the directory. But the agent range must be configured too.

So when the range is not configured, will the debug mode be enabled?

At this moment the debug mode is not enabled if the agent range is not configured. For the known application there are at least two clients spawned and if the jvmArgs setting were used directly, then it would block the web client.

The agent range is common for the debugger and jmx agents so jvmArgs just describes jmx and/or debug agents should be started with the web client.

We can change the code of WebClientsManager for the case when agent range is not configured but the jvmArgs is given so it builds the port range from the provided single port. I would do this change in order to make settings consistent if there are no objections.

Sergey Ivanovskiy wrote:

These web client settings (under webClient node)
[...]
are set automatically for the debug mode. The debug mode is detecting automatically if clientConfig/jvmArgs has debug settings as in this example
[...]

These settings are default
[...]
but for the debug mode it needs to change settings for "webServerMaxIdleTimeout" manually
[...]
because it could affect the debug mode and these setting is set at the server start time.
I do not know how to improve this part of settings.

Please write this up in Web Clients which is a part of the Directory Configuration Reference. I expect this is not just about debugging. It should document all the connection settings for web clients, including the content discussed in #7149 and #7981.

Sergey Ivanovskiy wrote:

At this moment the debug mode is not enabled if the agent range is not configured. For the known application there are at least two clients spawned and if the jvmArgs setting were used directly, then it would block the web client.

There may be several ways how to provide the Jave debug options for the spawned clients. I usually provide the debug options in jvmArgs in the directory. I then take responsibility for making sure the network ports don't conflict. So I think it makes sense to enable the debug mode for whatever client that was started with Java debugger for the sake of the increased flexibility.

Sergey Ivanovskiy wrote:

We can change the code of WebClientsManager for the case when agent range is not configured but the jvmArgs is given so it builds the port range from the provided single port. I would do this change in order to make settings consistent if there are no objections.

Yes, this would benefit all the other deployment/configuration cases.

Hynek Cihlar wrote:

Sergey Ivanovskiy wrote:

We can change the code of WebClientsManager for the case when agent range is not configured but the jvmArgs is given so it builds the port range from the provided single port. I would do this change in order to make settings consistent if there are no objections.

Yes, this would benefit all the other deployment/configuration cases.

Please review the committed revision 14868 (7824a).

Sergey Ivanovskiy wrote:

Please review the committed revision 14868 (7824a).

Code review.

According to the javaoc parseJMXPort (and parseDebuggerPort) should return -1 if the port is not found.

In parseJMXPort (and parseDebuggerPort), instead of getting a fixed sub string it will be safer to use matching group of the added regular expression. Just make the decimal a group (\\d+) and retrieve it with the matcher, @String port = matcher.group(1).

Otherwise the changes look good.

Please review rev 14889 (7824a) that fixed refs: #7824-17 and #7824-27.

7824a has been rebased. It seems that it can be merged into the trunc. Are there any objections?

This can merge after 7889a.

Planning to merge within next 15 minutes after rebase.

7824a has been rebased, merged into the trunc as rev 14893 and archived.

Sergey Ivanovskiy wrote:

Added the corresponding patch for the trunc source.

Thanks, Sergey. The patch looks good, I'll apply it so I can test.

Sergey, I applied the patch, and it doesn't work properly. It actually has suspend as null when it tries to build the args. I tried initializing suspendOption to "n" and that works for allowing the spawner to work, but it doesn't honor the value in the directory.xml.

Roger Borrello wrote:

Sergey, I applied the patch, and it doesn't work properly. It actually has suspend as null when it tries to build the args. I tried initializing suspendOption to "n" and that works for allowing the spawner to work, but it doesn't honor the value in the directory.xml.

Is an addition to WebClientsManger.java required, as well? I see the javaDebuggerSinglePortNumber is handled there:

       javaDebuggerSinglePortNumber = ClientBuilderOptions.parseDebuggerPort(jvmArgs);

This pattern should work. I tested

      Pattern p2 = Pattern.compile("suspend=([ny])");
      Matcher m2 = p2.matcher("-Duser.country=GB -Duser.language=en -Xmx512m -XX:MaxPermSize=64m -Djava.awt.headless=true -Xdebug -Xrunjdwp:transport=dt_socket,server=y,suspend=y,address=9988,ttt");
      if (m2.find())
      {
         if (m2.group() != null)
         {
            System.out.println("0->" + m2.group());
         }
         if (m2.group(1) != null)
         {
            System.out.println("1->" + m2.group(1));
         }
      }

0->suspend=y
1->y

Yes, the pattern works, but I had to add this to allow the server to start:

+
+   /** Storage for debugger suspend option */
+   private String suspendOption = "n";
+

    public void updatePortForJavaDebuggerAgent(int debuggerPort)
    {
-      if (debuggerPort > 0)
+      if ((debuggerPort > 0) && (suspendOption != null))

Still the "suspend=y" was not honored because the initialize method is never called prior to the spawn. It looks like it should be called somewhere before spawning, perhaps ClientSpawner.updateWebClientOptions? It was never changed from suspendOption = "n" .

Sergey is busy with other tasks, so you will have to implement the fixes and Sergey can review your changes.

Greg Shah wrote:

Sergey is busy with other tasks, so you will have to implement the fixes and Sergey can review your changes.

Updates are included in r14980, and ready for review.

Roger, your code changes helped me to find out what was missed. I think that WebClientCnfig should not have this suspend option as its field because suspend option is shared between all instances. It is enough to have

   protected ClientBuilderOptions(ClientBuilderOptions copy, String[] accountIds)
   {
      this.accountIds = accountIds;

      this.secure                = copy.secure;
      this.command               = copy.command;
      this.classPath             = copy.classPath;
      this.libPath               = copy.libPath;
      this.spawner               = copy.spawner;
      this.spawnerDebugEnabled   = copy.spawnerDebugEnabled;
      this.workingDirectory      = copy.workingDirectory;
      this.configFile            = copy.configFile;
      this.jvmArguments          = new LinkedList<String>(copy.jvmArguments);
      this.bootstrapConfig       = new LinkedList<String>(copy.bootstrapConfig);
      this.debuggerSuspendOption = copy.debuggerSuspendOption;
   }

Sergey Ivanovskiy wrote:

Roger, your code changes helped me to find out what was missed. I think that WebClientCnfig should not have this suspend option as its field because suspend option is shared between all instances. It is enough to have
[...]
so this.debuggerSuspendOption is filled from WebClientBuilderOptions.cbo static field.

Is that the same for debugAgentPort? I wanted to just follow how that was implements, since they work together for jvmArgs.

No, the debug port and the suspend option should be implemented differently. If the port range is not given, then the debug port is defined by the directory, otherwise it is defined by the port range

          <node class="integer" name="minAgentPort">
            <node-attribute name="value" value="9988"/>
          </node>
          <node class="integer" name="maxAgentPort">
            <node-attribute name="value" value="9999"/>
          </node>

WebClientBuilderOptions.cbo is initialized when WebClientBuilderOptions.initialize is called at the server startup time. In my code this part was only missed

   protected ClientBuilderOptions(ClientBuilderOptions copy, String[] accountIds)
   {
      ..............................................................
      this.debuggerSuspendOption = copy.debuggerSuspendOption; // this line is only needed to add after renaming the suspend option
   }

Thank you for clarifying. This is ready for review as revision 14989. Branch has been rebased to trunk revision 14985.

I found minor issue that you missed javadoc for this field

=== modified file 'src/com/goldencode/p2j/main/ClientBuilderOptions.java'
........................................................................
-   /** Storage for debugger suspend option */
-   private String debugSuspendOption;
-
+   private String suspendOption;

Oops... fixed and committed in revision 14990. I believe this is ready for merge to trunk.

Branch 7824b was rebased to trunk revision 15011 and is now at revision 15017.

I believe this is ready for merge.

Yes, I agree with Roger.

Is there any risk associated with these changes?

Greg Shah wrote:

Is there any risk associated with these changes?

No. They would only be used in debug scenarios.

Rebased to trunk revision 15012 and is at revision 15018.

You've tested non-debug scenarios to confirm that what "should" happen actually is the case?

Greg Shah wrote:

You've tested non-debug scenarios to confirm that what "should" happen actually is the case?

Yes... I've been using this update in all my testing (debug and non-debug) for a couple of weeks now.

Branch 7824b has been merged to trunk, and archived.