<div dir="ltr"><br><br><div class="gmail_quote"><div dir="ltr">On Thu, Dec 13, 2018 at 10:23 AM Radim Vansa <<a href="mailto:rvansa@redhat.com">rvansa@redhat.com</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">On 12/12/2018 03:30 PM, Sebastian Laskawiec wrote:<br>
> On Tue, Dec 11, 2018 at 11:13 AM Radim Vansa <<a href="mailto:rvansa@redhat.com" target="_blank">rvansa@redhat.com</a> <br>
> <mailto:<a href="mailto:rvansa@redhat.com" target="_blank">rvansa@redhat.com</a>>> wrote:<br>
><br>
> I dislike having any logic based on the port number in some range;<br>
> it's<br>
> not common that behaviour would change if you set port to 9xxx<br>
> instead<br>
> of 8xxx.<br>
><br>
><br>
> That's not a problem with my approach, since you can always manually <br>
> turn the setting off or on. Here's how you do it:<br>
> ConfigurationBuilder cb = ...<br>
> cb.singlePort(SinglePortMode.ENABLED); // other options: DISABLED and AUTO<br>
<br>
Adding config options is just a way to avoid solving problems :) <br>
Remember the famous quote: "Less knobs!"<br></blockquote><div><br></div><div>It depends on the problem in my opinion. I always preferred to have more configuration options with good defaults.</div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
<br>
><br>
><br>
> Is there an (up-to-date) design doc?<br>
><br>
><br>
> No, this is just a proposal. I was hoping that you guys like it and <br>
> then, with some thumbs up, I could update the design doc.<br>
><br>
> Here's the most up-to-date version in case you were looking for it: <br>
> <a href="https://github.com/infinispan/infinispan-designs/blob/master/Single_port.adoc" rel="noreferrer" target="_blank">https://github.com/infinispan/infinispan-designs/blob/master/Single_port.adoc</a><br>
><br>
><br>
> I don't fully follow, but if there's a problem in the HTTP<br>
> handlers you<br>
> can add a PING-detecting handler below...?<br>
><br>
><br>
> Thanks for the hint Radim!<br>
><br>
> Inspired by your idea I went ahead and checked how OpenShift Router <br>
> behaves. It turns out that it responds HTTP 400 if you throw Hotrod <br>
> bytes at it and then drops the connection.<br>
<br>
I understand that the reason to have Hot Rod PING sent as the first <br>
operation is to make sure that a new client that tries to connect to old <br>
server won't confuse the server, is that correct? Or is there anything else?<br></blockquote><div><br></div><div>Not only. We (Tristan and I when we were discussing it) wanted to introduce the smallest possible hit on a default scenario - a Hotrod client connecting to Infinispan without Single Port. If we mess up this scenario, we might quickly get into all sorts of problem (including performance). </div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
<br>
I'll assume that a new server will handle both Hot Rod PING and HTTP <br>
request correctly without any prelude, and old one will be ok with Hot <br>
Rod PING only. I don't really understand the:<br></blockquote><div><br></div><div>I'm not sure if I follow. The Single Port functionality for non-TLS scenario requires only REST interface. Hotrod is optional. We upgrade to Hotrod using HTTP/1.1 Upgrade procedure (see RFC 7230 [1]). This essentially means, that you need to send an HTTP request in order to upgrade to another protocol (such as Hotrod) and reuse the same TCP connection.</div><div><br></div><div>So as you can see, the Single Port can not speak both protocols, HTTP and Hotrod, at the same time. It requires sending an HTTP request with proper header, and then upgrading to Hotrod. The reason it has been implemented this way is to support Reverse Proxies (like HAProxy in the OpenShift Router, but of course there are more of them). As I mentioned in my last email, the HA proxy immediately responds with HTTP 400 if you throw binary payload at it. That's why we need to follow the HTTP upgrade procedure - to get through it. </div><div><br></div><div>[1] <a href="https://svn.tools.ietf.org/svn/wg/httpbis/specs/rfc7230.html#header.upgrade">https://svn.tools.ietf.org/svn/wg/httpbis/specs/rfc7230.html#header.upgrade</a></div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
<br>
> implementing it this way seems a bit "inconvenient" to me. The Ping<br>
> Operation uses 60s timeout, which seems to be a good fit as a default.<br>
> Unfortunately, for the Single Port functionality, this means we'd need<br>
> to wait 60s until we try to send HTTP request and do an upgrade<br>
<br>
why would you wait for 60 seconds? If the other end is Infinispan server <br>
(old or new), you just send HR PING and you're done, server will proceed <br>
correctly. If the other server is a router, you'll get a response <br>
starting with 'HTTP': in Hot Rod protocol that would be parsed as opCode <br>
0x54 which is illegal response code (the id belongs to <br>
COUNTER_RESET_REQUEST). At this point you know that this connection is <br>
going to be closed, and can immediately start another one (is *this* the <br>
problem?) that will send a HTTP request with Upgrade header.<br></blockquote><div><br></div><div>This is correct. However, you silently assumed that the HTTP Server (either our own Infinispan REST Server or any type of Reverse Proxy we're passing through) will respond with an HTTP message (either 4xx or 5xx) when receiving binary traffic. At this point I know that HAProxy does that, and the same does Envoy (as you told us in the bottom of this email). Infinispan REST server just ignores such a request. </div><div><br></div><div>So it is safe to assume, that all Reverse Proxies will return an HTTP message when we send a bunch of bytes at it? If yes, than we need to correct the REST implementation (that should not be too hard) and kick the Single Port in, when a Hotrod client receives a HTTP message back when sending PING. But then, if some Reverse Proxy between the server and client does something different (ignoring such a request as our Infinispan REST server does now, or simply refuse a connection, or anything else) we won't be able to upgrade the connection. </div><div><br></div><div>My proposal is to set the Single Port support on the client, so that the client starts the communication with HTTP message (and not the Hotrod PING). Doing it this way, allows us to forget about all those crazy situations with Reverse Proxies. The key point is that we never throw raw bytes at them. In my opinion this can save us a lot of headache. </div><div><br></div><div>So, to be honest, I can implement it whichever way you prefer. My personal feeling is that an additional setting on the Hotrod client is much safer bet (at least for now).</div><div><br></div><div>@Tristan, @Ryan, I think you guys also have some experience in this matter. I would be very interested in hearing your opinion as well.</div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
<br>
> I also realized, there's one more moving bit - TCP Keepalive. Luckily, <br>
> we can control this setting over configuration in our standalone.xml. <br>
> However, it is perfectly legal what I've seen in Netty (do not respond <br>
> and keep the connection alive assuming that TCP Keepalive is set to true).<br>
><br>
> The more I think about this, the more I'm convinced that the Single <br>
> Port support should be explicitly set in the client (or inferred from <br>
> the configuration). I do not know how Nginx, Linkerd or Envoy behaves <br>
> in situation when they expect HTTP and get a stream of bytes. Relying <br>
> on this partially unknown behavior for doing our upgrade procedure <br>
> doesn't seem right to me.<br>
<br>
FYI Envoy does the same, send 400 and terminate the connection.<br></blockquote><div><br></div><div>Ok, so it behaves the same as HA Proxy.</div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
<br>
><br>
> Just in case you're worried about the additional logic on the client <br>
> side - it's super small. Really, only 13 lines including brackets ;) <br>
> <a href="https://github.com/infinispan/infinispan/pull/6133/files#diff-684a10c939f31fcfef0f5f48d469393aR618" rel="noreferrer" target="_blank">https://github.com/infinispan/infinispan/pull/6133/files#diff-684a10c939f31fcfef0f5f48d469393aR618</a><br>
<br>
I am not worried about any logic in the client code, I am worried about <br>
logic between chair and keyboard.<br></blockquote><div><br></div><div>Ouch! That was pretty rude, Radim. Words like that make me feel very uncomfortable and actually offend me. I would like to ask you to stick to the technical aspect of this thread and do not go personal.</div><div><br></div><div>As we've known each other for a pretty long time, I will pretend I didn't see this and I just read "please have a look what I just wrote, maybe you will reconsider your implementation?".</div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
<br>
R.<br>
<br>
><br>
> Radim<br>
><br>
> On 12/10/2018 03:27 PM, Sebastian Laskawiec wrote:<br>
> > Hey guys,<br>
> ><br>
> > During Infinispan F2F, I had a short discussion with Tristan on<br>
> Single<br>
> > Port client-side implementation. Back then, we agreed that the<br>
> client<br>
> > should always send a Hotrod Ping request and if won't get any<br>
> response<br>
> > (or get some HTTP content back), it will try to upgrade to the<br>
> Hotrod<br>
> > protocol using Single Port.<br>
> ><br>
> > I've been playing with the implementation for a while, and<br>
> > implementing it this way seems a bit "inconvenient" to me. The Ping<br>
> > Operation uses 60s timeout, which seems to be a good fit as a<br>
> default.<br>
> > Unfortunately, for the Single Port functionality, this means<br>
> we'd need<br>
> > to wait 60s until we try to send HTTP request and do an upgrade.<br>
> Also,<br>
> > another problematic part is in Netty's HTTP handlers<br>
> > (HttpObjectDecoder, HttpServerCodec and ByteToMessageDecoder). When<br>
> > those classes fail to decode a message (REST expects HTTP rather<br>
> than<br>
> > a stream of bytes specific to Hotrod protocol), they just ignore it<br>
> > and keep the channel in active state (which also makes sense for<br>
> > HTTP/1.1 and HTTP/2).<br>
> ><br>
> > At this point, my intuition tells, that this doesn't look right and<br>
> > seems to be a over-complicated. The whole HTTP upgrade idea<br>
> seems to<br>
> > work the other way around, use HTTP as a fallback and then<br>
> upgrade to<br>
> > other protocols. Forcing it to work a bit differently requires some<br>
> > more effort.<br>
> ><br>
> > What if we preserved the Single Port setting in the client<br>
> > configuration but implemented it as an enum with the following<br>
> values<br>
> > - true/false/auto. In automatic mode, the client would check if the<br>
> > server port is set to 8\d{1,3} (this covers 80, 8080, 8081, 8443<br>
> and<br>
> > friends). If that is true, we'd try to follow HTTP Upgrade<br>
> procedure.<br>
> > This looks very simple and I think this might actually work. Please<br>
> > note, that we need the single port setting in the client<br>
> configuration<br>
> > to cover some corner cases like the Single Port exposed on<br>
> different<br>
> > port (like 4444) or Hot Rod exposed on port that starts with 8.<br>
> ><br>
> > What do you think about such simplification?<br>
> ><br>
> > Thanks,<br>
> > Sebastian<br>
> ><br>
> ><br>
> ><br>
> ><br>
> > _______________________________________________<br>
> > infinispan-dev mailing list<br>
> > <a href="mailto:infinispan-dev@lists.jboss.org" target="_blank">infinispan-dev@lists.jboss.org</a><br>
> <mailto:<a href="mailto:infinispan-dev@lists.jboss.org" target="_blank">infinispan-dev@lists.jboss.org</a>><br>
> > <a href="https://lists.jboss.org/mailman/listinfo/infinispan-dev" rel="noreferrer" target="_blank">https://lists.jboss.org/mailman/listinfo/infinispan-dev</a><br>
><br>
><br>
> -- <br>
> Radim Vansa <<a href="mailto:rvansa@redhat.com" target="_blank">rvansa@redhat.com</a> <mailto:<a href="mailto:rvansa@redhat.com" target="_blank">rvansa@redhat.com</a>>><br>
> JBoss Performance Team<br>
><br>
> _______________________________________________<br>
> infinispan-dev mailing list<br>
> <a href="mailto:infinispan-dev@lists.jboss.org" target="_blank">infinispan-dev@lists.jboss.org</a> <mailto:<a href="mailto:infinispan-dev@lists.jboss.org" target="_blank">infinispan-dev@lists.jboss.org</a>><br>
> <a href="https://lists.jboss.org/mailman/listinfo/infinispan-dev" rel="noreferrer" target="_blank">https://lists.jboss.org/mailman/listinfo/infinispan-dev</a><br>
><br>
><br>
><br>
> _______________________________________________<br>
> infinispan-dev mailing list<br>
> <a href="mailto:infinispan-dev@lists.jboss.org" target="_blank">infinispan-dev@lists.jboss.org</a><br>
> <a href="https://lists.jboss.org/mailman/listinfo/infinispan-dev" rel="noreferrer" target="_blank">https://lists.jboss.org/mailman/listinfo/infinispan-dev</a><br>
<br>
<br>
-- <br>
Radim Vansa <<a href="mailto:rvansa@redhat.com" target="_blank">rvansa@redhat.com</a>><br>
JBoss Performance Team<br>
<br>
_______________________________________________<br>
infinispan-dev mailing list<br>
<a href="mailto:infinispan-dev@lists.jboss.org" target="_blank">infinispan-dev@lists.jboss.org</a><br>
<a href="https://lists.jboss.org/mailman/listinfo/infinispan-dev" rel="noreferrer" target="_blank">https://lists.jboss.org/mailman/listinfo/infinispan-dev</a></blockquote></div></div>