Skip to content

Conversation

Mythra
Copy link
Member

@Mythra Mythra commented Aug 9, 2019

Description:

switch Envoy::Buffer to Brian Pane's implementation (aka "new buffers") by default.

Risk Level: Medium

  • There's been decent testing on this, but it's still a pretty massive change.

Testing:

Ran through the following test plan locally:

1. Build envoy on this PR.
2. Spin up a server with no CLI Args: `./bazel-bin/source/exe/envoy-static -c /tmp/config.yaml` validate that: `[2019-08-09 10:11:52.900][6365][info][main] [source/server/server.cc:269] buffer implementation: new` gets logged
3. Ensure new buffers can be turned off by running: `./bazel-bin/source/exe/envoy-static --use-libevent-buffers 1 -c /tmp/config.yaml` validate that: `[2019-08-09 10:12:14.247][6431][info][main] [source/server/server.cc:269] buffer implementation: old (libevent)` got logged

There's also be some testers in actual environments see #7314 , and Envoy Meeting Minutes.

Docs Changes: None
Release Notes: Added a line about how it's the default, including how to turn it off.

fixes #7314

@lizan lizan requested a review from jmarantz August 12, 2019 03:17
@Mythra
Copy link
Member Author

Mythra commented Aug 12, 2019

@lizan ,

ince @jmarantz is OOO any chance you could review this as a senior maintainer? I'd like to get it out before the security releases tomorrow as well so I can pick these onto our local branch along with the security fixes, it has been approved as of the latest envoy meeting.

lizan
lizan previously approved these changes Aug 12, 2019
@lizan
Copy link
Member

lizan commented Aug 12, 2019

@securityinsanity sure, the code looks good to me, however I'm not merging this until the security release, to keep the upgrade process smooth.

@Mythra
Copy link
Member Author

Mythra commented Aug 12, 2019

👍 I agree there, our pipeline just checks for approved PRs before picking.

Thanks!

@Mythra
Copy link
Member Author

Mythra commented Aug 13, 2019

I believe we should be good to merge this now, since 1.11.1 is fully out?

@mattklein123
Copy link
Member

🤞 please merge master.

/wait

@Mythra
Copy link
Member Author

Mythra commented Aug 14, 2019

Can do here in a bit.

Cynthia Coan added 2 commits August 14, 2019 10:57
fixes envoyproxy#7314

this commit changes the default implementation of buffers to the
new implementation by default.

Signed-off-by: Cynthia Coan <ccoan@instructure.com>
Signed-off-by: Cynthia Coan <ccoan@instructure.com>
@Mythra
Copy link
Member Author

Mythra commented Aug 14, 2019

Master has been merged, successfully 👍 .

Copy link
Member

@mattklein123 mattklein123 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks!

@mattklein123 mattklein123 merged commit 3dd84d3 into envoyproxy:master Aug 14, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

buffer: Switch from the libevent impl of Envoy::Buffer to Brian Pane's implementation
4 participants