🔍 Deep Dive: bump google.golang.org/grpc v1.20.1 by AkihiroSuda · Pull Request #936 · moby/buildkit · GitHub

AkihiroSuda · 2019-04-13T12:22:54Z

Signed-off-by: Akihiro Suda suda.akihiro@lab.ntt.co.jp

AkihiroSuda · 2019-04-13T12:42:41Z

=== RUN   TestSimpleMarshal
--- PASS: TestSimpleMarshal (0.00s)
PASS
ok  	github.com/moby/buildkit/cache/remotecache/v1	0.009s
?   	github.com/moby/buildkit/cache/util	[no test files]
No output has been received in the last 10m0s, this potentially indicates a stalled build or something wrong with the build itself.
Check the details on how to adjust your build configuration on: https://docs.travis-ci.com/user/common-build-problems/#Build-times-out-because-no-output-was-received
The build has been terminated

AkihiroSuda · 2019-04-13T21:41:49Z

for f in oci oci-rootless containerd containerd-1.0; do TESTFLAGS="--run //worker=$f\$ -v" ./hack/test integration; done succeeds but ./hack/test integration hangs.

Seems something is not working when multiple workers are simultaneously being tested.

AkihiroSuda · 2019-04-14T09:15:40Z

Even with multiple workers, the test suite passes after commenting out some testcases in client_test.go.

I assume we are hitting some quota (FD?) when we have lots of testcases/workers running.

AkihiroSuda · 2019-04-14T11:19:47Z

0cc96a1 once succeeded but failed after restarting CI. Locally always fails.

thaJeztah · 2019-04-14T11:23:54Z

@tonistiigi ptal

AkihiroSuda · 2019-04-14T11:46:02Z

TESTPKGS="./client" TESTFLAGS="-race -v" ./hack/test integration reported some races

==================
WARNING: DATA RACE
Write at 0x00c0000d5a50 by goroutine 6:
  google.golang.org/grpc.(*clientStream).CloseSend()
      /src/vendor/google.golang.org/grpc/stream.go:757 +0x64
  github.com/moby/buildkit/api/services/control.(*controlSessionClient).CloseSend()
      <autogenerated>:1 +0x62
  github.com/moby/buildkit/session/grpchijack.(*conn).Close.func1()
      /src/session/grpchijack/dial.go:96 +0x5df
  sync.(*Once).Do()
      /usr/local/go/src/sync/once.go:44 +0xde
  github.com/moby/buildkit/session/grpchijack.(*conn).Close()
      /src/session/grpchijack/dial.go:90 +0x7d
  github.com/moby/buildkit/session.(*Session).Close()
      /src/session/session.go:111 +0x103
  github.com/moby/buildkit/client.(*Client).solve.func2.1()
      /src/client/solve.go:171 +0xae
  github.com/moby/buildkit/client.(*Client).solve.func2()
      /src/client/solve.go:189 +0x769
  golang.org/x/sync/errgroup.(*Group).Go.func1()
      /src/vendor/golang.org/x/sync/errgroup/errgroup.go:58 +0x64

Previous read at 0x00c0000d5a50 by goroutine 131:
  google.golang.org/grpc.(*clientStream).SendMsg()
      /src/vendor/google.golang.org/grpc/stream.go:674 +0xf8
  github.com/moby/buildkit/api/services/control.(*controlSessionClient).SendMsg()
      <autogenerated>:1 +0x7a
  github.com/moby/buildkit/session/grpchijack.(*conn).Write()
      /src/session/grpchijack/dial.go:83 +0xf3
  bufio.(*Writer).Flush()
      /usr/local/go/src/bufio/bufio.go:590 +0x13f
  golang.org/x/net/http2.(*bufferedWriter).Flush()
      /src/vendor/golang.org/x/net/http2/http2.go:293 +0x67
  golang.org/x/net/http2.(*serverConn).Flush()
      /src/vendor/golang.org/x/net/http2/server.go:568 +0x50
  golang.org/x/net/http2.(*flushFrameWriter).writeFrame()
      /src/vendor/golang.org/x/net/http2/write.go:68 +0x54
  golang.org/x/net/http2.(*serverConn).writeFrameAsync()
      /src/vendor/golang.org/x/net/http2/server.go:741 +0x58

Goroutine 6 (running) created at:
  golang.org/x/sync/errgroup.(*Group).Go()
      /src/vendor/golang.org/x/sync/errgroup/errgroup.go:55 +0x73
  github.com/moby/buildkit/client.(*Client).solve()
      /src/client/solve.go:161 +0xe1f
  github.com/moby/buildkit/client.(*Client).Build()
      /src/client/build.go:59 +0x5fb
  github.com/moby/buildkit/client.testClientGatewayFailedSolve()
      /src/client/build_test.go:125 +0x1e9
  github.com/moby/buildkit/util/testutil/integration.Run.func2.1()
      /src/util/testutil/integration/run.go:172 +0x2c2
  testing.tRunner()
      /usr/local/go/src/testing/testing.go:865 +0x163

