New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Blog post about N4 support #26
Conversation
## How to enable? | ||
|
||
We encourage Finagle users to try out new Netty 4 transports for their protocols hence jump on a | ||
fast tract to upcoming changes around resiliency (think of HTTP/2), performance (think of Netty 4 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sorry: should be track.
|
||
## Where to report problems? | ||
|
||
Please, reach out directly to our [Gitter room][gitter] or fail a [Github issue][github] if anything |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sorry: should be file
|
||
It's been quite a while since [Netty 4 migration in Finagle][netty4-blog-post] was initially | ||
announced. We've travelled a long way and are happy to announce that there is now (as per | ||
[Finagle 6.42][6.42]) an experimental support for Netty 4 transports in most of the protocols |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
i think this is underselling the current state. we are running n4 (thrift)mux, memcached, mysql, thrift, and kestrel for all services. i think we should publicize that.
http/1 and http/2 are still experimental in my mind.
perhaps we could state that while we have not yet defaulted them to on for those other protocols, there are no known issues.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
in this vein, I'd drop the 'experimental' from the title
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks Kevin and Daniel! I dropped the "experimental" part.
|
||
## How to enable? | ||
|
||
We encourage Finagle users to try out new Netty 4 transports for their protocols hence jump on a |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
hence seems out of place here. also s/tract/track/
|
||
We encourage Finagle users to try out new Netty 4 transports for their protocols hence jump on a | ||
fast tract to upcoming changes around resiliency (think of HTTP/2), performance (think of Netty 4 | ||
pooling), and security (think of service-to-service auth). |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
s/auth/authentication/
fast tract to upcoming changes around resiliency (think of HTTP/2), performance (think of Netty 4 | ||
pooling), and security (think of service-to-service auth). | ||
|
||
To switch the transport over to Netty 4 supply the following command line flag for a JVM process |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
omit "JVM process running Finagle" as I think that goes without saying
Where `$protocol` is one of the following: `mux` (use this for ThriftMux), `thrift`, `mysql`, | ||
`memcached`. | ||
|
||
This command line flag overrides [a feature toggle][toggles] that's evaluated at application |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
s/that's/that is/
HTTP/1.1 on Netty 4 is still work in progress. There are known limitations for HTTP clients, but | ||
we've been successfully running finagle-http servers with Netty 4 in production for several weeks. | ||
|
||
That said we encourage everyone to enable Netty 4 transport on HTTP servers running Finagle as show |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
s/everyone/you/
s/show/shown/
|
||
## What's next? | ||
|
||
We're quite optimistic about enabling Netty 4 by default in the next Finagle release for all |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
afaik we have not set a concrete date for this. how about we give ourselves more room and say in the next 3 months barring unforeseen issues.
Getting a production-ready HTTP/1.1 implementation based on Netty 4 in Finagle is one of our top | ||
priorities right now. Even though we're not there yet, we feel very proud of the work we've done | ||
and the progress we've made. It took us several years and dozens of engineers to be able to start | ||
serving Netty 4 traffic in production. We understand this kind of velocity isn't typical for most of |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
i don't think this is necessary.
|
||
## Where to report problems? | ||
|
||
Please, reach out directly to our [Gitter room][gitter] or fail a [Github issue][github] if anything |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
s/fail/file/
i think in general we'd prefer github issues over gitter. consider omitting gitter?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yup. Let's drop Gitter.
|
||
## What about HTTP/1.1? | ||
|
||
HTTP/1.1 on Netty 4 is still work in progress. There are known limitations for HTTP clients, but |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
still work => still a work
HTTP/1.1 on Netty 4 is still work in progress. There are known limitations for HTTP clients, but | ||
we've been successfully running finagle-http servers with Netty 4 in production for several weeks. | ||
|
||
That said we encourage everyone to enable Netty 4 transport on HTTP servers running Finagle as show |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That said => That being the case (or that said),
|
||
It's been quite a while since [Netty 4 migration in Finagle][netty4-blog-post] was initially | ||
announced. We've travelled a long way and are happy to announce that there is now (as per | ||
[Finagle 6.42][6.42]) an experimental support for Netty 4 transports in most of the protocols |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
remove an in "an experimental"
|
||
It's been quite a while since [Netty 4 migration in Finagle][netty4-blog-post] was initially | ||
announced. We've travelled a long way and are happy to announce that there is now (as per | ||
[Finagle 6.42][6.42]) an experimental support for Netty 4 transports in most of the protocols |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
extra space before protocols
|
||
## How to enable? | ||
|
||
We encourage Finagle users to try out new Netty 4 transports for their protocols hence jump on a |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
try out the new
We're quite optimistic about enabling Netty 4 by default in the next Finagle release for all | ||
protocols besides HTTP/1.1. | ||
|
||
Getting a production-ready HTTP/1.1 implementation based on Netty 4 in Finagle is one of our top |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What issue are you thinking of that prompts this? I think it undersells the state of n4/http 1.1, it's been in prod w/ macaw-meta for over 2 quarters, a month w/ MS and is about to go into tfe.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sorry I should've been more specific here. I meant clients. MS and MM are both running HTTP servers, not clients.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Only small grammar suggestions from me. I've never been good at english, so please ignore if they don't make sense.
Lgtm after addressing the other suggestions. 👍
[Finagle 6.42][6.42]) an experimental support for Netty 4 transports in most of the protocols | ||
(Thrift, ThriftMux, Memcached, MySQL) and production-ready support for Redis. | ||
|
||
## How to enable? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Add Netty 4
to the end?
This command line flag overrides [a feature toggle][toggles] that's evaluated at application | ||
start up and is *global for all clients/servers running on the same JVM instance*. | ||
|
||
Note that Netty 4 is already enabled by default in `finagle-redis` so no need for extra CLI flag |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
'so there is no need for an extra CLI flag.'
736a1e3
to
e964518
Compare
@kevinoliver, @dschobel, @bryce-anderson, @taylorleese Please take a look again - I hope I didn't forget anything. And thanks a lot for these thoughtful reviews! |
|
||
Note that Netty 4 is already enabled by default in `finagle-redis` so no need for an extra CLI flag. | ||
|
||
## What about HTTP/1.1? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
should we add a section for the state of H2 since it's one of the biggest features of the upgrade?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks! I added a new section about h2.
we've been successfully running `finagle-http` servers with Netty 4 in production for several weeks. | ||
|
||
That said we encourage you to enable Netty 4 transport on HTTP servers running Finagle as shown | ||
above. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
can you also add a note that http/2 should be ready for beta use in the next couple months?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm going to add a new section about h2 as per @dschobel's request.
|
||
It's been quite a while since [Netty 4 migration in Finagle][netty4-blog-post] was initially | ||
announced. We've travelled a long way and are happy to announce that there is now (as per | ||
[Finagle 6.42][6.42]) a support for Netty 4 transports in most of the protocols: Thrift, ThriftMux, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
s/a support/support
--- | ||
|
||
It's been quite a while since [Netty 4 migration in Finagle][netty4-blog-post] was initially | ||
announced. We've travelled a long way and are happy to announce that there is now (as per |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
s/there is now/we now
It's been quite a while since [Netty 4 migration in Finagle][netty4-blog-post] was initially | ||
announced. We've travelled a long way and are happy to announce that there is now (as per | ||
[Finagle 6.42][6.42]) a support for Netty 4 transports in most of the protocols: Thrift, ThriftMux, | ||
Memcached, MySQL, Kestrel, and Redis. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
how about we add a note that http/1.1 and http/2 are in the pipeline too?
## How to enable Netty 4? | ||
|
||
While we have not yet defaulted to Netty 4, we've been running it in production for several months | ||
and gained enough confidence to publicize the availability of the alternative transports in Finagle. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
s/and gained/and have gained
and gained enough confidence to publicize the availability of the alternative transports in Finagle. | ||
|
||
We encourage Finagle users to try out the new Netty 4 transports for their protocols and jump on a | ||
fast track to upcoming changes around resiliency (think of HTTP/2) and performance (think of Netty 4 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
instead of saying Netty 4 pooling, how about describing a generic improvement instead? that way people who don't work closely with netty know what we're talking about.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sure. How about "think of Netty 4's reduced allocation profile and simplified threading model"?
## What's next? | ||
|
||
We're quite optimistic about enabling Netty 4 by default in the next couple of months. Even though | ||
we're not there yet, we feel very proud of the work we've done and the progress we've made.It took |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
s/made.It/made. It
|
||
We're quite optimistic about enabling Netty 4 by default in the next couple of months. Even though | ||
we're not there yet, we feel very proud of the work we've done and the progress we've made.It took | ||
us several years and dozens of engineers to be able to start serving Netty 4 traffic in production. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
dozens? I don't think it's quite that many. maybe more concretely say, "several years of engineering effort"
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Love it! Thanks!
tags: Finagle, Netty4 | ||
--- | ||
|
||
It's been quite a while since [Netty 4 migration in Finagle][netty4-blog-post] was initially |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
since => since the
While we have not yet defaulted to Netty 4, we've been running it in production for several months | ||
and gained enough confidence to publicize the availability of the alternative transports in Finagle. | ||
|
||
We encourage Finagle users to try out the new Netty 4 transports for their protocols and jump on a |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
jump on a fast track => jump on the fast track
|
||
## What about HTTP/1.1? | ||
|
||
HTTP/1.1 on Netty 4 is still work in progress. There are known limitations for HTTP clients, but |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
still work => still a work
HTTP/1.1 on Netty 4 is still work in progress. There are known limitations for HTTP clients, but | ||
we've been successfully running `finagle-http` servers with Netty 4 in production for several weeks. | ||
|
||
That said we encourage you to enable Netty 4 transport on HTTP servers running Finagle as shown |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That said => That said,
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
enable Netty 4 transport => enable the Netty 4 transport
e964518
to
3739cde
Compare
Thanks again @mosesn, @taylorleese, @dschobel! I uploaded a new version - hope it looks better this time! |
3739cde
to
84f5b53
Compare
fast track to upcoming changes around resiliency (think of HTTP/2) and performance (think of reduced | ||
allocation profile and better threading model in Netty 4). | ||
|
||
To switch the transport over to Netty 4 supply the following command line flag:. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nit: stray .
at the end of the line
|
||
HTTP/2 (i.e., `finagle-http2`) should be considered beta as there are known issues with the ALPN | ||
support. We're hoping to roll out a feature-complete HTTP/2 implementation in the next couple of | ||
months. In the meantime, HTTP/2 support could be experimentally enabled on any Finagle HTTP client |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
s/could/can/
## What about HTTP/1.1? | ||
|
||
HTTP/1.1 on Netty 4 is still a work in progress. There are known limitations for HTTP clients, but | ||
we've been successfully running `finagle-http` servers with Netty 4 in production for several weeks. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
we should note that TwitterServer's admin http server has been running it for months.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
just a few nits. lgtm!
84f5b53
to
61770ab
Compare
Thanks @kevinoliver! Just uploaded a new version - let me know if anything doesn't look right. |
--- | ||
|
||
It's been quite a while since the [Netty 4 migration in Finagle][netty4-blog-post] was initially | ||
announced. We've travelled a long way and are happy to announce that there is now (as per |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
as per => as of?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think "as of" is better. Thanks!
Finagle. | ||
|
||
We encourage Finagle users to try out the new Netty 4 transports for their protocols and jump on the | ||
fast track to upcoming changes around resiliency (think of HTTP/2) and performance (think of reduced |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
a reduced allocation profile
`memcached`, `kestrel`. | ||
|
||
This command line flag overrides [a feature toggle][toggles] that is evaluated at application | ||
start up and is *global for all clients/servers running on the same JVM instance*. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
start-up
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thinking about this more it should probably be just one word: "startup".
we've been successfully running `finagle-http` servers with Netty 4 in production for several weeks | ||
and on [TwitterServer][ts]'s admin interface for several months. | ||
|
||
That said, we encourage you to enable the Netty 4 transport on HTTP servers running Finagle as shown |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I would remove "that said", usually what follows is a caviat :)
|
||
## What about HTTP/2? | ||
|
||
HTTP/2 (i.e., `finagle-http2`) should be considered beta as there are known issues with the ALPN |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ffti: in beta?
61770ab
to
847b643
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM, but I think it would be nice to add a note about http/1.1 and http/2 in the first paragraph.
It's been quite a while since the [Netty 4 migration in Finagle][netty4-blog-post] was initially | ||
announced. We've travelled a long way and are happy to announce that there is now (as of | ||
[Finagle 6.42][6.42]) support for Netty 4 transports in most of the protocols: Thrift, ThriftMux, | ||
Memcached, MySQL, Kestrel, and Redis. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
how about adding a note here about http/1.1 and http/2 coming soon?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sure! Let's add it.
847b643
to
05059ba
Compare
Uploaded a new diff. Thanks @mosesn and @jcrossley for the review! |
No description provided.