Skip to content

Commit

Permalink
Upgrade thrift to have better client socket timeout
Browse files Browse the repository at this point in the history
This is a big breaking change. Starting from this commit, Baseplate.go
would require all its users using HEAD version of thrift compiler
(minimal required version is bcae3bb, but 4db7a0a is recommended),
until thrift 0.14 is released.

This change allows client calls to have a controlled, low overhead over
the deadline they set in the context object, in the cases of slow
servers.
  • Loading branch information
fishy committed Jul 7, 2020
1 parent 9b5f7bb commit 0dcb92f
Show file tree
Hide file tree
Showing 16 changed files with 410 additions and 328 deletions.
4 changes: 2 additions & 2 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ with `*-remote` directories removed.
They are excluded from the linter.
DO NOT EDIT.

They were generated with [thrift compiler 0.13.0][thrift-version] and
They were generated with [thrift compiler 4db7a0a][thrift-version] and
[`baseplate.thrift`][baseplate.thrift] using command under `internal/`:

```
Expand Down Expand Up @@ -68,4 +68,4 @@ bazel test //...:all

[godev]: https://pkg.go.dev/github.com/reddit/baseplate.go

[thrift-version]: https://github.com/apache/thrift/tree/v0.13.0
[thrift-version]: https://github.com/apache/thrift/tree/4db7a0af13ac9614e3e9758d42b2791040f4dc7e
8 changes: 4 additions & 4 deletions edgecontext/edgecontext.go
Original file line number Diff line number Diff line change
Expand Up @@ -161,15 +161,15 @@ func New(ctx context.Context, impl *Impl, args NewArgs) (*EdgeRequestContext, er
}, nil
}

// FromHeader returns a new EdgeRequestContext from the given header string using
// the given Impl.
func FromHeader(header string, impl *Impl) (*EdgeRequestContext, error) {
// FromHeader returns a new EdgeRequestContext from the given header string
// using the given Impl.
func FromHeader(ctx context.Context, header string, impl *Impl) (*EdgeRequestContext, error) {
if header == "" {
return nil, nil
}

request := baseplate.NewRequest()
if err := deserializerPool.ReadString(request, header); err != nil {
if err := deserializerPool.ReadString(ctx, request, header); err != nil {
return nil, err
}

Expand Down
12 changes: 6 additions & 6 deletions edgecontext/edgecontext_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -236,7 +236,7 @@ func TestFromHeader(t *testing.T) {
t.Run(
"no-header",
func(t *testing.T) {
e, err := edgecontext.FromHeader("", globalTestImpl)
e, err := edgecontext.FromHeader(context.Background(), "", globalTestImpl)
if err != nil {
t.Error(err)
}
Expand All @@ -250,7 +250,7 @@ func TestFromHeader(t *testing.T) {
"no-device-id-no-origin",
func(t *testing.T) {

e, err := edgecontext.FromHeader(headerWithNoAuthNoDevice, globalTestImpl)
e, err := edgecontext.FromHeader(context.Background(), headerWithNoAuthNoDevice, globalTestImpl)
if err != nil {
t.Fatal(err)
}
Expand Down Expand Up @@ -303,7 +303,7 @@ func TestFromHeader(t *testing.T) {
t.Run(
"no-auth",
func(t *testing.T) {
e, err := edgecontext.FromHeader(headerWithNoAuth, globalTestImpl)
e, err := edgecontext.FromHeader(context.Background(), headerWithNoAuth, globalTestImpl)
if err != nil {
t.Fatal(err)
}
Expand Down Expand Up @@ -402,7 +402,7 @@ func TestFromHeader(t *testing.T) {
t.Run(
"valid-auth",
func(t *testing.T) {
e, err := edgecontext.FromHeader(headerWithValidAuth, globalTestImpl)
e, err := edgecontext.FromHeader(context.Background(), headerWithValidAuth, globalTestImpl)
if err != nil {
t.Fatal(err)
}
Expand Down Expand Up @@ -519,7 +519,7 @@ func TestFromHeader(t *testing.T) {
t.Run(
"expired-auth",
func(t *testing.T) {
e, err := edgecontext.FromHeader(headerWithExpiredAuth, globalTestImpl)
e, err := edgecontext.FromHeader(context.Background(), headerWithExpiredAuth, globalTestImpl)
if err != nil {
t.Fatal(err)
}
Expand Down Expand Up @@ -625,7 +625,7 @@ func TestFromHeader(t *testing.T) {
t.Run(
"anon-auth",
func(t *testing.T) {
e, err := edgecontext.FromHeader(headerWithAnonAuth, globalTestImpl)
e, err := edgecontext.FromHeader(context.Background(), headerWithAnonAuth, globalTestImpl)
if err != nil {
t.Fatal(err)
}
Expand Down
8 changes: 4 additions & 4 deletions events/events_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -14,15 +14,15 @@ import (

type mockTStruct struct{}

func (mockTStruct) Read(_ thrift.TProtocol) error {
func (mockTStruct) Read(_ context.Context, _ thrift.TProtocol) error {
return nil
}

func (mockTStruct) Write(p thrift.TProtocol) error {
if err := p.WriteMessageBegin("mock", thrift.CALL, 0); err != nil {
func (mockTStruct) Write(ctx context.Context, p thrift.TProtocol) error {
if err := p.WriteMessageBegin(ctx, "mock", thrift.CALL, 0); err != nil {
return err
}
return p.WriteMessageEnd()
return p.WriteMessageEnd(ctx)
}

func TestV2Put(t *testing.T) {
Expand Down
4 changes: 2 additions & 2 deletions external.bzl
Original file line number Diff line number Diff line change
Expand Up @@ -34,8 +34,8 @@ def go_dependencies():
go_repository(
name = "com_github_apache_thrift",
importpath = "github.com/apache/thrift",
sum = "h1:hy6+UGGxgL+IxKzSgJBugF5C5pf5K4WsUB8xA7zV/XM=",
version = "v0.13.1-0.20200609200738-cfbb905034c9",
sum = "h1:NHSR+Cgd6sHVc5YcdhOch+PD+Ew9RXoKxYcXNh6hBpU=",
version = "v0.13.1-0.20200701185044-4db7a0af13ac",
)
go_repository(
name = "com_github_armon_consul_api",
Expand Down
2 changes: 1 addition & 1 deletion go.mod
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ require (
github.com/VividCortex/gohistogram v1.0.0 // indirect
github.com/alicebob/gopher-json v0.0.0-20180125190556-5a6b3ba71ee6 // indirect
github.com/alicebob/miniredis v2.5.0+incompatible
github.com/apache/thrift v0.13.1-0.20200609200738-cfbb905034c9
github.com/apache/thrift v0.13.1-0.20200701185044-4db7a0af13ac
github.com/avast/retry-go v2.6.0+incompatible
github.com/getsentry/sentry-go v0.6.0
github.com/go-kit/kit v0.9.0
Expand Down
4 changes: 2 additions & 2 deletions go.sum
Original file line number Diff line number Diff line change
Expand Up @@ -13,8 +13,8 @@ github.com/alicebob/gopher-json v0.0.0-20180125190556-5a6b3ba71ee6 h1:45bxf7AZMw
github.com/alicebob/gopher-json v0.0.0-20180125190556-5a6b3ba71ee6/go.mod h1:SGnFV6hVsYE877CKEZ6tDNTjaSXYUk6QqoIK6PrAtcc=
github.com/alicebob/miniredis v2.5.0+incompatible h1:yBHoLpsyjupjz3NL3MhKMVkR41j82Yjf3KFv7ApYzUI=
github.com/alicebob/miniredis v2.5.0+incompatible/go.mod h1:8HZjEj4yU0dwhYHky+DxYx+6BMjkBbe5ONFIF1MXffk=
github.com/apache/thrift v0.13.1-0.20200609200738-cfbb905034c9 h1:hy6+UGGxgL+IxKzSgJBugF5C5pf5K4WsUB8xA7zV/XM=
github.com/apache/thrift v0.13.1-0.20200609200738-cfbb905034c9/go.mod h1:cp2SuWMxlEZw2r+iP2GNCdIi4C1qmUzdZFSVb+bacwQ=
github.com/apache/thrift v0.13.1-0.20200701185044-4db7a0af13ac h1:NHSR+Cgd6sHVc5YcdhOch+PD+Ew9RXoKxYcXNh6hBpU=
github.com/apache/thrift v0.13.1-0.20200701185044-4db7a0af13ac/go.mod h1:cp2SuWMxlEZw2r+iP2GNCdIi4C1qmUzdZFSVb+bacwQ=
github.com/armon/consul-api v0.0.0-20180202201655-eb2c6b5be1b6/go.mod h1:grANhF5doyWs3UAsr3K4I6qtAmlQcZDesFNEHPZAzj8=
github.com/avast/retry-go v2.6.0+incompatible h1:FelcMrm7Bxacr1/RM8+/eqkDkmVN7tjlsy51dOzB3LI=
github.com/avast/retry-go v2.6.0+incompatible/go.mod h1:XtSnn+n/sHqQIpZ10K1qAevBhOOCWBLXXy3hyiqqBrY=
Expand Down
2 changes: 1 addition & 1 deletion httpbp/middlewares.go
Original file line number Diff line number Diff line change
Expand Up @@ -161,7 +161,7 @@ func InitializeEdgeContextFromTrustedRequest(
args.Logger.Log("Error while parsing EdgeRequestContext: " + err.Error())
return ctx
}
ec, err := edgecontext.FromHeader(string(header), args.EdgeContextImpl)
ec, err := edgecontext.FromHeader(ctx, string(header), args.EdgeContextImpl)
if err != nil {
args.Logger.Log("Error while parsing EdgeRequestContext: " + err.Error())
return ctx
Expand Down
3 changes: 1 addition & 2 deletions internal/gen-go/reddit/baseplate/GoUnusedProtection__.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

5 changes: 3 additions & 2 deletions internal/gen-go/reddit/baseplate/baseplate-consts.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Loading

0 comments on commit 0dcb92f

Please sign in to comment.