HTTP/2 Concurrent streams question

2018-11-07 Thread Elias Schulze
Hi!

Not sure if this is the right place to ask, please point me in the right
direction if it's not. My question is regarding the IOException "too many
concurrent streams" in Http2Connection line 440. Probably my understanding
of HTTP/2 and net.http is wrong, but I expected the HttpClient to open new
parallel connections if the max concurrent streams is reached for any one
open connection and there's more request to be sent. Why doesn't it? Am I
meant to handle such cases myself? The server in this case is an amazon
load balancer, so I can't change the server HTTP/2 settings.

Thanks in advance!
Elias


Re: HTTP/2 Concurrent streams question

2018-11-07 Thread Michael McMahon

Hi,

This is the right place to ask! It was implemented that way because RFC 7540
recommends that:

   Clients SHOULD NOT open more than one HTTP/2 connection to a given
   host and port pair, where the host is derived from a URI, a selected
   alternative service [ALT-SVC  
], or a configured proxy.

One workaround would be to create a second HttpClient in that situation.
As a matter of interest, what limit is imposed by the server in this case?

Thanks,
Michael

On 07/11/2018, 10:22, Elias Schulze wrote:

Hi!

Not sure if this is the right place to ask, please point me in the 
right direction if it's not. My question is regarding the IOException 
"too many concurrent streams" in Http2Connection line 440. Probably my 
understanding of HTTP/2 and net.http is wrong, but I expected the 
HttpClient to open new parallel connections if the max concurrent 
streams is reached for any one open connection and there's more 
request to be sent. Why doesn't it? Am I meant to handle such cases 
myself? The server in this case is an amazon load balancer, so I can't 
change the server HTTP/2 settings.


Thanks in advance!
Elias


Re: HTTP/2 Concurrent streams question

2018-11-07 Thread Simone Bordet
Hi,

On Wed, Nov 7, 2018 at 11:44 AM Michael McMahon
 wrote:
>
> Hi,
>
> This is the right place to ask! It was implemented that way because RFC 7540
> recommends that:
>
>Clients SHOULD NOT open more than one HTTP/2 connection to a given
>host and port pair, where the host is derived from a URI, a selected
>alternative service [ALT-SVC], or a configured proxy.
>
> One workaround would be to create a second HttpClient in that situation.

>From the Jetty experience in the field, opening just one connection is
most often impractical.

The first reason is that the typical session flow control window will
be (almost) immediately exhausted and you get a large performance hit
due to the flow control frames roundtrips.
The second reason is that even enlarging the session flow control
window to very large values, you then end up hitting TCP flow control,
with the same effects on performance.

Opening few connections (many many less that you would with HTTP/1.1)
showed to be a good compromise between resource utilization and
performance (and outperforms HTTP/1.1).

-- 
Simone Bordet
---
Finally, no matter how good the architecture and design are,
to deliver bug-free software with optimal performance and reliability,
the implementation technique must be flawless.   Victoria Livschitz


RFR [12] 8210493: Bind to node- or linklocal ipv6 multicast address fails

2018-11-07 Thread Pavel Rappo
Hello,

Please review the following change:

  http://cr.openjdk.java.net/~prappo/8210493/webrev.00

This change fixes binding to interface-local and link-local IPv6 multicast
addresses on Linux.

Thanks,
-Pavel




RFR [12] 8213418: Socket/ServerSocket supportedOptions does not work with custom SocketImpl

2018-11-07 Thread Chris Hegarty

Socket and ServerSocket supportedOptions() caches the supported options
in a static field on the assumption that all implementations support
the same set of options. This assumption is incorrect.

The cache can be on a per-socket basis ( which has been implemented in
the webrev ), or maybe even removed completely and just defer to the
impl. The reason I left the per-socket cache is to avoid creating the
additional unmodifiable wrapper for every access ( Argh! why didn't we
specify that SocketImpl::supportedOptions returns an unmodifiable set,
then there would be no need for this defensive wrapper )

This should to be fixed in advance of JEP 337, which will use a RDMA
SocketImpl that supports RDMA specific socket options.

http://cr.openjdk.java.net/~chegar/8213418/webrev.00/

-Chris.


Re: RFR [12] 8210493: Bind to node- or linklocal ipv6 multicast address fails

2018-11-07 Thread Alan Bateman

On 07/11/2018 14:12, Pavel Rappo wrote:

Hello,

Please review the following change:

   http://cr.openjdk.java.net/~prappo/8210493/webrev.00

This change fixes binding to interface-local and link-local IPv6 multicast
addresses on Linux.
Does the comment at L806 need to be updated? Also the change makes me 
wondering about the 2.4 kernel support, maybe it is time to think about 
dropping some of the older code path.


