Update to use the canonical version of LB proto by dgquintas · Pull Request #15840 · grpc/grpc

dgquintas · 2018-06-21T23:41:19Z

For now, load_balance.proto is a copy of https://github.com/grpc/grpc-proto/blob/master/grpc/lb/v1/load_balancer.proto . In the future we may want to submodule grpc/grpc-proto instead.

The approach taken involves compiling google.protobuf.duration and google.protobuf.timestamp using nanopb, thus generating C code the core can compile and link. Most other changes are to auxiliary scripts used to perform this compilation.

Fixes #15728

grpc-testing · 2018-06-21T23:53:49Z

[trickle] No significant performance differences

grpc-testing · 2018-06-22T00:10:31Z

****************************************************************

libgrpc.so

     VM SIZE                                            FILE SIZE
 ++++++++++++++ GROWING                              ++++++++++++++
  +0.0%     +32 [None]                               +2.45Ki  +0.0%
      +0.0%     +32 [Unmapped]                           +2.45Ki  +0.0%
      [NEW]     +87 google_protobuf_Duration_fields          +87  [NEW]
      [NEW]     +87 google_protobuf_Timestamp_fields         +87  [NEW]

  +0.0%     +32 TOTAL                                +2.45Ki  +0.0%


****************************************************************

libgrpc++.so

     VM SIZE        FILE SIZE
 ++++++++++++++  ++++++++++++++

  [ = ]       0        0  [ = ]

grpc-testing · 2018-06-22T00:18:47Z

[trickle] No significant performance differences

grpc-testing · 2018-06-22T00:36:11Z

[microbenchmarks] No significant performance differences

carl-mastrangelo

Awesome, LGTM

grpc-testing · 2018-06-22T00:50:21Z

****************************************************************

libgrpc.so

     VM SIZE                                            FILE SIZE
 ++++++++++++++ GROWING                              ++++++++++++++
  +0.0%     +32 [None]                               +2.45Ki  +0.0%
      +0.0%     +32 [Unmapped]                           +2.45Ki  +0.0%
      [NEW]     +87 google_protobuf_Duration_fields          +87  [NEW]
      [NEW]     +87 google_protobuf_Timestamp_fields         +87  [NEW]

  +0.0%     +32 TOTAL                                +2.45Ki  +0.0%


****************************************************************

libgrpc++.so

     VM SIZE        FILE SIZE
 ++++++++++++++  ++++++++++++++

  [ = ]       0        0  [ = ]

grpc-testing · 2018-06-22T00:58:26Z

[trickle] No significant performance differences

grpc-testing · 2018-06-22T01:03:22Z

[microbenchmarks] No significant performance differences

grpc-testing · 2018-06-22T01:41:49Z

[microbenchmarks] No significant performance differences

grpc-testing · 2018-06-22T01:54:20Z

****************************************************************

libgrpc.so

     VM SIZE                                            FILE SIZE
 ++++++++++++++ GROWING                              ++++++++++++++
  +0.0%     +32 [None]                               +2.45Ki  +0.0%
      +0.0%     +32 [Unmapped]                           +2.45Ki  +0.0%
      [NEW]     +87 google_protobuf_Duration_fields          +87  [NEW]
      [NEW]     +87 google_protobuf_Timestamp_fields         +87  [NEW]

  +0.0%     +32 TOTAL                                +2.45Ki  +0.0%


****************************************************************

libgrpc++.so

     VM SIZE        FILE SIZE
 ++++++++++++++  ++++++++++++++

  [ = ]       0        0  [ = ]

grpc-testing · 2018-06-22T02:02:10Z

[trickle] No significant performance differences

grpc-testing · 2018-06-22T02:46:40Z

[microbenchmarks] No significant performance differences

AspirinSJL · 2018-06-22T03:03:25Z

src/proto/grpc/lb/v1/load_balancer.proto

@@ -1,4 +1,4 @@
-// Copyright 2016 gRPC authors.
+// Copyright 2015 The gRPC Authors


I'm just curious about why this gets decreased?

The earlier copyright takes precedence. This file was copied a few times, but the original date holds.

AspirinSJL · 2018-06-22T03:07:06Z

src/proto/grpc/lb/v1/load_balancer.proto

 // See the License for the specific language governing permissions and
 // limitations under the License.

+// This file defines an the GRPCLB LoadBalancing protocol.


Nit: "an" can be removed.

@carl-mastrangelo can you fix that upstream?

@AspirinSJL actually has a PR upstream already, perhaps she can attach the change to her existing PR?

Oh, I thought this only exists in downstream copies. I will attach the change to my PR.

AspirinSJL · 2018-06-22T03:10:58Z

src/proto/grpc/lb/v1/load_balancer.proto

-  // length of the name is 256 bytes.
-  // The name might include a port number. How to handle the port number is up
-  // to the balancer.
+  // Name of load balanced service (IE, service.googleapis.com). Its


This overwrites my change before. I should probably update the canonical file.

Updated in grpc/grpc-proto#28.

markdroth · 2018-06-22T13:57:53Z

tools/distrib/check_nanopb_output.sh

 popd

+##
+## Checks for load_balancer.proto


Why is this all commented out?

grpc-testing · 2018-06-22T18:01:11Z

****************************************************************

