Skip to content

configuration: enable h2 ping by default#2270

Merged
Augustyniak merged 7 commits intomainfrom
enable-h2-ping-by-default
May 12, 2022
Merged

configuration: enable h2 ping by default#2270
Augustyniak merged 7 commits intomainfrom
enable-h2-ping-by-default

Conversation

@Augustyniak
Copy link
Copy Markdown
Contributor

Description: Enable h2 ping by default. Lyft currently has this functionality enabled to all of users who have Envoy Mobile running. It turned out to be effective when it comes to getting rid of stale connections.
Risk Level: Low, enables well tested functionality by default.
Testing: N/A
Docs Changes: Done
Release Notes: Done

Signed-off-by: Rafal Augustyniak raugustyniak@lyft.com

Signed-off-by: Rafal Augustyniak <raugustyniak@lyft.com>
Signed-off-by: Rafal Augustyniak <raugustyniak@lyft.com>
@Augustyniak Augustyniak force-pushed the enable-h2-ping-by-default branch from 0d5b979 to e8d464c Compare May 12, 2022 13:39
@Augustyniak Augustyniak requested review from goaway and jpsim May 12, 2022 14:00
Copy link
Copy Markdown
Contributor

@jpsim jpsim left a comment

Choose a reason for hiding this comment

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

Typos. We should also document how to disable now that it's not the default.

Thanks for adding release notes!

Augustyniak and others added 2 commits May 12, 2022 16:47
Co-authored-by: JP Simard <jp@jpsim.com>
Signed-off-by: Rafal Augustyniak <raugustyniak@lyft.com>
Co-authored-by: JP Simard <jp@jpsim.com>
Signed-off-by: Rafal Augustyniak <raugustyniak@lyft.com>
@Augustyniak Augustyniak force-pushed the enable-h2-ping-by-default branch from 25162bb to ff2f26f Compare May 12, 2022 20:47
Signed-off-by: Rafal Augustyniak <raugustyniak@lyft.com>
Signed-off-by: Rafal Augustyniak <raugustyniak@lyft.com>
@Augustyniak Augustyniak requested a review from jpsim May 12, 2022 20:52
@Augustyniak
Copy link
Copy Markdown
Contributor Author

Augustyniak commented May 12, 2022

Typos. We should also document how to disable now that it's not the default.

Great point. Updated the doc. Not sure whether it's OK now - let me know. This API for enabling/disabling H2 ping should be eventually updated to access true/false as opposed to this time interval I think. That's a separate thing tho.

Signed-off-by: Rafal Augustyniak <raugustyniak@lyft.com>
Copy link
Copy Markdown
Contributor

@jpsim jpsim left a comment

Choose a reason for hiding this comment

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

This is fine as it's documenting existing behavior but we should make this field an optional to make it more obvious how to disable it.

@Augustyniak Augustyniak enabled auto-merge (squash) May 12, 2022 20:59
@Augustyniak Augustyniak merged commit 614c098 into main May 12, 2022
@Augustyniak Augustyniak deleted the enable-h2-ping-by-default branch May 12, 2022 21:06
jpsim added a commit that referenced this pull request May 17, 2022
* origin/main: (97 commits)
  docs: update python packages to work with Python 3.10 (#2286)
  test: adding a cancel test, cleaning up copy-paste code (#2283)
  envoy: update to d88f31b (#2279)
  api: disallow setting 'host' header directly (#2275)
  android: add support for registering a platform KV store (#2134)
  Bump Lyft Support Rotation (#2278)
  tools: Enable the VSCode completion db to use bazelisk if available (#2277)
  Release v0.4.6.20220513-4
  Fix android_release_deploy
  Release v0.4.6.20220513-3
  Release v0.4.6.20220513-2
  net: enable happy eyeballs by default (#2272)
  git: avoid merge conflicts when adding changelog entries (#2273)
  docs: fix sphinx reference mismatch warning (#2274)
  tests: add -Xcheck:jni to kotlin integration tests by default (#2269)
  configuration: enable h2 ping by default (#2270)
  Add version history entries for user-facing changes (#2271)
  configuration: filter unroutable addresses on Android by default (#2267)
  Integrate rules_xcodeproj (#2263)
  Add assert when failing to get_env (#2253)
  ...

Signed-off-by: JP Simard <jp@jpsim.com>
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.

2 participants