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

Respect SETTINGS_HEADER_TABLE_SIZE http2 setting #2045

Merged
merged 4 commits into from May 11, 2018

Conversation

Projects
None yet
4 participants
@lyuxuan
Contributor

lyuxuan commented May 2, 2018

fix #1928

lyuxuan added some commits Apr 18, 2018

@mumoshu

This comment has been minimized.

mumoshu commented May 10, 2018

@lyuxuan This worked like a charm! nginx-ingress-controller is now happy to load-balance grpc over h2c workloads to the grpc server w/ this change. Thanks a lot for the fix 👍

var svrTransport ServerTransport
var i int
for i := 0; i < 1000; i++ {

This comment has been minimized.

@MakMukhi

MakMukhi May 11, 2018

Contributor

This i is not same as the i above. := assigns a new local variable

This comment has been minimized.

@lyuxuan

lyuxuan May 11, 2018

Contributor

nice catch!

}
break
}
if i == 1000 {

This comment has been minimized.

@MakMukhi

MakMukhi May 11, 2018

Contributor

Same thing i is local to the for loop above.

@MakMukhi MakMukhi assigned lyuxuan and unassigned MakMukhi May 11, 2018

@lyuxuan lyuxuan merged commit 9a54b9a into grpc:master May 11, 2018

2 checks passed

cla/linuxfoundation lyuxuan authorized
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details

hyperledger-github pushed a commit to hyperledger/fabric that referenced this pull request Jul 3, 2018

[FAB-10885] Upgrade grpc-go
We should upgrade anyway, but
grpc/grpc-go#2045
prevents the use of proxies such as nginx
from working with Fabric nodes.

Change-Id: I3dc8af85ede23548f26c8fc38f984b94f6cc4e4b
Signed-off-by: Gari Singh <gari.r.singh@gmail.com>
@him0

This comment has been minimized.

him0 commented Sep 12, 2018

I cannot find this PR at release note , but found this changes in version 1.15.0(8dea3dc). Has this pull request already released?

var updateHeaderTblSize = func(e *hpack.Encoder, v uint32) {
e.SetMaxDynamicTableSizeLimit(v)
}

@lyuxuan

This comment has been minimized.

Contributor

lyuxuan commented Sep 12, 2018

@him0, this feature is released since version 1.13.0. You can look at the commit page here, and figure out the versions having this feature. We forgot to include it in the 1.13.0 release note, which is an unfortunate oversight. Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment