Closed ops4j-issues closed 5 years ago
Benson Margulies commented
I made a PR for setting this parameter: https://github.com/ops4j/org.ops4j.pax.web/pull/57. I did not add any system properties.
Benson Margulies commented
In case someone else runs into this, here's my workaround:
<Configure id="Server" class="org.eclipse.jetty.server.Server">
<New id="httpConfig" class="org.eclipse.jetty.server.HttpConfiguration">
<Set name="secureScheme">https</Set>
<Set name="securePort"><Property name="jetty.tls.port" default="8443" /></Set>
<Set name="outputBufferSize">32768</Set>
<Set name="requestHeaderSize">8192</Set>
<Set name="responseHeaderSize">8192</Set>
</New>
<Call name="addConnector">
<Arg>
<New id="httpConnector" class="org.eclipse.jetty.server.ServerConnector">
<Arg name="server"><Ref refid="Server" /></Arg>
<Arg name="acceptors" type="int"><Property name="jetty.http.acceptors" deprecated="http.acceptors" default="-1"/></Arg>
<Arg name="selectors" type="int"><Property name="jetty.http.selectors" deprecated="http.selectors" default="-1"/></Arg>
<Arg name="factories">
<Array type="org.eclipse.jetty.server.ConnectionFactory">
<Item>
<New class="org.eclipse.jetty.server.HttpConnectionFactory">
<Arg name="config"><Ref refid="httpConfig" /></Arg>
</New>
</Item>
</Array>
</Arg>
<Set name="name">default</Set>
<Set name="host">0.0.0.0</Set>
<Set name="idleTimeout">60000</Set>
<!-- This has to match what's in the pax-web config. Ugh -->
<Set name="port">8181</Set>
</New>
</Arg>
</Call>
</Configure>
Christoph Läubrich commented
I'm not sure if i get you right but pax-web already uses the standard osgi properties so you can use:
<Set name="port" type="java.lang.Integer"><SystemProperty name="org.osgi.service.http.port.secure"/></Set>
or
<Set name="port" type="java.lang.Integer"><SystemProperty name="org.osgi.service.http.port"/></Set>
Benson Margulies commented
Christoph Läubrich For my education, can you please explain a bit more?
I know already about -D for these properties. My situation is that I'm karaf, I'm not setting any properties with -D, but I've got an pax-web ".cfg" file. I don't see, in the code, how settings like
org.osgi.service.http.port=8183
are mapped to system properties that can be referenced from Jetty xml files.
Christoph Läubrich commented
The cfg file are the configuration properties of pax-web itself (see https://ops4j1.jira.com/wiki/display/paxweb/Basic+Configuration), system-properties are defined in the custom.properties file (as an alternative to the -D options), just keep in mind that you have to restart your container then.
Even though you can put them into the cfg file, they then would not be avaiable for the jetty config, so at least for the standard ones (org.osgi.service.http.port / secure) I would suggest to define them as global system-properties.
Benson Margulies commented
So, that would seem to support the logic of my patch – I'd like to avoid dealing in custom.properties and -D, and I'd like to set the idleTimeout, and I'd like to avoid echoing the port setting in the xml file. Add those up, and it seems a reason to add this value to the basic properties.
Achim Nierbeck commented
Actually it's either system properties or configuration admin service properties. The Documentation does list all properties available to configure Pax-Web WebContainers. And even though I'm not a fan of "advertising" the book I worked on, but in this case, you'll also find a complete description of the configuration properties in the Apache Karaf Cookbook.
Benson Margulies commented
Achim Nierbeck I've traced all the code in pax-web, but not in karaf. Jetty reads system properties, pax-web does not ever call System.setProperty. Does karaf call System.setProperty for all all the config admin service properties? If that's true, I better start giving mine longer names.
I'm happy to buy the book, but, anyway, is the patch objectionable? It's a pretty important property if you are in EC2 behind an ELB, why not expose it easily?
Christoph Läubrich commented
I think you missed the point: Pax-Web (or Karaf) do not expose anything TO the System-Properties but reads FROM System-Properties. So you define them ONCE and they are fetched by Pax-Web and can be used by Jetty.xml ...
Writing System-Properties at runtime is bad style IMO....
Achim Nierbeck commented
I didn't have time to go thoroughly through the patch, and right now I'm also looking at it in a general perspective. Is it something that might also be needed for undertow and tomcat. If so I'd rather introduce a new property to PAX Web.
And I still don't get why you are so "obsessed" by the system properties. Jetty and all the other containers are in full control by PAX Web. That's the reason a standard jetty config doesn't work. By the time the jetty.xml is read the container is already started and therefore you only can adapt it.
But as I don't have much time the coming days it'll have to wait a bit till I either accept the patch or come up with something different maybe more appropriate for PAX Web.
Benson Margulies commented
Achim, you are completely misunderstanding me.
The patch defines a new property. The whole discussion of system properties was just part of explaining why I thought it was a good idea. When you get around to it, you'll see.
Achim Nierbeck commented
Tomcat supports a connection Timeout: connectionTimeout
Undertow also does have a idle timeout for connections on the listener: IDLE_TIMEOUT
The supplied patch is incomplete in the following terms:
Benson Margulies commented
Achim Nierbeck commented
if 3 can't be achieved in a simple way, we need to screw it. But I usually prefer to have itests to prevent future regressions
regarding 2) it's here: https://github.com/ops4j/org.ops4j.pax.web/blob/master/pax-web-runtime/src/main/resources/OSGI-INF/metatype/metatype.xml
and thanks for taking care :smile:
Benson Margulies created PAXWEB-980
org.ops4j.pax.web.service.jetty.internal.JettyFactory#createConnector does not accept an idleTimeout value.
So, to set the connector idle timeout (which is different from the thread idle timeout controlled by a pax-web config param), you have to explicitly define your connector in jetty.xml.
And to do that, you need to explicitly set the port in the connector to match the port from the pax web configuration, or else the pax-web code won't recognize your connector as the master.
Should there just be another pax-web param that ends up in this idleTimeout slot? Or should pax-web set some system properties that could be used in the jetty.xml to pick up the pax-web properties?
Affects: 4.2.4 Fixed in: 8.0.0, 7.3.5, 7.2.12 Votes: 1, Watches: 4