Skip to content

Commit

Permalink
[1/3] Bump grpc to 1.32.x to fix a too_many_pings regression
Browse files Browse the repository at this point in the history
Part 1: add v1.32.x version to third_party/grpc
Note: partly switches to v1.32.x too as not all bits are versioned and
      some of unversioned bits are used from other third_party targets
Composed PR: bazelbuild#12273

grpc-java transition from v1.26.0 to v1.31.1 enabled auto flow control
which  started failing in RBE with

io.grpc.StatusRuntimeException: RESOURCE_EXHAUSTED: Bandwidth exhausted
HTTP/2 error code: ENHANCE_YOUR_CALM
Received Goaway
too_many_pings

grpc-java v1.32.2 has a bugfix attempt on that
grpc v1.32.0 also has something new around keepalive pings

Hopefully version bump to those helps

bazelbuild#12264

Note: also an attempt and disabling auto flow by default is made in
bazelbuild#12266

Closes bazelbuild#12279
  • Loading branch information
dmivankov authored and coeuvre committed Oct 22, 2020
1 parent c99c88b commit 9c0010c
Show file tree
Hide file tree
Showing 16 changed files with 154 additions and 45 deletions.
18 changes: 9 additions & 9 deletions third_party/grpc/BUILD
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ load("//tools/distributions:distribution_rules.bzl", "distrib_java_import", "dis

licenses(["notice"]) # Apache v2

exports_files(["grpc_1.31.1.patch"])
exports_files(["grpc_1.31.1.patch", "grpc_1.32.0.patch"])

package(default_visibility = ["//visibility:public"])

Expand All @@ -32,14 +32,14 @@ filegroup(
distrib_jar_filegroup(
name = "bootstrap-grpc-jars",
srcs = [
"grpc-api-1.31.1.jar",
"grpc-auth-1.31.1.jar",
"grpc-context-1.31.1.jar",
"grpc-core-1.31.1.jar",
"grpc-netty-1.31.1.jar",
"grpc-protobuf-1.31.1.jar",
"grpc-protobuf-lite-1.31.1.jar",
"grpc-stub-1.31.1.jar",
"grpc-api-1.32.2.jar",
"grpc-auth-1.32.2.jar",
"grpc-context-1.32.2.jar",
"grpc-core-1.32.2.jar",
"grpc-netty-1.32.2.jar",
"grpc-protobuf-1.32.2.jar",
"grpc-protobuf-lite-1.32.2.jar",
"grpc-stub-1.32.2.jar",
],
enable_distributions = ["debian"],
)
Expand Down
16 changes: 11 additions & 5 deletions third_party/grpc/README.bazel.md
Original file line number Diff line number Diff line change
@@ -1,24 +1,30 @@
# How to update the C++ sources of gRPC:

1. Update the gRPC definitions in WORKSPACE file, currently we use
https://github.com/grpc/grpc/archive/v1.31.1.tar.gz
https://github.com/grpc/grpc/archive/v1.32.2.tar.gz
2. Update the gRPC patch file if necessary, it mostly helps avoid unnecessary dependencies.
3. Update third_party/grpc/BUILD to redirect targets to @com_github_grpc_grpc if necessary.

# How to update the BUILD/bzl sources of gRPC:

1. `git clone http://github.com/grpc/grpc.git` in a convenient directory
2. `git checkout <tag>` (current is `v1.31.1`, commithash `7d7e456762`)
2. `git checkout <tag>` (current is `v1.32.0`, commithash `414bb8322d`)
3. `mkdir -p third_party/grpc/bazel`
4. `cp <gRPC git tree>/bazel/{BUILD,cc_grpc_library.bzl,generate_cc.bzl,protobuf.bzl} third_party/grpc/bazel`
5. In the `third_party/grpc` directory, apply local patches:
`patch -p3 < bazel_1.31.1.patch`
`patch -p3 < bazel_1.32.0.patch`

# How to update the Java plugin:

1. Checkout tag `v1.31.1` from https://github.com/grpc/grpc-java
1. Checkout tag `v1.32.2` from https://github.com/grpc/grpc-java
2. `cp -R <grpc-java git tree>/compiler/src/java_plugin third_party/grpc/compiler/src`

# How to update the Java code:

Download the necessary jars at version `1.31.1` from maven central.
Download the necessary jars at version `1.32.2` from maven central.

# Submitting the change needs 3 pull requests

1. Update third_party/grpc to include files from new version
2. Switch WORKSPACE, scripts/bootstrap/compile.sh and any other references to new version
3. Remove older version from third_party/grpc
7 changes: 6 additions & 1 deletion third_party/grpc/bazel/protobuf.bzl
Original file line number Diff line number Diff line change
Expand Up @@ -214,7 +214,12 @@ def get_out_dir(protos, context):
path = out_dir,
import_path = out_dir[out_dir.find(_VIRTUAL_IMPORTS) + 1:],
)
return struct(path = context.genfiles_dir.path, import_path = None)

out_dir = context.genfiles_dir.path
ws_root = context.label.workspace_root
if ws_root:
out_dir = out_dir + "/" + ws_root
return struct(path = out_dir, import_path = None)

def is_in_virtual_imports(source_file, virtual_folder = _VIRTUAL_IMPORTS):
"""Determines if source_file is virtual (is placed in _virtual_imports
Expand Down
File renamed without changes.
38 changes: 21 additions & 17 deletions third_party/grpc/compiler/src/java_plugin/cpp/java_generator.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -37,18 +37,22 @@
#define XSTR(s) STR(s)
#endif

#ifndef FALLTHROUGH_INTENDED
#define FALLTHROUGH_INTENDED
#ifdef ABSL_FALLTHROUGH_INTENDED
#define FALLTHROUGH ABSL_FALLTHROUGH_INTENDED
#else
#define FALLTHROUGH
#endif

namespace java_grpc_generator {

using google::protobuf::FileDescriptor;
using google::protobuf::ServiceDescriptor;
using google::protobuf::MethodDescriptor;
using google::protobuf::Descriptor;
using google::protobuf::io::Printer;
using google::protobuf::SourceLocation;
namespace protobuf = google::protobuf;

using protobuf::Descriptor;
using protobuf::FileDescriptor;
using protobuf::MethodDescriptor;
using protobuf::ServiceDescriptor;
using protobuf::SourceLocation;
using protobuf::io::Printer;
using std::to_string;

// java keywords from: https://docs.oracle.com/javase/specs/jls/se8/html/jls-3.html#jls-3.9
Expand Down Expand Up @@ -162,7 +166,7 @@ static inline std::string MethodIdFieldName(const MethodDescriptor* method) {
}

static inline std::string MessageFullJavaName(const Descriptor* desc) {
return google::protobuf::compiler::java::ClassName(desc);
return protobuf::compiler::java::ClassName(desc);
}

// TODO(nmittler): Remove once protobuf includes javadoc methods in distribution.
Expand Down Expand Up @@ -425,12 +429,12 @@ static void PrintMethodFields(
" .setFullMethodName(generateFullMethodName(SERVICE_NAME, \"$method_name$\"))\n");

bool safe = method->options().idempotency_level()
== google::protobuf::MethodOptions_IdempotencyLevel_NO_SIDE_EFFECTS;
== protobuf::MethodOptions_IdempotencyLevel_NO_SIDE_EFFECTS;
if (safe) {
p->Print(*vars, " .setSafe(true)\n");
} else {
bool idempotent = method->options().idempotency_level()
== google::protobuf::MethodOptions_IdempotencyLevel_IDEMPOTENT;
== protobuf::MethodOptions_IdempotencyLevel_IDEMPOTENT;
if (idempotent) {
p->Print(*vars, " .setIdempotent(true)\n");
}
Expand Down Expand Up @@ -542,7 +546,7 @@ static void PrintStub(
break;
case BLOCKING_CLIENT_INTERFACE:
interface = true;
FALLTHROUGH_INTENDED;
FALLTHROUGH;
case BLOCKING_CLIENT_IMPL:
call_type = BLOCKING_CALL;
stub_name += "BlockingStub";
Expand All @@ -551,7 +555,7 @@ static void PrintStub(
break;
case FUTURE_CLIENT_INTERFACE:
interface = true;
FALLTHROUGH_INTENDED;
FALLTHROUGH;
case FUTURE_CLIENT_IMPL:
call_type = FUTURE_CALL;
stub_name += "FutureStub";
Expand Down Expand Up @@ -916,7 +920,7 @@ static void PrintGetServiceDescriptorMethod(const ServiceDescriptor* service,
(*vars)["proto_base_descriptor_supplier"] = service->name() + "BaseDescriptorSupplier";
(*vars)["proto_file_descriptor_supplier"] = service->name() + "FileDescriptorSupplier";
(*vars)["proto_method_descriptor_supplier"] = service->name() + "MethodDescriptorSupplier";
(*vars)["proto_class_name"] = google::protobuf::compiler::java::ClassName(service->file());
(*vars)["proto_class_name"] = protobuf::compiler::java::ClassName(service->file());
p->Print(
*vars,
"private static abstract class $proto_base_descriptor_supplier$\n"
Expand Down Expand Up @@ -1186,7 +1190,7 @@ void PrintImports(Printer* p) {
}

void GenerateService(const ServiceDescriptor* service,
google::protobuf::io::ZeroCopyOutputStream* out,
protobuf::io::ZeroCopyOutputStream* out,
ProtoFlavor flavor,
bool disable_version) {
// All non-generated classes must be referred by fully qualified names to
Expand Down Expand Up @@ -1242,7 +1246,7 @@ void GenerateService(const ServiceDescriptor* service,
}

std::string ServiceJavaPackage(const FileDescriptor* file) {
std::string result = google::protobuf::compiler::java::ClassName(file);
std::string result = protobuf::compiler::java::ClassName(file);
size_t last_dot_pos = result.find_last_of('.');
if (last_dot_pos != std::string::npos) {
result.resize(last_dot_pos);
Expand All @@ -1252,7 +1256,7 @@ std::string ServiceJavaPackage(const FileDescriptor* file) {
return result;
}

std::string ServiceClassName(const google::protobuf::ServiceDescriptor* service) {
std::string ServiceClassName(const ServiceDescriptor* service) {
return service->name() + "Grpc";
}

Expand Down
14 changes: 8 additions & 6 deletions third_party/grpc/compiler/src/java_plugin/cpp/java_generator.h
Original file line number Diff line number Diff line change
Expand Up @@ -47,24 +47,26 @@ class LogHelper {
// Abort the program after logging the mesage.
#define GRPC_CODEGEN_FAIL GRPC_CODEGEN_CHECK(false)

using namespace std;

namespace java_grpc_generator {

namespace impl {
namespace protobuf = google::protobuf;
} // namespace impl

enum ProtoFlavor {
NORMAL, LITE
};

// Returns the package name of the gRPC services defined in the given file.
std::string ServiceJavaPackage(const google::protobuf::FileDescriptor* file);
std::string ServiceJavaPackage(const impl::protobuf::FileDescriptor* file);

// Returns the name of the outer class that wraps in all the generated code for
// the given service.
std::string ServiceClassName(const google::protobuf::ServiceDescriptor* service);
std::string ServiceClassName(const impl::protobuf::ServiceDescriptor* service);

// Writes the generated service interface into the given ZeroCopyOutputStream
void GenerateService(const google::protobuf::ServiceDescriptor* service,
google::protobuf::io::ZeroCopyOutputStream* out,
void GenerateService(const impl::protobuf::ServiceDescriptor* service,
impl::protobuf::io::ZeroCopyOutputStream* out,
ProtoFlavor flavor,
bool disable_version);

Expand Down
16 changes: 9 additions & 7 deletions third_party/grpc/compiler/src/java_plugin/cpp/java_plugin.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,8 @@
#include <google/protobuf/descriptor.h>
#include <google/protobuf/io/zero_copy_stream.h>

namespace protobuf = google::protobuf;

static std::string JavaPackageToDir(const std::string& package_name) {
std::string package_dir = package_name;
for (size_t i = 0; i < package_dir.size(); ++i) {
Expand All @@ -38,7 +40,7 @@ static std::string JavaPackageToDir(const std::string& package_name) {
return package_dir;
}

class JavaGrpcGenerator : public google::protobuf::compiler::CodeGenerator {
class JavaGrpcGenerator : public protobuf::compiler::CodeGenerator {
public:
JavaGrpcGenerator() {}
virtual ~JavaGrpcGenerator() {}
Expand All @@ -47,12 +49,12 @@ class JavaGrpcGenerator : public google::protobuf::compiler::CodeGenerator {
return FEATURE_PROTO3_OPTIONAL;
}

virtual bool Generate(const google::protobuf::FileDescriptor* file,
virtual bool Generate(const protobuf::FileDescriptor* file,
const std::string& parameter,
google::protobuf::compiler::GeneratorContext* context,
protobuf::compiler::GeneratorContext* context,
std::string* error) const override {
std::vector<std::pair<std::string, std::string> > options;
google::protobuf::compiler::ParseGeneratorParameter(parameter, &options);
protobuf::compiler::ParseGeneratorParameter(parameter, &options);

java_grpc_generator::ProtoFlavor flavor =
java_grpc_generator::ProtoFlavor::NORMAL;
Expand All @@ -69,10 +71,10 @@ class JavaGrpcGenerator : public google::protobuf::compiler::CodeGenerator {
std::string package_name = java_grpc_generator::ServiceJavaPackage(file);
std::string package_filename = JavaPackageToDir(package_name);
for (int i = 0; i < file->service_count(); ++i) {
const google::protobuf::ServiceDescriptor* service = file->service(i);
const protobuf::ServiceDescriptor* service = file->service(i);
std::string filename = package_filename
+ java_grpc_generator::ServiceClassName(service) + ".java";
std::unique_ptr<google::protobuf::io::ZeroCopyOutputStream> output(
std::unique_ptr<protobuf::io::ZeroCopyOutputStream> output(
context->Open(filename));
java_grpc_generator::GenerateService(
service, output.get(), flavor, disable_version);
Expand All @@ -83,5 +85,5 @@ class JavaGrpcGenerator : public google::protobuf::compiler::CodeGenerator {

int main(int argc, char* argv[]) {
JavaGrpcGenerator generator;
return google::protobuf::compiler::PluginMain(argc, argv, &generator);
return protobuf::compiler::PluginMain(argc, argv, &generator);
}
Binary file added third_party/grpc/grpc-api-1.32.2.jar
Binary file not shown.
Binary file added third_party/grpc/grpc-auth-1.32.2.jar
Binary file not shown.
Binary file added third_party/grpc/grpc-context-1.32.2.jar
Binary file not shown.
Binary file added third_party/grpc/grpc-core-1.32.2.jar
Binary file not shown.
Binary file added third_party/grpc/grpc-netty-1.32.2.jar
Binary file not shown.
Binary file added third_party/grpc/grpc-protobuf-1.32.2.jar
Binary file not shown.
Binary file added third_party/grpc/grpc-protobuf-lite-1.32.2.jar
Binary file not shown.
Binary file added third_party/grpc/grpc-stub-1.32.2.jar
Binary file not shown.
90 changes: 90 additions & 0 deletions third_party/grpc/grpc_1.32.0.patch
Original file line number Diff line number Diff line change
@@ -0,0 +1,90 @@
commit bb0d04663c7dc6c0096f8717cb4ec26330a5ae40
Author: Yun Peng <pcloudy@google.com>
Date: Wed Jun 3 15:35:31 2020 +0200

Patch grpc v1.26.0 for Bazel build

- Avoid loading dependencies that're not needed for the gRPC C++
libraries
- Add bazel mirror URL for upb and cares
- Redirect zlib to @//third_party/zlib

diff --git a/bazel/grpc_build_system.bzl b/bazel/grpc_build_system.bzl
index 7bb6b8bdb9..7644107b70 100644
--- a/bazel/grpc_build_system.bzl
+++ b/bazel/grpc_build_system.bzl
@@ -25,7 +25,7 @@

load("//bazel:cc_grpc_library.bzl", "cc_grpc_library")
load("@upb//bazel:upb_proto_library.bzl", "upb_proto_library")
-load("@build_bazel_rules_apple//apple:ios.bzl", "ios_unit_test")
+# load("@build_bazel_rules_apple//apple:ios.bzl", "ios_unit_test")

# The set of pollers to test against if a test exercises polling
POLLERS = ["epollex", "epoll1", "poll"]
@@ -181,13 +181,13 @@ def ios_cc_test(
testonly = 1,
)
ios_test_deps = [ios_test_adapter, ":" + test_lib_ios]
- ios_unit_test(
- name = name + "_on_ios",
- size = kwargs.get("size"),
- tags = ios_tags,
- minimum_os_version = "9.0",
- deps = ios_test_deps,
- )
+ # ios_unit_test(
+ # name = name + "_on_ios",
+ # size = kwargs.get("size"),
+ # tags = ios_tags,
+ # minimum_os_version = "9.0",
+ # deps = ios_test_deps,
+ # )

def grpc_cc_test(name, srcs = [], deps = [], external_deps = [], args = [], data = [], uses_polling = True, language = "C++", size = "medium", timeout = None, tags = [], exec_compatible_with = [], exec_properties = {}, shard_count = None, flaky = None):
copts = if_mac(["-DGRPC_CFSTREAM"])
diff --git a/bazel/grpc_deps.bzl b/bazel/grpc_deps.bzl
index 09fcad95a2..9b737e5deb 100644
--- a/bazel/grpc_deps.bzl
+++ b/bazel/grpc_deps.bzl
@@ -33,7 +33,7 @@ def grpc_deps():

native.bind(
name = "madler_zlib",
- actual = "@zlib//:zlib",
+ actual = "@//third_party/zlib",
)

native.bind(
diff --git a/bazel/grpc_extra_deps.bzl b/bazel/grpc_extra_deps.bzl
index 4c1dfad2e8..f63c54ddef 100644
--- a/bazel/grpc_extra_deps.bzl
+++ b/bazel/grpc_extra_deps.bzl
@@ -1,11 +1,6 @@
"""Loads the dependencies necessary for the external repositories defined in grpc_deps.bzl."""

-load("@com_google_protobuf//:protobuf_deps.bzl", "protobuf_deps")
load("@upb//bazel:workspace_deps.bzl", "upb_deps")
-load("@envoy_api//bazel:repositories.bzl", "api_dependencies")
-load("@io_bazel_rules_go//go:deps.bzl", "go_register_toolchains", "go_rules_dependencies")
-load("@build_bazel_rules_apple//apple:repositories.bzl", "apple_rules_dependencies")
-load("@build_bazel_apple_support//lib:repositories.bzl", "apple_support_dependencies")

def grpc_extra_deps():
"""Loads the extra dependencies.
@@ -26,15 +21,5 @@ def grpc_extra_deps():
grpc_extra_deps()
```
"""
- protobuf_deps()
-
upb_deps()

- api_dependencies()
-
- go_rules_dependencies()
- go_register_toolchains()
-
- apple_rules_dependencies()
-
- apple_support_dependencies()

0 comments on commit 9c0010c

Please sign in to comment.