-Alan


JDK-7014531

2018-11-07 Thread MUELLER-SCHRAMM Gerd
Hi,

I’ve got a question regarding 
JDK-7014531. This bug is 
still open and after checking it against Java 11 still valid. Will it ever be 
fixed?
I’ve took a look at the source code and the bug seems to be in 
DefaultProxySelector.c, Java_sun_net_spi_DefaultProxySelector_getSystemProxies.
This method doesn’t care about wildcards but only compares the host names.

Regards,
Gerd

Gerd Müller-Schramm
Software Developer, GeoMedia Smart Client Kommunal
T: +49 341 92 60 30 47
E: 
gerd.mueller-schr...@hexagon.com

HxGN Safty & Infrastructure GmbH
Wittenberger Straße 15B
04129 Leipzig, Germany
hexagon.com
HxGN SI is part of HEXAGON



Re: RFR [12] 8210493: Bind to node- or linklocal ipv6 multicast address fails

2018-11-07 Thread Chris Hegarty

On 07/11/18 14:12, Pavel Rappo wrote:

Hello,

Please review the following change:

   http://cr.openjdk.java.net/~prappo/8210493/webrev.00


Thanks for doing this Pavel. The changes in the webrev
look good to me.

-Chris.


Re: RFR [12] 8210493: Bind to node- or linklocal ipv6 multicast address fails

2018-11-07 Thread Pavel Rappo
> On 7 Nov 2018, at 15:22, Alan Bateman  wrote:
> 
> Does the comment at L806 need to be updated? Also the change makes me 
> wondering about the 2.4 kernel support, maybe it is time to think about 
> dropping some of the older code path.

Alan, 

Is that better?

  http://cr.openjdk.java.net/~prappo/8210493/webrev.01/

A note to reviewers 
===

It looks like the initial terminology "node-local" for multicast IPv6 addresses
introduced in [1] more than 20 years ago has since changed to "interface-local"
[2]:

   -  Multicast scope changes:
 o  Changed name of scope value 1 from "node-local" to
"interface-local"
 o  Defined scope value 4 as "admin-local"

[1] https://tools.ietf.org/html/rfc1884#page-14
[2] https://tools.ietf.org/html/rfc3513#page-24



Re: RFR [12] 8210493: Bind to node- or linklocal ipv6 multicast address fails

2018-11-07 Thread Alan Bateman




On 07/11/2018 16:23, Pavel Rappo wrote:

On 7 Nov 2018, at 15:22, Alan Bateman  wrote:

Does the comment at L806 need to be updated? Also the change makes me wondering 
about the 2.4 kernel support, maybe it is time to think about dropping some of 
the older code path.

Alan,

Is that better?

   http://cr.openjdk.java.net/~prappo/8210493/webrev.01/


Looks good, thanks for digging into the history and terminology change too.


Re: HTTP/2 Concurrent streams question

2018-11-07 Thread Michael McMahon
That's useful feedback Simone. It sounds like this is a somewhat 
different issue though.
How would you recommend determining when to open a new connection to 
maximise

throughput?

Thanks,
Michael.

On 07/11/2018, 12:00, Simone Bordet wrote:

Hi,

On Wed, Nov 7, 2018 at 11:44 AM Michael McMahon
  wrote:

Hi,

This is the right place to ask! It was implemented that way because RFC 7540
recommends that:

Clients SHOULD NOT open more than one HTTP/2 connection to a given
host and port pair, where the host is derived from a URI, a selected
alternative service [ALT-SVC], or a configured proxy.

One workaround would be to create a second HttpClient in that situation.

 From the Jetty experience in the field, opening just one connection is
most often impractical.

The first reason is that the typical session flow control window will
be (almost) immediately exhausted and you get a large performance hit
due to the flow control frames roundtrips.
The second reason is that even enlarging the session flow control
window to very large values, you then end up hitting TCP flow control,
with the same effects on performance.

Opening few connections (many many less that you would with HTTP/1.1)
showed to be a good compromise between resource utilization and
performance (and outperforms HTTP/1.1).



Re: HTTP/2 Concurrent streams question

2018-11-07 Thread Simone Bordet
Hi,

On Wed, Nov 7, 2018 at 7:09 PM Michael McMahon
 wrote:
>
> That's useful feedback Simone. It sounds like this is a somewhat
> different issue though.
> How would you recommend determining when to open a new connection to
> maximise throughput?

Opening a new connection and maximizing throughput may not be related,
so let's say just opening a new connection.

The client must not exceed the server's max_concurrent_streams.
When it does, it may open a new connection.
This would solve the OP problem.