Goroutine 131 (running) created at:
  golang.org/x/net/http2.(*serverConn).startFrameWrite()
      /src/vendor/golang.org/x/net/http2/server.go:1119 +0x367
  golang.org/x/net/http2.(*serverConn).scheduleFrameWrite()
      /src/vendor/golang.org/x/net/http2/server.go:1225 +0x2de
  golang.org/x/net/http2.(*serverConn).wroteFrame()
      /src/vendor/golang.org/x/net/http2/server.go:1181 +0x1de
  golang.org/x/net/http2.(*serverConn).serve()
      /src/vendor/golang.org/x/net/http2/server.go:834 +0x134d
  golang.org/x/net/http2.(*Server).ServeConn()
      /src/vendor/golang.org/x/net/http2/server.go:438 +0xd7b
  github.com/moby/buildkit/session.serve()
      /src/session/grpc.go:24 +0x17b
  github.com/moby/buildkit/session.(*Session).Run()
      /src/session/session.go:103 +0xa10
  github.com/moby/buildkit/client.(*Client).solve.func1()
      /src/client/solve.go:156 +0xf4
  golang.org/x/sync/errgroup.(*Group).Go.func1()
      /src/vendor/golang.org/x/sync/errgroup/errgroup.go:58 +0x64
==================

==================
WARNING: DATA RACE
Write at 0x00c0005138e0 by goroutine 95:
  bytes.(*Buffer).Read()
      /usr/local/go/src/bytes/buffer.go:300 +0x4a
  bufio.(*Scanner).Scan()
      /usr/local/go/src/bufio/scan.go:213 +0x19a
  github.com/moby/buildkit/util/testutil/integration.(*sandbox).PrintLogs()
      /src/util/testutil/integration/oci.go:125 +0x162
  github.com/moby/buildkit/util/testutil/integration.Run.func2.1.1()
      /src/util/testutil/integration/run.go:169 +0xba
  github.com/moby/buildkit/util/testutil/integration.Run.func2.1()
      /src/util/testutil/integration/run.go:173 +0x2c3
  testing.tRunner()
      /usr/local/go/src/testing/testing.go:865 +0x163

Previous write at 0x00c0005138e0 by goroutine 120:
  bytes.(*Buffer).ReadFrom()
      /usr/local/go/src/bytes/buffer.go:203 +0x4b
  io.copyBuffer()
      /usr/local/go/src/io/io.go:388 +0x3fa
  os/exec.(*Cmd).writerDescriptor.func1()
      /usr/local/go/src/io/io.go:364 +0x7a
  os/exec.(*Cmd).Start.func1()
      /usr/local/go/src/os/exec/exec.go:409 +0x34

Goroutine 95 (running) created at:
  testing.(*T).Run()
      /usr/local/go/src/testing/testing.go:916 +0x65a
  github.com/moby/buildkit/util/testutil/integration.Run.func2()
      /src/util/testutil/integration/run.go:154 +0x1eb
  github.com/moby/buildkit/util/testutil/integration.Run()
      /src/util/testutil/integration/run.go:175 +0x6b7
  github.com/moby/buildkit/client.TestClientGatewayIntegration()
      /src/client/build_test.go:25 +0x128
  testing.tRunner()
      /usr/local/go/src/testing/testing.go:865 +0x163

Goroutine 120 (running) created at:
  os/exec.(*Cmd).Start()
      /usr/local/go/src/os/exec/exec.go:408 +0x9c2
  github.com/moby/buildkit/util/testutil/integration.startCmd()
      /src/util/testutil/integration/util.go:28 +0x61
  github.com/moby/buildkit/util/testutil/integration.runBuildkitd()
      /src/util/testutil/integration/oci.go:192 +0x7c4
  github.com/moby/buildkit/util/testutil/integration.(*oci).New()
      /src/util/testutil/integration/oci.go:98 +0x563
  github.com/moby/buildkit/util/testutil/integration.Run.func2.1()
      /src/util/testutil/integration/run.go:159 +0x22e
  testing.tRunner()
      /usr/local/go/src/testing/testing.go:865 +0x163
==================

Bisecting CI failure moby#936 in bumping up grpc 1.12 to 1.20 Signed-off-by: Akihiro Suda <akihiro.suda.cz@hco.ntt.co.jp>

AkihiroSuda · 2019-05-08T11:08:51Z

failure seems caused in grpc/grpc-go@40cd6b1

#981

Signed-off-by: Akihiro Suda <akihiro.suda.cz@hco.ntt.co.jp>