libgrpc.so

     VM SIZE                                            FILE SIZE
 ++++++++++++++ GROWING                              ++++++++++++++
  +0.0%     +32 [None]                               +2.45Ki  +0.0%
      +0.0%     +32 [Unmapped]                           +2.45Ki  +0.0%
      [NEW]     +87 google_protobuf_Duration_fields          +87  [NEW]
      [NEW]     +87 google_protobuf_Timestamp_fields         +87  [NEW]

  +0.0%     +32 TOTAL                                +2.45Ki  +0.0%


****************************************************************

libgrpc++.so

     VM SIZE        FILE SIZE
 ++++++++++++++  ++++++++++++++

  [ = ]       0        0  [ = ]

grpc-testing · 2018-06-22T18:08:49Z

[trickle] No significant performance differences

grpc-testing · 2018-06-22T18:52:54Z

[microbenchmarks] No significant performance differences

AspirinSJL · 2018-06-22T20:36:43Z

grpc/grpc-proto#28 is merged. Please sync. Thanks!

dgquintas · 2018-06-22T20:41:40Z

Done. PTAL.

grpc-testing · 2018-06-22T20:45:17Z

****************************************************************

libgrpc.so

     VM SIZE                                            FILE SIZE
 ++++++++++++++ GROWING                              ++++++++++++++
  [ = ]       0 [None]                               +2.46Ki  +0.0%
      [ = ]       0 [Unmapped]                           +2.46Ki  +0.0%
      [NEW]     +87 google_protobuf_Duration_fields          +87  [NEW]
      [NEW]     +87 google_protobuf_Timestamp_fields         +87  [NEW]

  [ = ]       0 TOTAL                                +2.46Ki  +0.0%


****************************************************************

libgrpc++.so

     VM SIZE        FILE SIZE
 ++++++++++++++  ++++++++++++++

  [ = ]       0        0  [ = ]

grpc-testing · 2018-06-22T20:51:24Z

[trickle] No significant performance differences

grpc-testing · 2018-06-22T21:36:56Z

[microbenchmarks] No significant performance differences

dgquintas · 2018-06-22T23:04:23Z

Issues: #15827

dgquintas requested review from AspirinSJL and carl-mastrangelo June 21, 2018 23:41

dgquintas requested a review from markdroth as a code owner June 21, 2018 23:41

AspirinSJL mentioned this pull request Jun 21, 2018

Update comment of LB token grpc/grpc-proto#27

Merged

dgquintas force-pushed the common_nanopb branch from 2f63ee1 to 5535cd7 Compare June 22, 2018 00:06

grpc deleted a comment from grpc-testing Jun 22, 2018

carl-mastrangelo approved these changes Jun 22, 2018

View reviewed changes

Update to use the canonical version of LB proto

f3955ba

dgquintas force-pushed the common_nanopb branch from 5535cd7 to f3955ba Compare June 22, 2018 00:46

dgquintas added the kokoro:force-run label Jun 22, 2018

kokoro-team removed the kokoro:force-run label Jun 22, 2018

AspirinSJL reviewed Jun 22, 2018

View reviewed changes

markdroth reviewed Jun 22, 2018

View reviewed changes

Uncomment nanopb output check

8c8cd02

Sync'd load_balancer.proto

1645e1c

AspirinSJL approved these changes Jun 22, 2018

View reviewed changes

dgquintas merged commit 3acf8e6 into grpc:master Jun 22, 2018

dgquintas mentioned this pull request Jun 25, 2018

PHP distrib tests failing on linux and macos #15865

Closed

dgquintas added release notes: yes Indicates if PR needs to be in release notes lang/core labels Jul 27, 2018

lock bot locked as resolved and limited conversation to collaborators Oct 25, 2018

		@@ -1,4 +1,4 @@
		// Copyright 2016 gRPC authors.
		// Copyright 2015 The gRPC Authors

Conversation

dgquintas commented Jun 21, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

grpc-testing commented Jun 21, 2018

Uh oh!

grpc-testing commented Jun 22, 2018

Uh oh!

grpc-testing commented Jun 22, 2018

Uh oh!

grpc-testing commented Jun 22, 2018

Uh oh!

carl-mastrangelo left a comment

Choose a reason for hiding this comment

Uh oh!

grpc-testing commented Jun 22, 2018

Uh oh!

grpc-testing commented Jun 22, 2018

Uh oh!

grpc-testing commented Jun 22, 2018

Uh oh!

grpc-testing commented Jun 22, 2018

Uh oh!

grpc-testing commented Jun 22, 2018

Uh oh!

grpc-testing commented Jun 22, 2018

Uh oh!

grpc-testing commented Jun 22, 2018

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

grpc-testing commented Jun 22, 2018

Uh oh!

grpc-testing commented Jun 22, 2018

Uh oh!

grpc-testing commented Jun 22, 2018

Uh oh!

AspirinSJL commented Jun 22, 2018

Uh oh!

dgquintas commented Jun 22, 2018

Uh oh!

grpc-testing commented Jun 22, 2018

Uh oh!

grpc-testing commented Jun 22, 2018

Uh oh!

grpc-testing commented Jun 22, 2018

Uh oh!

dgquintas commented Jun 22, 2018

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

dgquintas commented Jun 21, 2018 •

edited

Loading