8:27 a.m.
Hey guys,
During Infinispan F2F, I had a short discussion with Tristan on Single Port
client-side implementation. Back then, we agreed that the client should
always send a Hotrod Ping request and if won't get any response (or get
some HTTP content back), it will try to upgrade to the Hotrod protocol
using Single Port.
I've been playing with the implementation for a while, and implementing it
this way seems a bit "inconvenient" to me. The Ping Operation uses 60s
timeout, which seems to be a good fit as a default. Unfortunately, for the
Single Port functionality, this means we'd need to wait 60s until we try to
send HTTP request and do an upgrade. Also, another problematic part is in
Netty's HTTP handlers (HttpObjectDecoder, HttpServerCodec and
ByteToMessageDecoder). When those classes fail to decode a message (REST
expects HTTP rather than a stream of bytes specific to Hotrod protocol),
they just ignore it and keep the channel in active state (which also makes
sense for HTTP/1.1 and HTTP/2).
At this point, my intuition tells, that this doesn't look right and seems
to be a over-complicated. The whole HTTP upgrade idea seems to work the
other way around, use HTTP as a fallback and then upgrade to other
protocols. Forcing it to work a bit differently requires some more effort.
What if we preserved the Single Port setting in the client configuration
but implemented it as an enum with the following values - true/false/auto.
In automatic mode, the client would check if the server port is set to
8\d{1,3} (this covers 80, 8080, 8081, 8443 and friends). If that is true,
we'd try to follow HTTP Upgrade procedure. This looks very simple and I
think this might actually work. Please note, that we need the single port
setting in the client configuration to cover some corner cases like the
Single Port exposed on different port (like 4444) or Hot Rod exposed on
port that starts with 8.
What do you think about such simplification?
Thanks,
Sebastian
3:52 a.m.
An updated PR might be found here:
https://github.com/infinispan/infinispan/pull/6133
On Mon, Dec 10, 2018 at 3:27 PM Sebastian Laskawiec <slaskawi(a)redhat.com>
wrote:
Hey guys,
During Infinispan F2F, I had a short discussion with Tristan on Single
Port client-side implementation. Back then, we agreed that the client
should always send a Hotrod Ping request and if won't get any response (or
get some HTTP content back), it will try to upgrade to the Hotrod protocol
using Single Port.
I've been playing with the implementation for a while, and implementing it
this way seems a bit "inconvenient" to me. The Ping Operation uses 60s
timeout, which seems to be a good fit as a default. Unfortunately, for the
Single Port functionality, this means we'd need to wait 60s until we try to
send HTTP request and do an upgrade. Also, another problematic part is in
Netty's HTTP handlers (HttpObjectDecoder, HttpServerCodec and
ByteToMessageDecoder). When those classes fail to decode a message (REST
expects HTTP rather than a stream of bytes specific to Hotrod protocol),
they just ignore it and keep the channel in active state (which also makes
sense for HTTP/1.1 and HTTP/2).
At this point, my intuition tells, that this doesn't look right and seems
to be a over-complicated. The whole HTTP upgrade idea seems to work the
other way around, use HTTP as a fallback and then upgrade to other
protocols. Forcing it to work a bit differently requires some more effort.
What if we preserved the Single Port setting in the client configuration
but implemented it as an enum with the following values - true/false/auto.
In automatic mode, the client would check if the server port is set to
8\d{1,3} (this covers 80, 8080, 8081, 8443 and friends). If that is true,
we'd try to follow HTTP Upgrade procedure. This looks very simple and I
think this might actually work. Please note, that we need the single port
setting in the client configuration to cover some corner cases like the
Single Port exposed on different port (like 4444) or Hot Rod exposed on
port that starts with 8.
What do you think about such simplification?
Thanks,
Sebastian
4:13 a.m.
I dislike having any logic based on the port number in some range; it's
not common that behaviour would change if you set port to 9xxx instead
of 8xxx.
Is there an (up-to-date) design doc?
I don't fully follow, but if there's a problem in the HTTP handlers you
can add a PING-detecting handler below...?
Radim
On 12/10/2018 03:27 PM, Sebastian Laskawiec wrote:
Hey guys,
During Infinispan F2F, I had a short discussion with Tristan on Single
Port client-side implementation. Back then, we agreed that the client
should always send a Hotrod Ping request and if won't get any response
(or get some HTTP content back), it will try to upgrade to the Hotrod
protocol using Single Port.
I've been playing with the implementation for a while, and
implementing it this way seems a bit "inconvenient" to me. The Ping
Operation uses 60s timeout, which seems to be a good fit as a default.
Unfortunately, for the Single Port functionality, this means we'd need
to wait 60s until we try to send HTTP request and do an upgrade. Also,
another problematic part is in Netty's HTTP handlers
(HttpObjectDecoder, HttpServerCodec and ByteToMessageDecoder). When
those classes fail to decode a message (REST expects HTTP rather than
a stream of bytes specific to Hotrod protocol), they just ignore it
and keep the channel in active state (which also makes sense for
HTTP/1.1 and HTTP/2).
At this point, my intuition tells, that this doesn't look right and
seems to be a over-complicated. The whole HTTP upgrade idea seems to
work the other way around, use HTTP as a fallback and then upgrade to
other protocols. Forcing it to work a bit differently requires some
more effort.
What if we preserved the Single Port setting in the client
configuration but implemented it as an enum with the following values
- true/false/auto. In automatic mode, the client would check if the
server port is set to 8\d{1,3} (this covers 80, 8080, 8081, 8443 and
friends). If that is true, we'd try to follow HTTP Upgrade procedure.
This looks very simple and I think this might actually work. Please
note, that we need the single port setting in the client configuration
to cover some corner cases like the Single Port exposed on different
port (like 4444) or Hot Rod exposed on port that starts with 8.
What do you think about such simplification?
Thanks,
Sebastian
_______________________________________________
infinispan-dev mailing list
infinispan-dev(a)lists.jboss.org
https://lists.jboss.org/mailman/listinfo/infinispan-dev
--
Radim Vansa <rvansa(a)redhat.com>
JBoss Performance Team
8:30 a.m.
On Tue, Dec 11, 2018 at 11:13 AM Radim Vansa <rvansa(a)redhat.com> wrote:
I dislike having any logic based on the port number in some range;
it's
not common that behaviour would change if you set port to 9xxx instead
of 8xxx.
That's not a problem with my approach, since you can always manually turn
the setting off or on. Here's how you do it:
ConfigurationBuilder cb = ...
cb.singlePort(SinglePortMode.ENABLED); // other options: DISABLED and AUTO
Is there an (up-to-date) design doc?
No, this is just a proposal. I was hoping that you guys like it and then,
with some thumbs up, I could update the design doc.
Here's the most up-to-date version in case you were looking for it:
https://github.com/infinispan/infinispan-designs/blob/master/Single_port....
I don't fully follow, but if there's a problem in the HTTP handlers you
can add a PING-detecting handler below...?
Thanks for the hint Radim!
Inspired by your idea I went ahead and checked how OpenShift Router
behaves. It turns out that it responds HTTP 400 if you throw Hotrod bytes
at it and then drops the connection. I also realized, there's one more
moving bit - TCP Keepalive. Luckily, we can control this setting over
configuration in our standalone.xml. However, it is perfectly legal what
I've seen in Netty (do not respond and keep the connection alive assuming
that TCP Keepalive is set to true).
The more I think about this, the more I'm convinced that the Single Port
support should be explicitly set in the client (or inferred from the
configuration). I do not know how Nginx, Linkerd or Envoy behaves in
situation when they expect HTTP and get a stream of bytes. Relying on this
partially unknown behavior for doing our upgrade procedure doesn't seem
right to me.
Just in case you're worried about the additional logic on the client side -
it's super small. Really, only 13 lines including brackets ;)
https://github.com/infinispan/infinispan/pull/6133/files#diff-684a10c939f...
Radim
On 12/10/2018 03:27 PM, Sebastian Laskawiec wrote:
> Hey guys,
>
> During Infinispan F2F, I had a short discussion with Tristan on Single
> Port client-side implementation. Back then, we agreed that the client
> should always send a Hotrod Ping request and if won't get any response
> (or get some HTTP content back), it will try to upgrade to the Hotrod
> protocol using Single Port.
>
> I've been playing with the implementation for a while, and
> implementing it this way seems a bit "inconvenient" to me. The Ping
> Operation uses 60s timeout, which seems to be a good fit as a default.
> Unfortunately, for the Single Port functionality, this means we'd need
> to wait 60s until we try to send HTTP request and do an upgrade. Also,
> another problematic part is in Netty's HTTP handlers
> (HttpObjectDecoder, HttpServerCodec and ByteToMessageDecoder). When
> those classes fail to decode a message (REST expects HTTP rather than
> a stream of bytes specific to Hotrod protocol), they just ignore it
> and keep the channel in active state (which also makes sense for
> HTTP/1.1 and HTTP/2).
>
> At this point, my intuition tells, that this doesn't look right and
> seems to be a over-complicated. The whole HTTP upgrade idea seems to
> work the other way around, use HTTP as a fallback and then upgrade to
> other protocols. Forcing it to work a bit differently requires some
> more effort.
>
> What if we preserved the Single Port setting in the client
> configuration but implemented it as an enum with the following values
> - true/false/auto. In automatic mode, the client would check if the
> server port is set to 8\d{1,3} (this covers 80, 8080, 8081, 8443 and
> friends). If that is true, we'd try to follow HTTP Upgrade procedure.
> This looks very simple and I think this might actually work. Please
> note, that we need the single port setting in the client configuration
> to cover some corner cases like the Single Port exposed on different
> port (like 4444) or Hot Rod exposed on port that starts with 8.
>
> What do you think about such simplification?
>
> Thanks,
> Sebastian
>
>
>
>
> _______________________________________________
> infinispan-dev mailing list
> infinispan-dev(a)lists.jboss.org
> https://lists.jboss.org/mailman/listinfo/infinispan-dev
--
Radim Vansa <rvansa(a)redhat.com>
JBoss Performance Team
_______________________________________________
infinispan-dev mailing list
infinispan-dev(a)lists.jboss.org
https://lists.jboss.org/mailman/listinfo/infinispan-dev
3:08 a.m.
On 12/12/2018 03:30 PM, Sebastian Laskawiec wrote:
On Tue, Dec 11, 2018 at 11:13 AM Radim Vansa <rvansa(a)redhat.com
<mailto:rvansa@redhat.com>> wrote:
I dislike having any logic based on the port number in some range;
it's
not common that behaviour would change if you set port to 9xxx
instead
of 8xxx.
That's not a problem with my approach, since you can always manually
turn the setting off or on. Here's how you do it:
ConfigurationBuilder cb = ...
cb.singlePort(SinglePortMode.ENABLED); // other options: DISABLED and AUTO
Adding config options is just a way to avoid solving problems :)
Remember the famous quote: "Less knobs!"
Is there an (up-to-date) design doc?
No, this is just a proposal. I was hoping that you guys like it and
then, with some thumbs up, I could update the design doc.
Here's the most up-to-date version in case you were looking for it:
https://github.com/infinispan/infinispan-designs/blob/master/Single_port....
I don't fully follow, but if there's a problem in the HTTP
handlers you
can add a PING-detecting handler below...?
Thanks for the hint Radim!
Inspired by your idea I went ahead and checked how OpenShift Router
behaves. It turns out that it responds HTTP 400 if you throw Hotrod
bytes at it and then drops the connection.
I understand that the reason to have Hot Rod PING sent as the first
operation is to make sure that a new client that tries to connect to old
server won't confuse the server, is that correct? Or is there anything else?
I'll assume that a new server will handle both Hot Rod PING and HTTP
request correctly without any prelude, and old one will be ok with Hot
Rod PING only. I don't really understand the:
implementing it this way seems a bit "inconvenient" to me.
The Ping
Operation uses 60s timeout, which seems to be a good fit as a default.
Unfortunately, for the Single Port functionality, this means we'd need
to wait 60s until we try to send HTTP request and do an upgrade
why would you wait for 60 seconds? If the other end is Infinispan server
(old or new), you just send HR PING and you're done, server will proceed
correctly. If the other server is a router, you'll get a response
starting with 'HTTP': in Hot Rod protocol that would be parsed as opCode
0x54 which is illegal response code (the id belongs to
COUNTER_RESET_REQUEST). At this point you know that this connection is
going to be closed, and can immediately start another one (is *this* the
problem?) that will send a HTTP request with Upgrade header.
I also realized, there's one more moving bit - TCP Keepalive.
Luckily,
we can control this setting over configuration in our standalone.xml.
However, it is perfectly legal what I've seen in Netty (do not respond
and keep the connection alive assuming that TCP Keepalive is set to true).
The more I think about this, the more I'm convinced that the Single
Port support should be explicitly set in the client (or inferred from
the configuration). I do not know how Nginx, Linkerd or Envoy behaves
in situation when they expect HTTP and get a stream of bytes. Relying
on this partially unknown behavior for doing our upgrade procedure
doesn't seem right to me.
FYI Envoy does the same, send 400 and terminate the connection.
Just in case you're worried about the additional logic on the client
side - it's super small. Really, only 13 lines including brackets ;)
https://github.com/infinispan/infinispan/pull/6133/files#diff-684a10c939f...
I am not worried about any logic in the client code, I am worried about
logic between chair and keyboard.
R.
Radim
On 12/10/2018 03:27 PM, Sebastian Laskawiec wrote:
> Hey guys,
>
> During Infinispan F2F, I had a short discussion with Tristan on
Single
> Port client-side implementation. Back then, we agreed that the
client
> should always send a Hotrod Ping request and if won't get any
response
> (or get some HTTP content back), it will try to upgrade to the
Hotrod
> protocol using Single Port.
>
> I've been playing with the implementation for a while, and
> implementing it this way seems a bit "inconvenient" to me. The Ping
> Operation uses 60s timeout, which seems to be a good fit as a
default.
> Unfortunately, for the Single Port functionality, this means
we'd need
> to wait 60s until we try to send HTTP request and do an upgrade.
Also,
> another problematic part is in Netty's HTTP handlers
> (HttpObjectDecoder, HttpServerCodec and ByteToMessageDecoder). When
> those classes fail to decode a message (REST expects HTTP rather
than
> a stream of bytes specific to Hotrod protocol), they just ignore it
> and keep the channel in active state (which also makes sense for
> HTTP/1.1 and HTTP/2).
>
> At this point, my intuition tells, that this doesn't look right and
> seems to be a over-complicated. The whole HTTP upgrade idea
seems to
> work the other way around, use HTTP as a fallback and then
upgrade to
> other protocols. Forcing it to work a bit differently requires some
> more effort.
>
> What if we preserved the Single Port setting in the client
> configuration but implemented it as an enum with the following
values
> - true/false/auto. In automatic mode, the client would check if the
> server port is set to 8\d{1,3} (this covers 80, 8080, 8081, 8443
and
> friends). If that is true, we'd try to follow HTTP Upgrade
procedure.
> This looks very simple and I think this might actually work. Please
> note, that we need the single port setting in the client
configuration
> to cover some corner cases like the Single Port exposed on
different
> port (like 4444) or Hot Rod exposed on port that starts with 8.
>
> What do you think about such simplification?
>
> Thanks,
> Sebastian
>
>
>
>
> _______________________________________________
> infinispan-dev mailing list
> infinispan-dev(a)lists.jboss.org
<mailto:infinispan-dev@lists.jboss.org>
> https://lists.jboss.org/mailman/listinfo/infinispan-dev
--
Radim Vansa <rvansa(a)redhat.com <mailto:rvansa@redhat.com>>
JBoss Performance Team
_______________________________________________
infinispan-dev mailing list
infinispan-dev(a)lists.jboss.org <mailto:infinispan-dev@lists.jboss.org>
https://lists.jboss.org/mailman/listinfo/infinispan-dev
_______________________________________________
infinispan-dev mailing list
infinispan-dev(a)lists.jboss.org
https://lists.jboss.org/mailman/listinfo/infinispan-dev
--
Radim Vansa <rvansa(a)redhat.com>
JBoss Performance Team
7:45 a.m.
On Thu, Dec 13, 2018 at 10:23 AM Radim Vansa <rvansa(a)redhat.com> wrote:
On 12/12/2018 03:30 PM, Sebastian Laskawiec wrote:
> On Tue, Dec 11, 2018 at 11:13 AM Radim Vansa <rvansa(a)redhat.com
> <mailto:rvansa@redhat.com>> wrote:
>
> I dislike having any logic based on the port number in some range;
> it's
> not common that behaviour would change if you set port to 9xxx
> instead
> of 8xxx.
>
>
> That's not a problem with my approach, since you can always manually
> turn the setting off or on. Here's how you do it:
> ConfigurationBuilder cb = ...
> cb.singlePort(SinglePortMode.ENABLED); // other options: DISABLED and
AUTO
Adding config options is just a way to avoid solving problems :)
Remember the famous quote: "Less knobs!"
It depends on the problem in my opinion. I always preferred to have more
configuration options with good defaults.
>
>
> Is there an (up-to-date) design doc?
>
>
> No, this is just a proposal. I was hoping that you guys like it and
> then, with some thumbs up, I could update the design doc.
>
> Here's the most up-to-date version in case you were looking for it:
>
https://github.com/infinispan/infinispan-designs/blob/master/Single_port....
>
>
> I don't fully follow, but if there's a problem in the HTTP
> handlers you
> can add a PING-detecting handler below...?
>
>
> Thanks for the hint Radim!
>
> Inspired by your idea I went ahead and checked how OpenShift Router
> behaves. It turns out that it responds HTTP 400 if you throw Hotrod
> bytes at it and then drops the connection.
I understand that the reason to have Hot Rod PING sent as the first
operation is to make sure that a new client that tries to connect to old
server won't confuse the server, is that correct? Or is there anything
else?
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).
I'll assume that a new server will handle both Hot Rod PING and HTTP
request correctly without any prelude, and old one will be ok with Hot
Rod PING only. I don't really understand the:
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.
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.
[1]
https://svn.tools.ietf.org/svn/wg/httpbis/specs/rfc7230.html#header.upgrade
> implementing it this way seems a bit "inconvenient" to me. The Ping
> Operation uses 60s timeout, which seems to be a good fit as a default.
> Unfortunately, for the Single Port functionality, this means we'd need
> to wait 60s until we try to send HTTP request and do an upgrade
why would you wait for 60 seconds? If the other end is Infinispan server
(old or new), you just send HR PING and you're done, server will proceed
correctly. If the other server is a router, you'll get a response
starting with 'HTTP': in Hot Rod protocol that would be parsed as opCode
0x54 which is illegal response code (the id belongs to
COUNTER_RESET_REQUEST). At this point you know that this connection is
going to be closed, and can immediately start another one (is *this* the
problem?) that will send a HTTP request with Upgrade header.
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.
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.
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.
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).
@Tristan, @Ryan, I think you guys also have some experience in this matter.
I would be very interested in hearing your opinion as well.
> I also realized, there's one more moving bit - TCP Keepalive. Luckily,
> we can control this setting over configuration in our standalone.xml.
> However, it is perfectly legal what I've seen in Netty (do not respond
> and keep the connection alive assuming that TCP Keepalive is set to
true).
>
> The more I think about this, the more I'm convinced that the Single
> Port support should be explicitly set in the client (or inferred from
> the configuration). I do not know how Nginx, Linkerd or Envoy behaves
> in situation when they expect HTTP and get a stream of bytes. Relying
> on this partially unknown behavior for doing our upgrade procedure
> doesn't seem right to me.
FYI Envoy does the same, send 400 and terminate the connection.
Ok, so it behaves the same as HA Proxy.
>
> Just in case you're worried about the additional logic on the client
> side - it's super small. Really, only 13 lines including brackets ;)
>
https://github.com/infinispan/infinispan/pull/6133/files#diff-684a10c939f...
I am not worried about any logic in the client code, I am worried about
logic between chair and keyboard.
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.
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?".
R.
>
> Radim
>
> On 12/10/2018 03:27 PM, Sebastian Laskawiec wrote:
> > Hey guys,
> >
> > During Infinispan F2F, I had a short discussion with Tristan on
> Single
> > Port client-side implementation. Back then, we agreed that the
> client
> > should always send a Hotrod Ping request and if won't get any
> response
> > (or get some HTTP content back), it will try to upgrade to the
> Hotrod
> > protocol using Single Port.
> >
> > I've been playing with the implementation for a while, and
> > implementing it this way seems a bit "inconvenient" to me. The
Ping
> > Operation uses 60s timeout, which seems to be a good fit as a
> default.
> > Unfortunately, for the Single Port functionality, this means
> we'd need
> > to wait 60s until we try to send HTTP request and do an upgrade.
> Also,
> > another problematic part is in Netty's HTTP handlers
> > (HttpObjectDecoder, HttpServerCodec and ByteToMessageDecoder). When
> > those classes fail to decode a message (REST expects HTTP rather
> than
> > a stream of bytes specific to Hotrod protocol), they just ignore it
> > and keep the channel in active state (which also makes sense for
> > HTTP/1.1 and HTTP/2).
> >
> > At this point, my intuition tells, that this doesn't look right and
> > seems to be a over-complicated. The whole HTTP upgrade idea
> seems to
> > work the other way around, use HTTP as a fallback and then
> upgrade to
> > other protocols. Forcing it to work a bit differently requires some
> > more effort.
> >
> > What if we preserved the Single Port setting in the client
> > configuration but implemented it as an enum with the following
> values
> > - true/false/auto. In automatic mode, the client would check if the
> > server port is set to 8\d{1,3} (this covers 80, 8080, 8081, 8443
> and
> > friends). If that is true, we'd try to follow HTTP Upgrade
> procedure.
> > This looks very simple and I think this might actually work. Please
> > note, that we need the single port setting in the client
> configuration
> > to cover some corner cases like the Single Port exposed on
> different
> > port (like 4444) or Hot Rod exposed on port that starts with 8.
> >
> > What do you think about such simplification?
> >
> > Thanks,
> > Sebastian
> >
> >
> >
> >
> > _______________________________________________
> > infinispan-dev mailing list
> > infinispan-dev(a)lists.jboss.org
> <mailto:infinispan-dev@lists.jboss.org>
> > https://lists.jboss.org/mailman/listinfo/infinispan-dev
>
>
> --
> Radim Vansa <rvansa(a)redhat.com <mailto:rvansa@redhat.com>>
> JBoss Performance Team
>
> _______________________________________________
> infinispan-dev mailing list
> infinispan-dev(a)lists.jboss.org <mailto:
infinispan-dev(a)lists.jboss.org>
> https://lists.jboss.org/mailman/listinfo/infinispan-dev
>
>
>
> _______________________________________________
> infinispan-dev mailing list
> infinispan-dev(a)lists.jboss.org
> https://lists.jboss.org/mailman/listinfo/infinispan-dev
--
Radim Vansa <rvansa(a)redhat.com>
JBoss Performance Team
_______________________________________________
infinispan-dev mailing list
infinispan-dev(a)lists.jboss.org
https://lists.jboss.org/mailman/listinfo/infinispan-dev
9:38 a.m.
Hey Sebastian,
I am sorry that you took my comment about errors between chair and
keyboard completely wrong: it was definitely not pointed at you! What I
meant is that users will often misconfigure (I've been there many
times), and their experience will suffer. Therefore I am not concerned
about any 'smart' logic we write (regardless of number of lines of
code), but about any complexity required from, or offered to the users.
Rest of the comments inline...
On 12/13/2018 02:45 PM, Sebastian Laskawiec wrote:
On Thu, Dec 13, 2018 at 10:23 AM Radim Vansa <rvansa(a)redhat.com
<mailto:rvansa@redhat.com>> wrote:
On 12/12/2018 03:30 PM, Sebastian Laskawiec wrote:
> On Tue, Dec 11, 2018 at 11:13 AM Radim Vansa <rvansa(a)redhat.com
<mailto:rvansa@redhat.com>
> <mailto:rvansa@redhat.com <mailto:rvansa@redhat.com>>> wrote:
>
> I dislike having any logic based on the port number in some
range;
> it's
> not common that behaviour would change if you set port to 9xxx
> instead
> of 8xxx.
>
>
> That's not a problem with my approach, since you can always
manually
> turn the setting off or on. Here's how you do it:
> ConfigurationBuilder cb = ...
> cb.singlePort(SinglePortMode.ENABLED); // other options:
DISABLED and AUTO
Adding config options is just a way to avoid solving problems :)
Remember the famous quote: "Less knobs!"
It depends on the problem in my opinion. I always preferred to have
more configuration options with good defaults.
>
>
> Is there an (up-to-date) design doc?
>
>
> No, this is just a proposal. I was hoping that you guys like it and
> then, with some thumbs up, I could update the design doc.
>
> Here's the most up-to-date version in case you were looking for it:
>
https://github.com/infinispan/infinispan-designs/blob/master/Single_port....
>
>
> I don't fully follow, but if there's a problem in the HTTP
> handlers you
> can add a PING-detecting handler below...?
>
>
> Thanks for the hint Radim!
>
> Inspired by your idea I went ahead and checked how OpenShift Router
> behaves. It turns out that it responds HTTP 400 if you throw Hotrod
> bytes at it and then drops the connection.
I understand that the reason to have Hot Rod PING sent as the first
operation is to make sure that a new client that tries to connect
to old
server won't confuse the server, is that correct? Or is there
anything else?
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).
I'll assume that a new server will handle both Hot Rod PING and HTTP
request correctly without any prelude, and old one will be ok with
Hot
Rod PING only. I don't really understand the:
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.
Okay, then my assumption was incorrect. That means that single-port
server and non-single-port server are incompatible because neither can
accept other's initial request :-/
It seems that Hot Rod and HTTP communication is easily distinguishable
with first few bytes: could we enhance single-port to be able to handle
Hot Rod PING immediately without the HTTP Upgrade to accommodate older
clients? Then we could run single-port all the time and everyone wins.
Btw., I assume that we can't handle both TLS and non-TLS connections on
same port either, is that correct? Usually webservers can't be
configured to handle both HTTP and HTTPS on the same port - is there any
difference here?
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.
[1]
https://svn.tools.ietf.org/svn/wg/httpbis/specs/rfc7230.html#header.upgrade
> implementing it this way seems a bit "inconvenient" to me. The Ping
> Operation uses 60s timeout, which seems to be a good fit as a
default.
> Unfortunately, for the Single Port functionality, this means
we'd need
> to wait 60s until we try to send HTTP request and do an upgrade
why would you wait for 60 seconds? If the other end is Infinispan
server
(old or new), you just send HR PING and you're done, server will
proceed
correctly. If the other server is a router, you'll get a response
starting with 'HTTP': in Hot Rod protocol that would be parsed as
opCode
0x54 which is illegal response code (the id belongs to
COUNTER_RESET_REQUEST). At this point you know that this
connection is
going to be closed, and can immediately start another one (is
*this* the
problem?) that will send a HTTP request with Upgrade header.
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.
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.
That's a valid point - the spec says that it SHOULD [1] respond with 400
but it is not 100% (buggy proxies are to be expected on the internet).
We should probably correct the REST server...
[1] https://tools.ietf.org/html/rfc7230#section-3.1.1
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.
I am starting to think that we really should start the communication
with HTTP message, and this shouldn't be a separate config property
(single-port) but based on Hot Rod version.
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).
@Tristan, @Ryan, I think you guys also have some experience in this
matter. I would be very interested in hearing your opinion as well.
> I also realized, there's one more moving bit - TCP Keepalive.
Luckily,
> we can control this setting over configuration in our
standalone.xml.
> However, it is perfectly legal what I've seen in Netty (do not
respond
> and keep the connection alive assuming that TCP Keepalive is set
to true).
>
> The more I think about this, the more I'm convinced that the Single
> Port support should be explicitly set in the client (or inferred
from
> the configuration). I do not know how Nginx, Linkerd or Envoy
behaves
> in situation when they expect HTTP and get a stream of bytes.
Relying
> on this partially unknown behavior for doing our upgrade procedure
> doesn't seem right to me.
FYI Envoy does the same, send 400 and terminate the connection.
Ok, so it behaves the same as HA Proxy.
>
> Just in case you're worried about the additional logic on the
client
> side - it's super small. Really, only 13 lines including
brackets ;)
>
https://github.com/infinispan/infinispan/pull/6133/files#diff-684a10c939f...
I am not worried about any logic in the client code, I am worried
about
logic between chair and keyboard.
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.
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?".
See the top, please, and my apologies if you took the above as an
offense. TBH I haven't even checked the implementation, and I am far
from judging it.
Radim
R.
>
> Radim
>
> On 12/10/2018 03:27 PM, Sebastian Laskawiec wrote:
> > Hey guys,
> >
> > During Infinispan F2F, I had a short discussion with
Tristan on
> Single
> > Port client-side implementation. Back then, we agreed that the
> client
> > should always send a Hotrod Ping request and if won't get any
> response
> > (or get some HTTP content back), it will try to upgrade to the
> Hotrod
> > protocol using Single Port.
> >
> > I've been playing with the implementation for a while, and
> > implementing it this way seems a bit "inconvenient" to me.
The Ping
> > Operation uses 60s timeout, which seems to be a good fit as a
> default.
> > Unfortunately, for the Single Port functionality, this means
> we'd need
> > to wait 60s until we try to send HTTP request and do an
upgrade.
> Also,
> > another problematic part is in Netty's HTTP handlers
> > (HttpObjectDecoder, HttpServerCodec and
ByteToMessageDecoder). When
> > those classes fail to decode a message (REST expects HTTP
rather
> than
> > a stream of bytes specific to Hotrod protocol), they just
ignore it
> > and keep the channel in active state (which also makes
sense for
> > HTTP/1.1 and HTTP/2).
> >
> > At this point, my intuition tells, that this doesn't look
right and
> > seems to be a over-complicated. The whole HTTP upgrade idea
> seems to
> > work the other way around, use HTTP as a fallback and then
> upgrade to
> > other protocols. Forcing it to work a bit differently
requires some
> > more effort.
> >
> > What if we preserved the Single Port setting in the client
> > configuration but implemented it as an enum with the following
> values
> > - true/false/auto. In automatic mode, the client would
check if the
> > server port is set to 8\d{1,3} (this covers 80, 8080,
8081, 8443
> and
> > friends). If that is true, we'd try to follow HTTP Upgrade
> procedure.
> > This looks very simple and I think this might actually
work. Please
> > note, that we need the single port setting in the client
> configuration
> > to cover some corner cases like the Single Port exposed on
> different
> > port (like 4444) or Hot Rod exposed on port that starts
with 8.
> >
> > What do you think about such simplification?
> >
> > Thanks,
> > Sebastian
> >
> >
> >
> >
> > _______________________________________________
> > infinispan-dev mailing list
> > infinispan-dev(a)lists.jboss.org
<mailto:infinispan-dev@lists.jboss.org>
> <mailto:infinispan-dev@lists.jboss.org
<mailto:infinispan-dev@lists.jboss.org>>
> > https://lists.jboss.org/mailman/listinfo/infinispan-dev
>
>
> --
> Radim Vansa <rvansa(a)redhat.com <mailto:rvansa@redhat.com>
<mailto:rvansa@redhat.com <mailto:rvansa@redhat.com>>>
> JBoss Performance Team
>
> _______________________________________________
> infinispan-dev mailing list
> infinispan-dev(a)lists.jboss.org
<mailto:infinispan-dev@lists.jboss.org>
<mailto:infinispan-dev@lists.jboss.org
<mailto:infinispan-dev@lists.jboss.org>>
> https://lists.jboss.org/mailman/listinfo/infinispan-dev
>
>
>
> _______________________________________________
> infinispan-dev mailing list
> infinispan-dev(a)lists.jboss.org
<mailto:infinispan-dev@lists.jboss.org>
> https://lists.jboss.org/mailman/listinfo/infinispan-dev
--
Radim Vansa <rvansa(a)redhat.com <mailto:rvansa@redhat.com>>
JBoss Performance Team
_______________________________________________
infinispan-dev mailing list
infinispan-dev(a)lists.jboss.org <mailto:infinispan-dev@lists.jboss.org>
https://lists.jboss.org/mailman/listinfo/infinispan-dev
_______________________________________________
infinispan-dev mailing list
infinispan-dev(a)lists.jboss.org
https://lists.jboss.org/mailman/listinfo/infinispan-dev
--
Radim Vansa <rvansa(a)redhat.com>
JBoss Performance Team
4:49 a.m.
On Thu, Dec 13, 2018 at 4:42 PM Radim Vansa <rvansa(a)redhat.com> wrote:
Hey Sebastian,
I am sorry that you took my comment about errors between chair and
keyboard completely wrong: it was definitely not pointed at you! What I
meant is that users will often misconfigure (I've been there many
times), and their experience will suffer. Therefore I am not concerned
about any 'smart' logic we write (regardless of number of lines of
code), but about any complexity required from, or offered to the users.
No problem, Radim. I was a little but surprised by that "left jab" coming
from nowhere ;) Emails are such a bad media of communication when it comes
to situations like this. In normal F2F communication that would never
happen. I'm also sorry for all the confusion...
This actually made me thinking, maybe it's the right time to do a small 15
min meeting to discuss this F2F? @Tristan, @Ryan, @Galder - would you also
be interested? I'll prepare some slides to show you the main pain points.
Rest of the comments inline...
On 12/13/2018 02:45 PM, Sebastian Laskawiec wrote:
>
>
> On Thu, Dec 13, 2018 at 10:23 AM Radim Vansa <rvansa(a)redhat.com
> <mailto:rvansa@redhat.com>> wrote:
>
> On 12/12/2018 03:30 PM, Sebastian Laskawiec wrote:
> > On Tue, Dec 11, 2018 at 11:13 AM Radim Vansa <rvansa(a)redhat.com
> <mailto:rvansa@redhat.com>
> > <mailto:rvansa@redhat.com <mailto:rvansa@redhat.com>>>
wrote:
> >
> > I dislike having any logic based on the port number in some
> range;
> > it's
> > not common that behaviour would change if you set port to 9xxx
> > instead
> > of 8xxx.
> >
> >
> > That's not a problem with my approach, since you can always
> manually
> > turn the setting off or on. Here's how you do it:
> > ConfigurationBuilder cb = ...
> > cb.singlePort(SinglePortMode.ENABLED); // other options:
> DISABLED and AUTO
>
> Adding config options is just a way to avoid solving problems :)
> Remember the famous quote: "Less knobs!"
>
>
> It depends on the problem in my opinion. I always preferred to have
> more configuration options with good defaults.
>
>
> >
> >
> > Is there an (up-to-date) design doc?
> >
> >
> > No, this is just a proposal. I was hoping that you guys like it and
> > then, with some thumbs up, I could update the design doc.
> >
> > Here's the most up-to-date version in case you were looking for it:
> >
>
https://github.com/infinispan/infinispan-designs/blob/master/Single_port....
> >
> >
> > I don't fully follow, but if there's a problem in the HTTP
> > handlers you
> > can add a PING-detecting handler below...?
> >
> >
> > Thanks for the hint Radim!
> >
> > Inspired by your idea I went ahead and checked how OpenShift Router
> > behaves. It turns out that it responds HTTP 400 if you throw Hotrod
> > bytes at it and then drops the connection.
>
> I understand that the reason to have Hot Rod PING sent as the first
> operation is to make sure that a new client that tries to connect
> to old
> server won't confuse the server, is that correct? Or is there
> anything else?
>
>
> 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).
>
>
> I'll assume that a new server will handle both Hot Rod PING and HTTP
> request correctly without any prelude, and old one will be ok with
> Hot
> Rod PING only. I don't really understand the:
>
>
> 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.
Okay, then my assumption was incorrect. That means that single-port
server and non-single-port server are incompatible because neither can
accept other's initial request :-/
It seems that Hot Rod and HTTP communication is easily distinguishable
with first few bytes: could we enhance single-port to be able to handle
Hot Rod PING immediately without the HTTP Upgrade to accommodate older
clients? Then we could run single-port all the time and everyone wins.
Yes, that's absolutely doable, but it doesn't solve the problem, because
there might be some Reverse Proxies on the way (such as an OCP Router,
Nginx or anything else). Those proxies might mess up with our upgrade flow
(as you mentioned below, there can be buggy proxies on the way). The main
problem is that we don't know (we can assume but we are not 100% certain),
how the Reverse Proxy will behave if we throw binary protocol at it.
Also, if we always start with Single Port turned on, we'll add additional
round trip (send HTTP with upgrade request, wait for response, then switch
to Hotrod). This adds some delay and I'm not sure we should do it. If we
wanted to consider this, I would need to get Tristan's "green stamp" on it.
But I have serious doubts
Btw., I assume that we can't handle both TLS and non-TLS connections on
same port either, is that correct? Usually webservers can't be
configured to handle both HTTP and HTTPS on the same port - is there any
difference here?
The encrypted use case is soooo much easier. The protocol negotiation
happen during TLS handshake. Once it's finished and connection is
established (so after the handshake is done), you already know about the
communication protocol you will be using. If we wanted to support only
encrypted use cases, the implementation would in 10-20 lines of code.
Frankly, I expect people to be using unencrypted use cases when developing
features and encrypted use cases on production. That's my gut feeling how
this feature will be used.
>
> 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.
>
> [1]
>
https://svn.tools.ietf.org/svn/wg/httpbis/specs/rfc7230.html#header.upgrade
>
>
> > implementing it this way seems a bit "inconvenient" to me. The
Ping
> > Operation uses 60s timeout, which seems to be a good fit as a
> default.
> > Unfortunately, for the Single Port functionality, this means
> we'd need
> > to wait 60s until we try to send HTTP request and do an upgrade
>
> why would you wait for 60 seconds? If the other end is Infinispan
> server
> (old or new), you just send HR PING and you're done, server will
> proceed
> correctly. If the other server is a router, you'll get a response
> starting with 'HTTP': in Hot Rod protocol that would be parsed as
> opCode
> 0x54 which is illegal response code (the id belongs to
> COUNTER_RESET_REQUEST). At this point you know that this
> connection is
> going to be closed, and can immediately start another one (is
> *this* the
> problem?) that will send a HTTP request with Upgrade header.
>
>
> 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.
>
> 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.
That's a valid point - the spec says that it SHOULD [1] respond with 400
but it is not 100% (buggy proxies are to be expected on the internet).
We should probably correct the REST server...
Thanks for double checking. Yes, I believe we should. Let me test it a
little bit more and create proper JIRAs.
[1] https://tools.ietf.org/html/rfc7230#section-3.1.1
>
> 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.
I am starting to think that we really should start the communication
with HTTP message, and this shouldn't be a separate config property
(single-port) but based on Hot Rod version.
I also through about this but finally, I rejected the idea. The main
reasoning behind is this:
- We mess up with our default execution flow - hotrod client connecting to
a hotrod server. If there's any bug there we miss during testing, there is
a high chance we get a critical bug in return. And since Infinispan is not
my primary responsibility at the moment, I'm worried that this will cause
troubles and additional pressure to the team.
- We perform additional round trip when initiating a connection (as I
explained before in this email).
- The default server configuration would be more complicated (especially
the endpoint configuration).
- The final argument (probably the least important) is that we don't
support Websockets and Memcached in Single Port. So the configuration
wouldn't be symmetric. It's probably a nitpick, but I don't like it.
>
> 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).
>
> @Tristan, @Ryan, I think you guys also have some experience in this
> matter. I would be very interested in hearing your opinion as well.
>
>
> > I also realized, there's one more moving bit - TCP Keepalive.
> Luckily,
> > we can control this setting over configuration in our
> standalone.xml.
> > However, it is perfectly legal what I've seen in Netty (do not
> respond
> > and keep the connection alive assuming that TCP Keepalive is set
> to true).
> >
> > The more I think about this, the more I'm convinced that the Single
> > Port support should be explicitly set in the client (or inferred
> from
> > the configuration). I do not know how Nginx, Linkerd or Envoy
> behaves
> > in situation when they expect HTTP and get a stream of bytes.
> Relying
> > on this partially unknown behavior for doing our upgrade procedure
> > doesn't seem right to me.
>
> FYI Envoy does the same, send 400 and terminate the connection.
>
>
> Ok, so it behaves the same as HA Proxy.
>
>
> >
> > Just in case you're worried about the additional logic on the
> client
> > side - it's super small. Really, only 13 lines including
> brackets ;)
> >
>
https://github.com/infinispan/infinispan/pull/6133/files#diff-684a10c939f...
>
> I am not worried about any logic in the client code, I am worried
> about
> logic between chair and keyboard.
>
>
> 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.
>
> 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?".
See the top, please, and my apologies if you took the above as an
offense. TBH I haven't even checked the implementation, and I am far
from judging it.
All good Radim, no worries! And thanks for all the stuff you're doing!
Radim
>
> R.
>
> >
> > Radim
> >
> > On 12/10/2018 03:27 PM, Sebastian Laskawiec wrote:
> > > Hey guys,
> > >
> > > During Infinispan F2F, I had a short discussion with
> Tristan on
> > Single
> > > Port client-side implementation. Back then, we agreed that
the
> > client
> > > should always send a Hotrod Ping request and if won't get any
> > response
> > > (or get some HTTP content back), it will try to upgrade to
the
> > Hotrod
> > > protocol using Single Port.
> > >
> > > I've been playing with the implementation for a while, and
> > > implementing it this way seems a bit "inconvenient" to
me.
> The Ping
> > > Operation uses 60s timeout, which seems to be a good fit as a
> > default.
> > > Unfortunately, for the Single Port functionality, this means
> > we'd need
> > > to wait 60s until we try to send HTTP request and do an
> upgrade.
> > Also,
> > > another problematic part is in Netty's HTTP handlers
> > > (HttpObjectDecoder, HttpServerCodec and
> ByteToMessageDecoder). When
> > > those classes fail to decode a message (REST expects HTTP
> rather
> > than
> > > a stream of bytes specific to Hotrod protocol), they just
> ignore it
> > > and keep the channel in active state (which also makes
> sense for
> > > HTTP/1.1 and HTTP/2).
> > >
> > > At this point, my intuition tells, that this doesn't look
> right and
> > > seems to be a over-complicated. The whole HTTP upgrade idea
> > seems to
> > > work the other way around, use HTTP as a fallback and then
> > upgrade to
> > > other protocols. Forcing it to work a bit differently
> requires some
> > > more effort.
> > >
> > > What if we preserved the Single Port setting in the client
> > > configuration but implemented it as an enum with the
following
> > values
> > > - true/false/auto. In automatic mode, the client would
> check if the
> > > server port is set to 8\d{1,3} (this covers 80, 8080,
> 8081, 8443
> > and
> > > friends). If that is true, we'd try to follow HTTP Upgrade
> > procedure.
> > > This looks very simple and I think this might actually
> work. Please
> > > note, that we need the single port setting in the client
> > configuration
> > > to cover some corner cases like the Single Port exposed on
> > different
> > > port (like 4444) or Hot Rod exposed on port that starts
> with 8.
> > >
> > > What do you think about such simplification?
> > >
> > > Thanks,
> > > Sebastian
> > >
> > >
> > >
> > >
> > > _______________________________________________
> > > infinispan-dev mailing list
> > > infinispan-dev(a)lists.jboss.org
> <mailto:infinispan-dev@lists.jboss.org>
> > <mailto:infinispan-dev@lists.jboss.org
> <mailto:infinispan-dev@lists.jboss.org>>
> > > https://lists.jboss.org/mailman/listinfo/infinispan-dev
> >
> >
> > --
> > Radim Vansa <rvansa(a)redhat.com <mailto:rvansa@redhat.com>
> <mailto:rvansa@redhat.com <mailto:rvansa@redhat.com>>>
> > JBoss Performance Team
> >
> > _______________________________________________
> > infinispan-dev mailing list
> > infinispan-dev(a)lists.jboss.org
> <mailto:infinispan-dev@lists.jboss.org>
> <mailto:infinispan-dev@lists.jboss.org
> <mailto:infinispan-dev@lists.jboss.org>>
> > https://lists.jboss.org/mailman/listinfo/infinispan-dev
> >
> >
> >
> > _______________________________________________
> > infinispan-dev mailing list
> > infinispan-dev(a)lists.jboss.org
> <mailto:infinispan-dev@lists.jboss.org>
> > https://lists.jboss.org/mailman/listinfo/infinispan-dev
>
>
> --
> Radim Vansa <rvansa(a)redhat.com <mailto:rvansa@redhat.com>>
> JBoss Performance Team
>
> _______________________________________________
> infinispan-dev mailing list
> infinispan-dev(a)lists.jboss.org <mailto:
infinispan-dev(a)lists.jboss.org>
> https://lists.jboss.org/mailman/listinfo/infinispan-dev
>
>
>
> _______________________________________________
> infinispan-dev mailing list
> infinispan-dev(a)lists.jboss.org
> https://lists.jboss.org/mailman/listinfo/infinispan-dev
--
Radim Vansa <rvansa(a)redhat.com>
JBoss Performance Team
_______________________________________________
infinispan-dev mailing list
infinispan-dev(a)lists.jboss.org
https://lists.jboss.org/mailman/listinfo/infinispan-dev
7:26 a.m.
On 12/14/2018 11:49 AM, Sebastian Laskawiec wrote:
On Thu, Dec 13, 2018 at 4:42 PM Radim Vansa <rvansa(a)redhat.com
<mailto:rvansa@redhat.com>> wrote:
Hey Sebastian,
I am sorry that you took my comment about errors between chair and
keyboard completely wrong: it was definitely not pointed at you!
What I
meant is that users will often misconfigure (I've been there many
times), and their experience will suffer. Therefore I am not
concerned
about any 'smart' logic we write (regardless of number of lines of
code), but about any complexity required from, or offered to the
users.
No problem, Radim. I was a little but surprised by that "left jab"
coming from nowhere ;) Emails are such a bad media of communication
when it comes to situations like this. In normal F2F communication
that would never happen. I'm also sorry for all the confusion...
This actually made me thinking, maybe it's the right time to do a
small 15 min meeting to discuss this F2F? @Tristan, @Ryan, @Galder -
would you also be interested? I'll prepare some slides to show you the
main pain points.
Rest of the comments inline...
On 12/13/2018 02:45 PM, Sebastian Laskawiec wrote:
>
>
> On Thu, Dec 13, 2018 at 10:23 AM Radim Vansa <rvansa(a)redhat.com
<mailto:rvansa@redhat.com>
> <mailto:rvansa@redhat.com <mailto:rvansa@redhat.com>>> wrote:
>
> On 12/12/2018 03:30 PM, Sebastian Laskawiec wrote:
> > On Tue, Dec 11, 2018 at 11:13 AM Radim Vansa
<rvansa(a)redhat.com <mailto:rvansa@redhat.com>
> <mailto:rvansa@redhat.com <mailto:rvansa@redhat.com>>
> > <mailto:rvansa@redhat.com <mailto:rvansa@redhat.com>
<mailto:rvansa@redhat.com <mailto:rvansa@redhat.com>>>> wrote:
> >
> > I dislike having any logic based on the port number in
some
> range;
> > it's
> > not common that behaviour would change if you set port
to 9xxx
> > instead
> > of 8xxx.
> >
> >
> > That's not a problem with my approach, since you can always
> manually
> > turn the setting off or on. Here's how you do it:
> > ConfigurationBuilder cb = ...
> > cb.singlePort(SinglePortMode.ENABLED); // other options:
> DISABLED and AUTO
>
> Adding config options is just a way to avoid solving problems :)
> Remember the famous quote: "Less knobs!"
>
>
> It depends on the problem in my opinion. I always preferred to have
> more configuration options with good defaults.
>
>
> >
> >
> > Is there an (up-to-date) design doc?
> >
> >
> > No, this is just a proposal. I was hoping that you guys
like it and
> > then, with some thumbs up, I could update the design doc.
> >
> > Here's the most up-to-date version in case you were
looking for it:
> >
>
https://github.com/infinispan/infinispan-designs/blob/master/Single_port....
> >
> >
> > I don't fully follow, but if there's a problem in the HTTP
> > handlers you
> > can add a PING-detecting handler below...?
> >
> >
> > Thanks for the hint Radim!
> >
> > Inspired by your idea I went ahead and checked how
OpenShift Router
> > behaves. It turns out that it responds HTTP 400 if you
throw Hotrod
> > bytes at it and then drops the connection.
>
> I understand that the reason to have Hot Rod PING sent as
the first
> operation is to make sure that a new client that tries to
connect
> to old
> server won't confuse the server, is that correct? Or is there
> anything else?
>
>
> 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).
>
>
> I'll assume that a new server will handle both Hot Rod PING
and HTTP
> request correctly without any prelude, and old one will be
ok with
> Hot
> Rod PING only. I don't really understand the:
>
>
> 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.
Okay, then my assumption was incorrect. That means that single-port
server and non-single-port server are incompatible because neither
can
accept other's initial request :-/
It seems that Hot Rod and HTTP communication is easily
distinguishable
with first few bytes: could we enhance single-port to be able to
handle
Hot Rod PING immediately without the HTTP Upgrade to accommodate
older
clients? Then we could run single-port all the time and everyone wins.
Yes, that's absolutely doable, but it doesn't solve the problem,
because there might be some Reverse Proxies on the way (such as an OCP
Router, Nginx or anything else). Those proxies might mess up with our
upgrade flow (as you mentioned below, there can be buggy proxies on
the way). The main problem is that we don't know (we can assume but we
are not 100% certain), how the Reverse Proxy will behave if we throw
binary protocol at it.
Handling Hot Rod on single port in the first case would be there to
allow old clients connecting to new server. If you point an old client
to some proxy instead of Hot Rod server, you're out of luck anyway.
Also, if we always start with Single Port turned on, we'll add
additional round trip (send HTTP with upgrade request, wait for
response, then switch to Hotrod). This adds some delay and I'm not
sure we should do it. If we wanted to consider this, I would need to
get Tristan's "green stamp" on it. But I have serious doubts
We don't need extra roundtrip. Client sends HTTP Upgrade: Hot Rod,
server responds with 101 Switching protocols and immediately follows
with Hot Rod Ping response. The client will expect that. Any extra
parameters we currently send in PING could be added to the initial HTTP
Upgrade request as extra headers. We start every connection with PING
anyway so this would not add any latency.
Btw., I assume that we can't handle both TLS and non-TLS
connections on
same port either, is that correct? Usually webservers can't be
configured to handle both HTTP and HTTPS on the same port - is
there any
difference here?
The encrypted use case is soooo much easier. The protocol negotiation
happen during TLS handshake. Once it's finished and connection is
established (so after the handshake is done), you already know about
the communication protocol you will be using. If we wanted to support
only encrypted use cases, the implementation would in 10-20 lines of code.
Frankly, I expect people to be using unencrypted use cases when
developing features and encrypted use cases on production. That's my
gut feeling how this feature will be used.
I had rather the use case when you have a non-encrypted *and* encrypted
service exposed on the *same* port. Yes, ALPN can do a lot for you for free.
>
> 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.
>
> [1]
>
https://svn.tools.ietf.org/svn/wg/httpbis/specs/rfc7230.html#header.upgrade
>
>
> > implementing it this way seems a bit "inconvenient" to
me. The Ping
> > Operation uses 60s timeout, which seems to be a good fit as a
> default.
> > Unfortunately, for the Single Port functionality, this means
> we'd need
> > to wait 60s until we try to send HTTP request and do an
upgrade
>
> why would you wait for 60 seconds? If the other end is
Infinispan
> server
> (old or new), you just send HR PING and you're done, server will
> proceed
> correctly. If the other server is a router, you'll get a
response
> starting with 'HTTP': in Hot Rod protocol that would be
parsed as
> opCode
> 0x54 which is illegal response code (the id belongs to
> COUNTER_RESET_REQUEST). At this point you know that this
> connection is
> going to be closed, and can immediately start another one (is
> *this* the
> problem?) that will send a HTTP request with Upgrade header.
>
>
> 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.
>
> 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.
That's a valid point - the spec says that it SHOULD [1] respond
with 400
but it is not 100% (buggy proxies are to be expected on the
internet).
We should probably correct the REST server...
Thanks for double checking. Yes, I believe we should. Let me test it a
little bit more and create proper JIRAs.
[1] https://tools.ietf.org/html/rfc7230#section-3.1.1
>
> 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.
I am starting to think that we really should start the communication
with HTTP message, and this shouldn't be a separate config property
(single-port) but based on Hot Rod version.
I also through about this but finally, I rejected the idea. The main
reasoning behind is this:
- We mess up with our default execution flow - hotrod client
connecting to a hotrod server. If there's any bug there we miss during
testing, there is a high chance we get a critical bug in return. And
since Infinispan is not my primary responsibility at the moment, I'm
worried that this will cause troubles and additional pressure to the team.
1) old client connecting to new server (without proxy) would work, since
single port would support Hot-Rod-prior-knowledge.
2) old client connecting through proxy is already screwed
3) new client connecting to old server must already set the protocol
version manually (no change)
4) new client connecting to new server would fly nicely (just a few more
bytes sent, but no latency), through proxy or not
- We perform additional round trip when initiating a connection (as I
explained before in this email).
Not necessary, explained.
- The default server configuration would be more complicated
(especially the endpoint configuration).
I would think that with removing the options it would actually become
*less* complicated. Since we're working on ServerNG, the configuration
could actually declare that on one place, something like:
<single-port-connector socket-binding="foo" security-realm="..."
cache-container="clustered" >
I admit this needs to list all the use cases but the above is as complex
as declaring Hot Rod-only connector and involves them all.
- The final argument (probably the least important) is that we
don't
support Websockets and Memcached in Single Port. So the configuration
wouldn't be symmetric. It's probably a nitpick, but I don't like it.
Websockets are dropped. Memcached... the request starts with 0x80 so
it's distinguishable to our 0xA0 and HTTP's 'H' = 0x54 - let's put it
under the umbrella, too.
Radim
>
> 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).
>
> @Tristan, @Ryan, I think you guys also have some experience in this
> matter. I would be very interested in hearing your opinion as well.
>
>
> > I also realized, there's one more moving bit - TCP Keepalive.
> Luckily,
> > we can control this setting over configuration in our
> standalone.xml.
> > However, it is perfectly legal what I've seen in Netty (do not
> respond
> > and keep the connection alive assuming that TCP Keepalive
is set
> to true).
> >
> > The more I think about this, the more I'm convinced that
the Single
> > Port support should be explicitly set in the client (or
inferred
> from
> > the configuration). I do not know how Nginx, Linkerd or Envoy
> behaves
> > in situation when they expect HTTP and get a stream of bytes.
> Relying
> > on this partially unknown behavior for doing our upgrade
procedure
> > doesn't seem right to me.
>
> FYI Envoy does the same, send 400 and terminate the connection.
>
>
> Ok, so it behaves the same as HA Proxy.
>
>
> >
> > Just in case you're worried about the additional logic on the
> client
> > side - it's super small. Really, only 13 lines including
> brackets ;)
> >
>
https://github.com/infinispan/infinispan/pull/6133/files#diff-684a10c939f...
>
> I am not worried about any logic in the client code, I am
worried
> about
> logic between chair and keyboard.
>
>
> 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.
>
> 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?".
See the top, please, and my apologies if you took the above as an
offense. TBH I haven't even checked the implementation, and I am far
from judging it.
All good Radim, no worries! And thanks for all the stuff you're doing!
Radim
>
> R.
>
> >
> > Radim
> >
> > On 12/10/2018 03:27 PM, Sebastian Laskawiec wrote:
> > > Hey guys,
> > >
> > > During Infinispan F2F, I had a short discussion with
> Tristan on
> > Single
> > > Port client-side implementation. Back then, we
agreed that the
> > client
> > > should always send a Hotrod Ping request and if
won't get any
> > response
> > > (or get some HTTP content back), it will try to
upgrade to the
> > Hotrod
> > > protocol using Single Port.
> > >
> > > I've been playing with the implementation for a
while, and
> > > implementing it this way seems a bit "inconvenient"
to me.
> The Ping
> > > Operation uses 60s timeout, which seems to be a good
fit as a
> > default.
> > > Unfortunately, for the Single Port functionality,
this means
> > we'd need
> > > to wait 60s until we try to send HTTP request and do an
> upgrade.
> > Also,
> > > another problematic part is in Netty's HTTP handlers
> > > (HttpObjectDecoder, HttpServerCodec and
> ByteToMessageDecoder). When
> > > those classes fail to decode a message (REST expects
HTTP
> rather
> > than
> > > a stream of bytes specific to Hotrod protocol), they
just
> ignore it
> > > and keep the channel in active state (which also makes
> sense for
> > > HTTP/1.1 and HTTP/2).
> > >
> > > At this point, my intuition tells, that this doesn't
look
> right and
> > > seems to be a over-complicated. The whole HTTP
upgrade idea
> > seems to
> > > work the other way around, use HTTP as a fallback
and then
> > upgrade to
> > > other protocols. Forcing it to work a bit differently
> requires some
> > > more effort.
> > >
> > > What if we preserved the Single Port setting in the
client
> > > configuration but implemented it as an enum with the
following
> > values
> > > - true/false/auto. In automatic mode, the client would
> check if the
> > > server port is set to 8\d{1,3} (this covers 80, 8080,
> 8081, 8443
> > and
> > > friends). If that is true, we'd try to follow HTTP
Upgrade
> > procedure.
> > > This looks very simple and I think this might actually
> work. Please
> > > note, that we need the single port setting in the client
> > configuration
> > > to cover some corner cases like the Single Port
exposed on
> > different
> > > port (like 4444) or Hot Rod exposed on port that starts
> with 8.
> > >
> > > What do you think about such simplification?
> > >
> > > Thanks,
> > > Sebastian
> > >
> > >
> > >
> > >
> > > _______________________________________________
> > > infinispan-dev mailing list
> > > infinispan-dev(a)lists.jboss.org
<mailto:infinispan-dev@lists.jboss.org>
> <mailto:infinispan-dev@lists.jboss.org
<mailto:infinispan-dev@lists.jboss.org>>
> > <mailto:infinispan-dev@lists.jboss.org
<mailto:infinispan-dev@lists.jboss.org>
> <mailto:infinispan-dev@lists.jboss.org
<mailto:infinispan-dev@lists.jboss.org>>>
> > > https://lists.jboss.org/mailman/listinfo/infinispan-dev
> >
> >
> > --
> > Radim Vansa <rvansa(a)redhat.com
<mailto:rvansa@redhat.com> <mailto:rvansa@redhat.com
<mailto:rvansa@redhat.com>>
> <mailto:rvansa@redhat.com <mailto:rvansa@redhat.com>
<mailto:rvansa@redhat.com <mailto:rvansa@redhat.com>>>>
> > JBoss Performance Team
> >
> > _______________________________________________
> > infinispan-dev mailing list
> > infinispan-dev(a)lists.jboss.org
<mailto:infinispan-dev@lists.jboss.org>
> <mailto:infinispan-dev@lists.jboss.org
<mailto:infinispan-dev@lists.jboss.org>>
> <mailto:infinispan-dev@lists.jboss.org
<mailto:infinispan-dev@lists.jboss.org>
> <mailto:infinispan-dev@lists.jboss.org
<mailto:infinispan-dev@lists.jboss.org>>>
> > https://lists.jboss.org/mailman/listinfo/infinispan-dev
> >
> >
> >
> > _______________________________________________
> > infinispan-dev mailing list
> > infinispan-dev(a)lists.jboss.org
<mailto:infinispan-dev@lists.jboss.org>
> <mailto:infinispan-dev@lists.jboss.org
<mailto:infinispan-dev@lists.jboss.org>>
> > https://lists.jboss.org/mailman/listinfo/infinispan-dev
>
>
> --
> Radim Vansa <rvansa(a)redhat.com <mailto:rvansa@redhat.com>
<mailto:rvansa@redhat.com <mailto:rvansa@redhat.com>>>
> JBoss Performance Team
>
> _______________________________________________
> infinispan-dev mailing list
> infinispan-dev(a)lists.jboss.org
<mailto:infinispan-dev@lists.jboss.org>
<mailto:infinispan-dev@lists.jboss.org
<mailto:infinispan-dev@lists.jboss.org>>
> https://lists.jboss.org/mailman/listinfo/infinispan-dev
>
>
>
> _______________________________________________
> infinispan-dev mailing list
> infinispan-dev(a)lists.jboss.org
<mailto:infinispan-dev@lists.jboss.org>
> https://lists.jboss.org/mailman/listinfo/infinispan-dev
--
Radim Vansa <rvansa(a)redhat.com <mailto:rvansa@redhat.com>>
JBoss Performance Team
_______________________________________________
infinispan-dev mailing list
infinispan-dev(a)lists.jboss.org <mailto:infinispan-dev@lists.jboss.org>
https://lists.jboss.org/mailman/listinfo/infinispan-dev
_______________________________________________
infinispan-dev mailing list
infinispan-dev(a)lists.jboss.org
https://lists.jboss.org/mailman/listinfo/infinispan-dev
--
Radim Vansa <rvansa(a)redhat.com>
JBoss Performance Team
1983
days inactive
1987
days old
infinispan-dev@lists.jboss.org
8 comments
2 participants
participants (2)
-
Radim Vansa -
Sebastian Laskawiec