Then, have the client configurable with the max number of connections
per origin (which can be defaulted at 1 for HTTP/2 - and to a larger
value for HTTP/1.1).
Once you hit that too, start queuing (or rejecting).

Maximizing throughput is a different story, as it depends on a number
of factors that are volatile (e.g. the status of the network in that
precise moment, the bandwidth*delay product, dynamic adjustments of
the HTTP/2 flow control window, etc.).

-- 
Simone Bordet
---
Finally, no matter how good the architecture and design are,
to deliver bug-free software with optimal performance and reliability,
the implementation technique must be flawless.   Victoria Livschitz


Re: A new proposal to add methods to HttpsURLConnection to access SSLSession

2018-11-07 Thread Sean Mullan

On 11/5/18 1:52 PM, Xuelei Fan wrote:

Hi Chris and Sean,

There are a few feedback for the CSR approval.  I updated to use 
Optional for the returned value.  For more details, please 
refer to:

   https://bugs.openjdk.java.net/browse/JDK-8213161
   http://cr.openjdk.java.net/~xuelei/8212261/webrev.03/


I didn't see a test for SecureCacheResponse - is it possible? You could 
also add a test case for IllegalStateExc.


lines 108-113 of HttpsSession.java should be indented 4 more spaces. 
Otherwise looks ok to me.


--Sean




Please let me know if you are OK with this change.

Thanks,
Xuelei

On 11/2/2018 11:42 AM, Xuelei Fan wrote:

Thanks for the review and suggestions, Chris and Sean.

I just finalized the CSR.

Thanks,
Xuelei

On 11/2/2018 5:58 AM, Chris Hegarty wrote:

Thanks for the updates Xuelei, some minor comments inline..

On 1 Nov 2018, at 23:42, Xuelei Fan > wrote:


On 11/1/2018 11:24 AM, Sean Mullan wrote:

On 10/31/18 11:52 AM, Chris Hegarty wrote:

Xuelei,

On 30/10/18 20:55, Xuelei Fan wrote:

Hi,

For the current HttpsURLConnection, there is not much security 
parameters exposed in the public APIs.  An application may need 
richer information for the underlying TLS connections, for 
example the negotiated TLS protocol version.


Please let me know if you have concerns to add a new method 
HttpsURLConnection.getSSLSession() and deprecate the duplicated 
methods, by the end of Nov. 2, 2018.


Here is the proposal:
https://bugs.openjdk.java.net/browse/JDK-8213161
Are there any security issues associated with returning the 
SSLSession, since it is mutable?
It should be fine.  The update APIs of the session (invalidating, 
bind values) does not impact the connection.


Alternatively, as is done in the new HTTP Client, an immutable
SSLSession instance can be returned.

+ *   SHOULD override this method with appropriate 
implementation.

s/appropriate/an appropriate/
I would probably not capitalize "SHOULD" and just say "should". 
"SHOULD" is more common in RFCs. I don't see that much in javadocs.
+ * @implNote The JDK Reference Implementation supports this 
operation.
+ *   As an application may have to use this operation 
for more
+ *   security parameters, it is recommended to support 
this

+ *   operation in all implementations.
I think it should be obvious that the JDK implementation would 
override this method so not sure that first sentence is necessary. 
The other sentence seems like it could be combined with the 
previous sentence, ex:
"Subclasses should override this method with an appropriate 
implementation since an application may need to access additional 
parameters associated with the SSL session."

Updated accordingly, in the CSR and webrev:
https://bugs.openjdk.java.net/browse/JDK-8213161


The CSR looks good. I made a few minor edits to the verbiage
and added myself as reviewer.

The title will need to be updated to reflect the addition of the
new method in SecureCacheResponse. Maybe:

"Add SSLSession accessors to HttpsURLConnection and
SecureCacheResponse"

-Chris.





Re: A new proposal to add methods to HttpsURLConnection to access SSLSession

2018-11-07 Thread Xuelei Fan

On 11/7/2018 1:30 PM, Sean Mullan wrote:

   https://bugs.openjdk.java.net/browse/JDK-8213161
   http://cr.openjdk.java.net/~xuelei/8212261/webrev.03/


I didn't see a test for SecureCacheResponse - is it possible?

JDK does not have the reference implementation of SecureCacheResponse.


You could also add a test case for IllegalStateExc.


Added in the same test file.

lines 108-113 of HttpsSession.java should be indented 4 more spaces. 
Otherwise looks ok to me.



Updated.

New webrev:
http://cr.openjdk.java.net/~xuelei/8212261/webrev.04/

Thanks,
Xuelei