8000 Expose HTTP Message sizing configuration in the Consul namer by dadjeibaah · Pull Request #2287 · linkerd/linkerd · GitHub
[go: up one dir, main page]
More Web Proxy on the site http://driver.im/
Skip to content

Expose HTTP Message sizing configuration in the Consul namer #2287

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 3 commits into from
Jun 24, 2019
Merged

Expose HTTP Message sizing configuration in the Consul namer #2287

merged 3 commits into from
Jun 24, 2019

Conversation

dadjeibaah
Copy link
Contributor
@dadjeibaah dadjeibaah commented Jun 24, 2019 •
edited
Loading

When Consul sends back discovery information in an HTTP request that exceeds 5MB, the Consul namer will throw a TooLongMessageException. This is because Finagle has a default limit on an HTTP message (5MB). This limit can be configured and the settings to configure an HTTP message are already exposed on Linkerd's HTTP router protocol

This PR mimics the router protocol's config to expose these settings for the Consul Namer's client.

To reproduce the issue:

  • Create a Consul service configuration file that contains a large number of unique service addresses.
  • Load the file in a local Consul agent and make sure that you place the service configuration file in the path specified in the -config-dir flag of the consul command
consul agent -dev -advertise 127.0.0.1 -config-dir /etc/consul.d
  • Run Namerd and point this config file by default points to a locally running consul agent.
namers:
- kind: io.l5d.consul
  useHealthCheck: true
admin:
  port: 9991
storage:
  kind: io.l5d.inMemory
  namespaces:
    default: |
      /svc => /#/io.l5d.consul/dc1;
interfaces:
- kind: io.l5d.thriftNameInterpreter
- kind: io.l5d.httpController
- kind: io.l5d.mesh
  • In Namerd's dtab playground, resolve the name /svc/cat. Before this PR you would see Namerd
    throw the TooLongMessageException.

After this PR run the same test mentioned above but this time with this config for Namerd.

namers:
- kind: io.l5d.consul
  useHealthCheck: true
  maxResponseKB: 1000000  # Sets the max response the namer can receive to approx. 1GB
admin:
  port: 9991
storage:
  kind: io.l5d.inMemory
  namespaces:
    default: |
      /svc => /#/io.l5d.consul/dc1;
interfaces:
- kind: io.l5d.thriftNameInterpreter
- kind: io.l5d.httpController
- kind: io.l5d.mesh

The exception no longer appears in Namerd's logs.

Fixes #2286

Dennis Adjei-Baah added 2 commits June 24, 2019 09:27
enable streaming in consul client
c242e87 
Signed-off-by: Dennis Adjei-Baah <dennis@buoyant.io>
add http content configurations to consul client
14d1e84 
Signed-off-by: Dennis Adjei-Baah <dennis@buoyant.io>
fix tests
53c9661 
Signed-off-by: Dennis Adjei-Baah <dennis@buoyant.io>
adleong
adleong approved these changes Jun 24, 2019
View reviewed changes
Copy link
Member
@adleong adleong left a comment

Choose a reason for hiding this comment

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

⭐ ☕️ ⚛️ Nice work

@dadjeibaah dadjeibaah merged commit 8c2fb09 into master Jun 24, 2019
@dadjeibaah dadjeibaah deleted the dad/stream-http-consul-client branch June 24, 2019 21:58
cpretzer added a commit that referenced this pull request Jun 24, 2019
@cpretzer
Expose HTTP Message sizing configuration in the Consul namer (#2287)
279ea9a 
* add http content configurations to consul client

Signed-off-by: Dennis Adjei-Baah <dennis@buoyant.io>
(cherry picked from commit 8c2fb09)
Signed-off-by: cpretzer <cpretzer@kashyyyk.buoyant.int>
This was referenced Jun 25, 2019
Closed
Closed
@zaharidichev zaharidichev mentioned this pull request Jul 13, 2019
Merged
adleong pushed a commit that referenced this pull request Jul 17, 2019
@zaharidichev @adleong
Enable streaming in Consul HTTP client (#2295)
43a1130 
This work is continuation of #2287. In essence the change enables `streaming` for the consul http client and as a result of that makes handling large consul responses easier (i.e. not limited by max size config). The PR exposes the `fixedLengthStreamedAfterKB` which as described in the docs

> configures `fixedLengthStreamedAfter` limit, which effectively turns on streaming (think `withStreaming(true)`). The `fixedLengthStreamedAfter`, however, disables streaming for sufficiently small messages of known fixed length.

In order to reproduce the initial issue and validate the solution you can use the method described by @dadjeibaah in #2287. Note that if you want to see the initial issue you need to first run against [this](753fc17) version of the code.  After this PR is applied you can run the example again and convince yourself that exception is not thrown despite the fact that the message size configuration is not tweaked.

Fixes #2288 

Signed-off-by: Zahari Dichev <zaharidichev@gmail.com>
@cpretzer cpretzer mentioned this pull request Nov 26, 2020
Open
2 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@adleong adleong adleong approved these changes

@olix0r olix0r Awaiting requested review from olix0r olix0r is a code owner

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

TooLongMessageException in namerd when using consul with a large number of service replicas
2 participants
@dadjeibaah @adleong
0