AkihiroSuda · 2019-05-12T12:31:52Z

 			err = c.stream.RecvMsg(m)
 			if err != nil {
 				if err != io.EOF {
+					c.readMu.Unlock()


This was the cause of the failure.

CI now passing 🎉

@tonistiigi @thaJeztah PTAL

I think you could equivalently have moved the existing Unlock() to a defer next to the Lock()?

thaJeztah

LGTM

This was referenced Apr 13, 2019

bump google.golang.org/grpc v1.20.1 moby/moby#39062

Merged

bump google.golang.org/grpc v1.20.1 containerd/containerd#3192

Merged

AkihiroSuda added the help wanted label Apr 14, 2019

AkihiroSuda force-pushed the grpc-1-20 branch 3 times, most recently from bf7f63d to 0d4d708 Compare April 14, 2019 12:09

AkihiroSuda mentioned this pull request Apr 19, 2019

bump google.golang.org/grpc v1.20.1 docker/cli#1824

Merged

AkihiroSuda changed the title ~~bump google.golang.org/grpc v1.20.0~~ bump google.golang.org/grpc v1.20.1 May 5, 2019

AkihiroSuda force-pushed the grpc-1-20 branch from 0d4d708 to 8829a31 Compare May 5, 2019 08:25

AkihiroSuda added a commit to AkihiroSuda/buildkit_poc that referenced this pull request May 7, 2019

[DNM/bisect] grpc 1.16

55da99c

Bisecting CI failure moby#936 in bumping up grpc 1.12 to 1.20 Signed-off-by: Akihiro Suda <akihiro.suda.cz@hco.ntt.co.jp>

AkihiroSuda mentioned this pull request May 7, 2019

[DNM/bisect] grpc 1.16(fail) -> 1.14(fail) -> 1.13(succcess) #981

Closed

AkihiroSuda mentioned this pull request May 8, 2019

session/grpchijack: fix race #989

Merged

AkihiroSuda force-pushed the grpc-1-20 branch from 8829a31 to 6fd662b Compare May 12, 2019 04:14

bump google.golang.org/grpc v1.20.1

eebb08f

Signed-off-by: Akihiro Suda <akihiro.suda.cz@hco.ntt.co.jp>

AkihiroSuda force-pushed the grpc-1-20 branch from 6fd662b to eebb08f Compare May 12, 2019 06:20

session/grpchijack: fix unlock

15242f3

Signed-off-by: Akihiro Suda <akihiro.suda.cz@hco.ntt.co.jp>

AkihiroSuda removed the help wanted label May 12, 2019

AkihiroSuda commented May 12, 2019

View reviewed changes

thaJeztah approved these changes May 12, 2019

View reviewed changes

tonistiigi approved these changes May 13, 2019

View reviewed changes

tonistiigi merged commit 3625d72 into moby:master May 13, 2019

tonistiigi mentioned this pull request May 13, 2019

vendor: update buildkit to f238f1ef moby/moby#39210

Merged

thaJeztah mentioned this pull request May 14, 2019

[19.03 backport] BuildKit fixes docker-archive/engine#215

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

bump google.golang.org/grpc v1.20.1#936

bump google.golang.org/grpc v1.20.1#936
tonistiigi merged 2 commits into
moby:masterfrom
AkihiroSuda:grpc-1-20

AkihiroSuda commented Apr 13, 2019

Uh oh!

AkihiroSuda commented Apr 13, 2019

Uh oh!

AkihiroSuda commented Apr 13, 2019 •

edited

Loading

Uh oh!

AkihiroSuda commented Apr 14, 2019

Uh oh!

AkihiroSuda commented Apr 14, 2019

Uh oh!

thaJeztah commented Apr 14, 2019

Uh oh!

AkihiroSuda commented Apr 14, 2019

Uh oh!

AkihiroSuda commented May 8, 2019

Uh oh!

AkihiroSuda May 12, 2019

Uh oh!

ijc May 13, 2019

Uh oh!

thaJeztah left a comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

Conversation

AkihiroSuda commented Apr 13, 2019

Uh oh!

AkihiroSuda commented Apr 13, 2019

Uh oh!

AkihiroSuda commented Apr 13, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

AkihiroSuda commented Apr 14, 2019

Uh oh!

AkihiroSuda commented Apr 14, 2019

Uh oh!

thaJeztah commented Apr 14, 2019

Uh oh!

AkihiroSuda commented Apr 14, 2019

Uh oh!

AkihiroSuda commented May 8, 2019

Uh oh!

AkihiroSuda May 12, 2019

Choose a reason for hiding this comment

Uh oh!

ijc May 13, 2019

Choose a reason for hiding this comment

Uh oh!

thaJeztah left a comment

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

AkihiroSuda commented Apr 13, 2019 •

edited

Loading