Merge branch 'master' of https://github.com/grpc/grpc into channelz-pagination

26 files changed, 95 insertions, 279 deletions

diff --git a/src/core/ext/filters/client_channel/client_channel.cc b/src/core/ext/filters/client_channel/client_channel.cc
index bb3ea400d1..d63e2c66c2 100644
--- a/src/core/ext/filters/client_channel/client_channel.cc
+++ b/src/core/ext/filters/client_channel/client_channel.cc
@@ -2194,9 +2194,9 @@ static void add_retriable_send_initial_metadata_op(
                                    .grpc_previous_rpc_attempts);
   }
   if (GPR_UNLIKELY(calld->num_attempts_completed > 0)) {
-    grpc_mdelem retry_md = grpc_mdelem_from_slices(
+    grpc_mdelem retry_md = grpc_mdelem_create(
         GRPC_MDSTR_GRPC_PREVIOUS_RPC_ATTEMPTS,
-        *retry_count_strings[calld->num_attempts_completed - 1]);
+        *retry_count_strings[calld->num_attempts_completed - 1], nullptr);
     grpc_error* error = grpc_metadata_batch_add_tail(
         &retry_state->send_initial_metadata,
         &retry_state->send_initial_metadata_storage[calld->send_initial_metadata
diff --git a/src/core/ext/filters/client_channel/subchannel.cc b/src/core/ext/filters/client_channel/subchannel.cc
index 3a1c14c6f1..78ff426b11 100644
--- a/src/core/ext/filters/client_channel/subchannel.cc
+++ b/src/core/ext/filters/client_channel/subchannel.cc
@@ -760,9 +760,12 @@ static void get_call_status(grpc_subchannel_call* call,
     grpc_error_get_status(error, call->deadline, status, nullptr, nullptr,
                           nullptr);
   } else {
-    GPR_ASSERT(md_batch->idx.named.grpc_status != nullptr);
-    *status =
-        grpc_get_status_code_from_metadata(md_batch->idx.named.grpc_status->md);
+    if (md_batch->idx.named.grpc_status != nullptr) {
+      *status = grpc_get_status_code_from_metadata(
+          md_batch->idx.named.grpc_status->md);
+    } else {
+      *status = GRPC_STATUS_UNKNOWN;
+    }
   }
   GRPC_ERROR_UNREF(error);
 }
diff --git a/src/core/ext/filters/http/client_authority_filter.cc b/src/core/ext/filters/http/client_authority_filter.cc
index 1ca20ebb26..6383f12594 100644
--- a/src/core/ext/filters/http/client_authority_filter.cc
+++ b/src/core/ext/filters/http/client_authority_filter.cc
@@ -59,9 +59,8 @@ void authority_start_transport_stream_op_batch(
       initial_metadata->idx.named.authority == nullptr) {
     grpc_error* error = grpc_metadata_batch_add_head(
         initial_metadata, &calld->authority_storage,
-        grpc_mdelem_from_slices(
-            GRPC_MDSTR_AUTHORITY,
-            grpc_slice_ref_internal(chand->default_authority)));
+        grpc_mdelem_create(GRPC_MDSTR_AUTHORITY, chand->default_authority,
+                           nullptr));
     if (error != GRPC_ERROR_NONE) {
       grpc_transport_stream_op_batch_finish_with_failure(batch, error,
                                                          calld->call_combiner);
diff --git a/src/core/ext/transport/chttp2/transport/chttp2_transport.cc b/src/core/ext/transport/chttp2/transport/chttp2_transport.cc
index 2d4b4da4c6..8a481bb7d5 100644
--- a/src/core/ext/transport/chttp2/transport/chttp2_transport.cc
+++ b/src/core/ext/transport/chttp2/transport/chttp2_transport.cc
@@ -2128,8 +2128,7 @@ void grpc_chttp2_fake_status(grpc_chttp2_transport* t, grpc_chttp2_stream* s,
           "add_status_message",
           grpc_chttp2_incoming_metadata_buffer_replace_or_add(
               &s->metadata_buffer[1],
-              grpc_mdelem_from_slices(GRPC_MDSTR_GRPC_MESSAGE,
-                                      grpc_slice_ref_internal(slice))));
+              grpc_mdelem_create(GRPC_MDSTR_GRPC_MESSAGE, slice, nullptr)));
     }
     s->published_metadata[1] = GRPC_METADATA_SYNTHESIZED_FROM_FAKE;
     grpc_chttp2_maybe_complete_recv_trailing_metadata(t, s);
diff --git a/src/core/lib/channel/channelz.h b/src/core/lib/channel/channelz.h
index fddef793fb..88551befc8 100644
--- a/src/core/lib/channel/channelz.h
+++ b/src/core/lib/channel/channelz.h
@@ -42,13 +42,13 @@
 
 /** This is the default value for whether or not to enable channelz. If
  * GRPC_ARG_ENABLE_CHANNELZ is set, it will override this default value. */
-#define GRPC_ENABLE_CHANNELZ_DEFAULT false
+#define GRPC_ENABLE_CHANNELZ_DEFAULT true
 
 /** This is the default value for the maximum amount of memory used by trace
  * events per channel trace node. If
  * GRPC_ARG_MAX_CHANNEL_TRACE_EVENT_MEMORY_PER_NODE is set, it will override
  * this default value. */
-#define GRPC_MAX_CHANNEL_TRACE_EVENT_MEMORY_PER_NODE_DEFAULT 0
+#define GRPC_MAX_CHANNEL_TRACE_EVENT_MEMORY_PER_NODE_DEFAULT 1024 * 4
 
 namespace grpc_core {
 
diff --git a/src/core/lib/iomgr/tcp_posix.cc b/src/core/lib/iomgr/tcp_posix.cc
index e40bf81c90..aa2704ce26 100644
--- a/src/core/lib/iomgr/tcp_posix.cc
+++ b/src/core/lib/iomgr/tcp_posix.cc
@@ -468,7 +468,9 @@ static void tcp_do_read(grpc_tcp* tcp) {
     GRPC_STATS_INC_TCP_READ_SIZE(read_bytes);
     add_to_estimate(tcp, static_cast<size_t>(read_bytes));
     GPR_ASSERT((size_t)read_bytes <= tcp->incoming_buffer->length);
-    if (static_cast<size_t>(read_bytes) < tcp->incoming_buffer->length) {
+    if (static_cast<size_t>(read_bytes) == tcp->incoming_buffer->length) {
+      finish_estimate(tcp);
+    } else if (static_cast<size_t>(read_bytes) < tcp->incoming_buffer->length) {
       grpc_slice_buffer_trim_end(
           tcp->incoming_buffer,
           tcp->incoming_buffer->length - static_cast<size_t>(read_bytes),
@@ -498,7 +500,7 @@ static void tcp_read_allocation_done(void* tcpp, grpc_error* error) {
 
 static void tcp_continue_read(grpc_tcp* tcp) {
   size_t target_read_size = get_target_read_size(tcp);
-  if (tcp->incoming_buffer->length < target_read_size &&
+  if (tcp->incoming_buffer->length < target_read_size / 2 &&
       tcp->incoming_buffer->count < MAX_READ_IOVEC) {
     if (grpc_tcp_trace.enabled()) {
       gpr_log(GPR_INFO, "TCP:%p alloc_slices", tcp);
diff --git a/src/core/lib/iomgr/wakeup_fd_eventfd.cc b/src/core/lib/iomgr/wakeup_fd_eventfd.cc
index dcf7dab71f..d68c9ada1f 100644
--- a/src/core/lib/iomgr/wakeup_fd_eventfd.cc
+++ b/src/core/lib/iomgr/wakeup_fd_eventfd.cc
@@ -32,12 +32,11 @@
 #include "src/core/lib/profiling/timers.h"
 
 static grpc_error* eventfd_create(grpc_wakeup_fd* fd_info) {
-  int efd = eventfd(0, EFD_NONBLOCK | EFD_CLOEXEC);
-  if (efd < 0) {
+  fd_info->read_fd = eventfd(0, EFD_NONBLOCK | EFD_CLOEXEC);
+  fd_info->write_fd = -1;
+  if (fd_info->read_fd < 0) {
     return GRPC_OS_ERROR(errno, "eventfd");
   }
-  fd_info->read_fd = efd;
-  fd_info->write_fd = -1;
   return GRPC_ERROR_NONE;
 }
 
diff --git a/src/core/lib/security/credentials/plugin/plugin_credentials.cc b/src/core/lib/security/credentials/plugin/plugin_credentials.cc
index 73946ce039..4015124298 100644
--- a/src/core/lib/security/credentials/plugin/plugin_credentials.cc
+++ b/src/core/lib/security/credentials/plugin/plugin_credentials.cc
@@ -102,8 +102,7 @@ static grpc_error* process_plugin_result(
     } else {
       for (size_t i = 0; i < num_md; ++i) {
         grpc_mdelem mdelem =
-            grpc_mdelem_from_slices(grpc_slice_ref_internal(md[i].key),
-                                    grpc_slice_ref_internal(md[i].value));
+            grpc_mdelem_create(md[i].key, md[i].value, nullptr);
         grpc_credentials_mdelem_array_add(r->md_array, mdelem);
         GRPC_MDELEM_UNREF(mdelem);
       }
diff --git a/src/core/lib/surface/channel.cc b/src/core/lib/surface/channel.cc
index 4294434504..d7095c24d4 100644
--- a/src/core/lib/surface/channel.cc
+++ b/src/core/lib/surface/channel.cc
@@ -336,9 +336,8 @@ grpc_call* grpc_channel_create_call(grpc_channel* channel,
   grpc_core::ExecCtx exec_ctx;
   grpc_call* call = grpc_channel_create_call_internal(
       channel, parent_call, propagation_mask, cq, nullptr,
-      grpc_mdelem_from_slices(GRPC_MDSTR_PATH, grpc_slice_ref_internal(method)),
-      host != nullptr ? grpc_mdelem_from_slices(GRPC_MDSTR_AUTHORITY,
-                                                grpc_slice_ref_internal(*host))
+      grpc_mdelem_create(GRPC_MDSTR_PATH, method, nullptr),
+      host != nullptr ? grpc_mdelem_create(GRPC_MDSTR_AUTHORITY, *host, nullptr)
                       : GRPC_MDNULL,
       grpc_timespec_to_millis_round_up(deadline));
 
@@ -347,14 +346,13 @@ grpc_call* grpc_channel_create_call(grpc_channel* channel,
 
 grpc_call* grpc_channel_create_pollset_set_call(
     grpc_channel* channel, grpc_call* parent_call, uint32_t propagation_mask,
-    grpc_pollset_set* pollset_set, grpc_slice method, const grpc_slice* host,
-    grpc_millis deadline, void* reserved) {
+    grpc_pollset_set* pollset_set, const grpc_slice& method,
+    const grpc_slice* host, grpc_millis deadline, void* reserved) {
   GPR_ASSERT(!reserved);
   return grpc_channel_create_call_internal(
       channel, parent_call, propagation_mask, nullptr, pollset_set,
-      grpc_mdelem_from_slices(GRPC_MDSTR_PATH, grpc_slice_ref_internal(method)),
-      host != nullptr ? grpc_mdelem_from_slices(GRPC_MDSTR_AUTHORITY,
-                                                grpc_slice_ref_internal(*host))
+      grpc_mdelem_create(GRPC_MDSTR_PATH, method, nullptr),
+      host != nullptr ? grpc_mdelem_create(GRPC_MDSTR_AUTHORITY, *host, nullptr)
                       : GRPC_MDNULL,
       deadline);
 }
diff --git a/src/core/lib/surface/channel.h b/src/core/lib/surface/channel.h
index e5ff2c3596..4ac76b8a29 100644
--- a/src/core/lib/surface/channel.h
+++ b/src/core/lib/surface/channel.h
@@ -45,8 +45,8 @@ grpc_channel* grpc_channel_create_with_builder(
     value of \a propagation_mask (see propagation_bits.h for possible values) */
 grpc_call* grpc_channel_create_pollset_set_call(
     grpc_channel* channel, grpc_call* parent_call, uint32_t propagation_mask,
-    grpc_pollset_set* pollset_set, grpc_slice method, const grpc_slice* host,
-    grpc_millis deadline, void* reserved);
+    grpc_pollset_set* pollset_set, const grpc_slice& method,
+    const grpc_slice* host, grpc_millis deadline, void* reserved);
 
 /** Get a (borrowed) pointer to this channels underlying channel stack */
 grpc_channel_stack* grpc_channel_get_channel_stack(grpc_channel* channel);
diff --git a/src/core/lib/transport/metadata.cc b/src/core/lib/transport/metadata.cc
index d164502280..60af22393e 100644
--- a/src/core/lib/transport/metadata.cc
+++ b/src/core/lib/transport/metadata.cc
@@ -237,7 +237,7 @@ static void rehash_mdtab(mdtab_shard* shard) {
 }
 
 grpc_mdelem grpc_mdelem_create(
-    grpc_slice key, grpc_slice value,
+    const grpc_slice& key, const grpc_slice& value,
     grpc_mdelem_data* compatible_external_backing_store) {
   if (!grpc_slice_is_interned(key) || !grpc_slice_is_interned(value)) {
     if (compatible_external_backing_store != nullptr) {
@@ -324,7 +324,8 @@ grpc_mdelem grpc_mdelem_create(
   return GRPC_MAKE_MDELEM(md, GRPC_MDELEM_STORAGE_INTERNED);
 }
 
-grpc_mdelem grpc_mdelem_from_slices(grpc_slice key, grpc_slice value) {
+grpc_mdelem grpc_mdelem_from_slices(const grpc_slice& key,
+                                    const grpc_slice& value) {
   grpc_mdelem out = grpc_mdelem_create(key, value, nullptr);
   grpc_slice_unref_internal(key);
   grpc_slice_unref_internal(value);
diff --git a/src/core/lib/transport/metadata.h b/src/core/lib/transport/metadata.h
index 338082276c..989c7544c1 100644
--- a/src/core/lib/transport/metadata.h
+++ b/src/core/lib/transport/metadata.h
@@ -109,7 +109,8 @@ struct grpc_mdelem {
                               (uintptr_t)GRPC_MDELEM_STORAGE_INTERNED_BIT))
 
 /* Unrefs the slices. */
-grpc_mdelem grpc_mdelem_from_slices(grpc_slice key, grpc_slice value);
+grpc_mdelem grpc_mdelem_from_slices(const grpc_slice& key,
+                                    const grpc_slice& value);
 
 /* Cheaply convert a grpc_metadata to a grpc_mdelem; may use the grpc_metadata
    object as backing storage (so lifetimes should align) */
@@ -120,7 +121,7 @@ grpc_mdelem grpc_mdelem_from_grpc_metadata(grpc_metadata* metadata);
    compatible_external_backing_store if it is non-NULL (in which case it's the
    users responsibility to ensure that it outlives usage) */
 grpc_mdelem grpc_mdelem_create(
-    grpc_slice key, grpc_slice value,
+    const grpc_slice& key, const grpc_slice& value,
     grpc_mdelem_data* compatible_external_backing_store);
 
 bool grpc_mdelem_eq(grpc_mdelem a, grpc_mdelem b);
diff --git a/src/csharp/Grpc.Core/Internal/UnmanagedLibrary.cs b/src/csharp/Grpc.Core/Internal/UnmanagedLibrary.cs
index 7185d68efe..1786fc2e3f 100644
--- a/src/csharp/Grpc.Core/Internal/UnmanagedLibrary.cs
+++ b/src/csharp/Grpc.Core/Internal/UnmanagedLibrary.cs
@@ -51,11 +51,12 @@ namespace Grpc.Core.Internal
 
             Logger.Debug("Attempting to load native library \"{0}\"", this.libraryPath);
 
-            this.handle = PlatformSpecificLoadLibrary(this.libraryPath);
+            this.handle = PlatformSpecificLoadLibrary(this.libraryPath, out string loadLibraryErrorDetail);
 
             if (this.handle == IntPtr.Zero)
             {
-                throw new IOException(string.Format("Error loading native library \"{0}\"", this.libraryPath));
+                throw new IOException(string.Format("Error loading native library \"{0}\". {1}",
+                                                    this.libraryPath, loadLibraryErrorDetail));
             }
         }
 
@@ -129,31 +130,44 @@ namespace Grpc.Core.Internal
         /// <summary>
         /// Loads library in a platform specific way.
         /// </summary>
-        private static IntPtr PlatformSpecificLoadLibrary(string libraryPath)
+        private static IntPtr PlatformSpecificLoadLibrary(string libraryPath, out string errorMsg)
         {
             if (PlatformApis.IsWindows)
             {
+                // TODO(jtattermusch): populate the error on Windows
+                errorMsg = null;
                 return Windows.LoadLibrary(libraryPath);
             }
             if (PlatformApis.IsLinux)
             {
                 if (PlatformApis.IsMono)
                 {
-                    return Mono.dlopen(libraryPath, RTLD_GLOBAL + RTLD_LAZY);
+                    return LoadLibraryPosix(Mono.dlopen, Mono.dlerror, libraryPath, out errorMsg);
                 }
                 if (PlatformApis.IsNetCore)
                 {
-                    return CoreCLR.dlopen(libraryPath, RTLD_GLOBAL + RTLD_LAZY);
+                    return LoadLibraryPosix(CoreCLR.dlopen, CoreCLR.dlerror, libraryPath, out errorMsg);
                 }
-                return Linux.dlopen(libraryPath, RTLD_GLOBAL + RTLD_LAZY);
+                return LoadLibraryPosix(Linux.dlopen, Linux.dlerror, libraryPath, out errorMsg);
             }
             if (PlatformApis.IsMacOSX)
             {
-                return MacOSX.dlopen(libraryPath, RTLD_GLOBAL + RTLD_LAZY);
+                return LoadLibraryPosix(MacOSX.dlopen, MacOSX.dlerror, libraryPath, out errorMsg);
             }
             throw new InvalidOperationException("Unsupported platform.");
         }
 
+        private static IntPtr LoadLibraryPosix(Func<string, int, IntPtr> dlopenFunc, Func<IntPtr> dlerrorFunc, string libraryPath, out string errorMsg)
+        {
+            errorMsg = null;
+            IntPtr ret = dlopenFunc(libraryPath, RTLD_GLOBAL + RTLD_LAZY);
+            if (ret == IntPtr.Zero)
+            {
+                errorMsg = Marshal.PtrToStringAnsi(dlerrorFunc());
+            }
+            return ret;
+        }
+
         private static string FirstValidLibraryPath(string[] libraryPathAlternatives)
         {
             GrpcPreconditions.CheckArgument(libraryPathAlternatives.Length > 0, "libraryPathAlternatives cannot be empty.");
@@ -184,6 +198,9 @@ namespace Grpc.Core.Internal
             internal static extern IntPtr dlopen(string filename, int flags);
 
             [DllImport("libdl.so")]
+            internal static extern IntPtr dlerror();
+
+            [DllImport("libdl.so")]
             internal static extern IntPtr dlsym(IntPtr handle, string symbol);
         }
 
@@ -193,6 +210,9 @@ namespace Grpc.Core.Internal
             internal static extern IntPtr dlopen(string filename, int flags);
 
             [DllImport("libSystem.dylib")]
+            internal static extern IntPtr dlerror();
+
+            [DllImport("libSystem.dylib")]
             internal static extern IntPtr dlsym(IntPtr handle, string symbol);
         }
 
@@ -209,6 +229,9 @@ namespace Grpc.Core.Internal
             internal static extern IntPtr dlopen(string filename, int flags);
 
             [DllImport("__Internal")]
+            internal static extern IntPtr dlerror();
+
+            [DllImport("__Internal")]
             internal static extern IntPtr dlsym(IntPtr handle, string symbol);
         }
 
@@ -223,6 +246,9 @@ namespace Grpc.Core.Internal
             internal static extern IntPtr dlopen(string filename, int flags);
 
             [DllImport("libcoreclr.so")]
+            internal static extern IntPtr dlerror();
+
+            [DllImport("libcoreclr.so")]
             internal static extern IntPtr dlsym(IntPtr handle, string symbol);
         }
     }
diff --git a/src/proto/grpc/reflection/v1alpha/BUILD b/src/proto/grpc/reflection/v1alpha/BUILD
index 4d919d029e..4605418447 100644
--- a/src/proto/grpc/reflection/v1alpha/BUILD
+++ b/src/proto/grpc/reflection/v1alpha/BUILD
@@ -22,11 +22,3 @@ grpc_proto_library(
     name = "reflection_proto",
     srcs = ["reflection.proto"],
 )
-
-filegroup(
-    name = "reflection_proto_file",
-    srcs = [
-        "reflection.proto",
-    ],
-)
-
diff --git a/src/proto/grpc/testing/BUILD b/src/proto/grpc/testing/BUILD
index 7048911b9a..16721ff2ed 100644
--- a/src/proto/grpc/testing/BUILD
+++ b/src/proto/grpc/testing/BUILD
@@ -15,8 +15,6 @@
 licenses(["notice"])  # Apache v2
 
 load("//bazel:grpc_build_system.bzl", "grpc_proto_library", "grpc_package")
-load("@grpc_python_dependencies//:requirements.bzl", "requirement")
-load("@org_pubref_rules_protobuf//python:rules.bzl", "py_proto_library")
 
 grpc_package(name = "testing", visibility = "public")
 
@@ -60,30 +58,12 @@ grpc_proto_library(
     has_services = False,
 )
 
-py_proto_library(
-    name = "py_empty_proto",
-    protos = ["empty.proto",],
-    with_grpc = True,
-    deps = [
-        requirement('protobuf'),
-    ],
-)
-
 grpc_proto_library(
     name = "messages_proto",
     srcs = ["messages.proto"],
     has_services = False,
 )
 
-py_proto_library(
-    name = "py_messages_proto",
-    protos = ["messages.proto",],
-    with_grpc = True,
-    deps = [
-        requirement('protobuf'),
-    ],
-)
-
 grpc_proto_library(
     name = "metrics_proto",
     srcs = ["metrics.proto"],
@@ -136,17 +116,3 @@ grpc_proto_library(
         "messages_proto",
     ],
 )
-
-py_proto_library(
-    name = "py_test_proto",
-    protos = ["test.proto",],
-    with_grpc = True,
-    deps = [
-        requirement('protobuf'),
-    ],
-    proto_deps = [
-        ":py_empty_proto",
-        ":py_messages_proto",
-    ]
-)
-
diff --git a/src/proto/grpc/testing/control.proto b/src/proto/grpc/testing/control.proto
index a4a9c8fe57..4cfdc2cafb 100644
--- a/src/proto/grpc/testing/control.proto
+++ b/src/proto/grpc/testing/control.proto
@@ -25,6 +25,7 @@ enum ClientType {
   SYNC_CLIENT = 0;
   ASYNC_CLIENT = 1;
   OTHER_CLIENT = 2; // used for some language-specific variants
+  CALLBACK_CLIENT = 3;
 }
 
 enum ServerType {
diff --git a/src/proto/grpc/testing/proto2/BUILD.bazel b/src/proto/grpc/testing/proto2/BUILD.bazel
deleted file mode 100644
index c4c4f004ef..0000000000
--- a/src/proto/grpc/testing/proto2/BUILD.bazel
+++ /dev/null
@@ -1,30 +0,0 @@
-load("@grpc_python_dependencies//:requirements.bzl", "requirement")
-load("@org_pubref_rules_protobuf//python:rules.bzl", "py_proto_library")
-
-package(default_visibility = ["//visibility:public"])
-
-py_proto_library(
-    name = "empty2_proto",
-    protos = [
-        "empty2.proto",
-    ],
-    with_grpc = True,
-    deps = [
-        requirement('protobuf'),
-    ],
-)
-
-py_proto_library(
-    name = "empty2_extensions_proto",
-    protos = [
-        "empty2_extensions.proto",
-    ],
-    proto_deps = [
-        ":empty2_proto",
-    ],
-    with_grpc = True,
-    deps = [
-        requirement('protobuf'),
-    ],
-)
-
diff --git a/src/python/grpcio/grpc/_channel.py b/src/python/grpcio/grpc/_channel.py
index 3494c9b15a..eeeb4ddb33 100644
--- a/src/python/grpcio/grpc/_channel.py
+++ b/src/python/grpcio/grpc/_channel.py
@@ -981,4 +981,7 @@ class Channel(grpc.Channel):
         # then deletion of this grpc._channel.Channel instance can be made to
         # effect closure of the underlying cygrpc.Channel instance.
         cygrpc.fork_unregister_channel(self)
-        _moot(self._connectivity_state)
+        # This prevent the failed-at-initializing object removal from failing.
+        # Though the __init__ failed, the removal will still trigger __del__.
+        if hasattr(self, "_connectivity_state"):
+            _moot(self._connectivity_state)
diff --git a/src/python/grpcio/grpc/_cython/_cygrpc/arguments.pxd.pxi b/src/python/grpcio/grpc/_cython/_cygrpc/arguments.pxd.pxi
index 6cb1bc0c05..e0e068e452 100644
--- a/src/python/grpcio/grpc/_cython/_cygrpc/arguments.pxd.pxi
+++ b/src/python/grpcio/grpc/_cython/_cygrpc/arguments.pxd.pxi
@@ -32,7 +32,7 @@ cdef class _ArgumentProcessor:
 
   cdef grpc_arg c_argument
 
-  cdef void c(self, argument, grpc_arg_pointer_vtable *vtable, references)
+  cdef void c(self, argument, grpc_arg_pointer_vtable *vtable, references) except *
 
 
 cdef class _ArgumentsProcessor:
@@ -42,5 +42,5 @@ cdef class _ArgumentsProcessor:
   cdef readonly list _references
   cdef grpc_channel_args _c_arguments
 
-  cdef grpc_channel_args *c(self, grpc_arg_pointer_vtable *vtable)
+  cdef grpc_channel_args *c(self, grpc_arg_pointer_vtable *vtable) except *
   cdef un_c(self)
diff --git a/src/python/grpcio/grpc/_cython/_cygrpc/arguments.pyx.pxi b/src/python/grpcio/grpc/_cython/_cygrpc/arguments.pyx.pxi
index 2239e26b32..b7a4277ff6 100644
--- a/src/python/grpcio/grpc/_cython/_cygrpc/arguments.pyx.pxi
+++ b/src/python/grpcio/grpc/_cython/_cygrpc/arguments.pyx.pxi
@@ -52,7 +52,7 @@ cdef grpc_arg _unwrap_grpc_arg(tuple wrapped_arg):
 
 cdef class _ArgumentProcessor:
 
-  cdef void c(self, argument, grpc_arg_pointer_vtable *vtable, references):
+  cdef void c(self, argument, grpc_arg_pointer_vtable *vtable, references) except *:
     key, value = argument
     cdef bytes encoded_key = _encode(key)
     if encoded_key is not key:
@@ -89,7 +89,7 @@ cdef class _ArgumentsProcessor:
     self._argument_processors = []
     self._references = []
 
-  cdef grpc_channel_args *c(self, grpc_arg_pointer_vtable *vtable):
+  cdef grpc_channel_args *c(self, grpc_arg_pointer_vtable *vtable) except *:
     self._c_arguments.arguments_length = len(self._arguments)
     if self._c_arguments.arguments_length == 0:
       return NULL
diff --git a/src/python/grpcio_reflection/grpc_reflection/v1alpha/BUILD.bazel b/src/python/grpcio_reflection/grpc_reflection/v1alpha/BUILD.bazel
deleted file mode 100644
index 3a2ba26371..0000000000
--- a/src/python/grpcio_reflection/grpc_reflection/v1alpha/BUILD.bazel
+++ /dev/null
@@ -1,34 +0,0 @@
-load("@grpc_python_dependencies//:requirements.bzl", "requirement")
-load("@org_pubref_rules_protobuf//python:rules.bzl", "py_proto_library")
-
-package(default_visibility = ["//visibility:public"])
-
-genrule(
-    name = "mv_reflection_proto",
-    srcs = [
-        "//src/proto/grpc/reflection/v1alpha:reflection_proto_file",
-    ],
-    outs = ["reflection.proto",],
-    cmd = "cp $< $@",
-)
-
-py_proto_library(
-    name = "py_reflection_proto",
-    protos = [":mv_reflection_proto",],
-    with_grpc = True,
-    deps = [
-        requirement('protobuf'),
-    ],
-)
-
-py_library(
-    name = "grpc_reflection",
-    srcs = ["reflection.py",],
-    deps = [
-        ":py_reflection_proto",
-        "//src/python/grpcio/grpc:grpcio",
-        requirement('protobuf'),
-    ],
-    imports=["../../",],
-)
-
diff --git a/src/python/grpcio_tests/commands.py b/src/python/grpcio_tests/commands.py
index 0dfbf3180b..6931d93ef0 100644
--- a/src/python/grpcio_tests/commands.py
+++ b/src/python/grpcio_tests/commands.py
@@ -116,6 +116,8 @@ class TestGevent(setuptools.Command):
         # eventually succeed, but need to dig into performance issues.
         'unit._cython._no_messages_server_completion_queue_per_call_test.Test.test_rpcs',
         'unit._cython._no_messages_single_server_completion_queue_test.Test.test_rpcs',
+        # TODO(https://github.com/grpc/grpc/issues/16890) enable this test
+        'unit._cython._channel_test.ChannelTest.test_multiple_channels_lonely_connectivity',
         # I have no idea why this doesn't work in gevent, but it shouldn't even be
         # using the c-core
         'testing._client_test.ClientTest.test_infinite_request_stream_real_time',
diff --git a/src/python/grpcio_tests/tests/interop/BUILD.bazel b/src/python/grpcio_tests/tests/interop/BUILD.bazel
deleted file mode 100644
index a39f30d32a..0000000000
--- a/src/python/grpcio_tests/tests/interop/BUILD.bazel
+++ /dev/null
@@ -1,97 +0,0 @@
-load("@grpc_python_dependencies//:requirements.bzl", "requirement")
-
-package(default_visibility = ["//visibility:public"])
-
-py_library(
-    name = "_intraop_test_case",
-    srcs = ["_intraop_test_case.py"],
-    deps = [
-        ":methods",
-    ],
-    imports=["../../",],
-)
-
-py_library(
-    name = "client",
-    srcs = ["client.py"],
-    deps = [
-        "//src/python/grpcio/grpc:grpcio",
-        ":methods",
-        ":resources",
-        "//src/proto/grpc/testing:py_test_proto",
-        requirement('google-auth'),
-    ],
-    imports=["../../",],
-)
-
-py_library(
-    name = "methods",
-    srcs = ["methods.py"],
-    deps = [
-        "//src/python/grpcio/grpc:grpcio",
-        "//src/proto/grpc/testing:py_empty_proto",
-        "//src/proto/grpc/testing:py_messages_proto",
-        "//src/proto/grpc/testing:py_test_proto",
-        requirement('google-auth'),
-        requirement('requests'),
-        requirement('enum34'),
-    ],
-    imports=["../../",],
-)
-
-py_library(
-    name = "resources",
-    srcs = ["resources.py"],
-    data = [
-        "//src/python/grpcio_tests/tests/interop/credentials",
-    ],
-)
-
-py_library(
-    name = "server",
-    srcs = ["server.py"],
-    deps = [
-        "//src/python/grpcio/grpc:grpcio",
-        ":methods",
-        ":resources",
-        "//src/python/grpcio_tests/tests/unit:test_common",
-        "//src/proto/grpc/testing:py_test_proto",
-    ],
-    imports=["../../",],
-)
-
-py_test(
-    name="_insecure_intraop_test",
-    size="small",
-    srcs=["_insecure_intraop_test.py",],
-    main="_insecure_intraop_test.py",
-    deps=[
-        "//src/python/grpcio/grpc:grpcio",
-        ":_intraop_test_case",
-        ":methods",
-        ":server",
-        "//src/python/grpcio_tests/tests/unit:test_common",
-        "//src/proto/grpc/testing:py_test_proto",
-    ],
-    imports=["../../",],
-    data=[
-        "//src/python/grpcio_tests/tests/unit/credentials",
-    ],
-)
-
-py_test(
-    name="_secure_intraop_test",
-    size="small",
-    srcs=["_secure_intraop_test.py",],
-    main="_secure_intraop_test.py",
-    deps=[
-        "//src/python/grpcio/grpc:grpcio",
-        ":_intraop_test_case",
-        ":methods",
-        ":server",
-        "//src/python/grpcio_tests/tests/unit:test_common",
-        "//src/proto/grpc/testing:py_test_proto",
-    ],
-    imports=["../../",],
-)
-
diff --git a/src/python/grpcio_tests/tests/interop/credentials/BUILD.bazel b/src/python/grpcio_tests/tests/interop/credentials/BUILD.bazel
deleted file mode 100644
index bc2b997292..0000000000
--- a/src/python/grpcio_tests/tests/interop/credentials/BUILD.bazel
+++ /dev/null
@@ -1,9 +0,0 @@
-package(default_visibility = ["//visibility:public"])
-
-filegroup(
-    name="credentials",
-    srcs=glob([
-        "**",
-    ]),
-)
-
diff --git a/src/python/grpcio_tests/tests/reflection/BUILD.bazel b/src/python/grpcio_tests/tests/reflection/BUILD.bazel
deleted file mode 100644
index c0efb0b7ce..0000000000
--- a/src/python/grpcio_tests/tests/reflection/BUILD.bazel
+++ /dev/null
@@ -1,21 +0,0 @@
-load("@grpc_python_dependencies//:requirements.bzl", "requirement")
-
-package(default_visibility = ["//visibility:public"])
-
-py_test(
-    name="_reflection_servicer_test",
-    size="small",
-    timeout="moderate",
-    srcs=["_reflection_servicer_test.py",],
-    main="_reflection_servicer_test.py",
-    deps=[
-        "//src/python/grpcio/grpc:grpcio",
-        "//src/python/grpcio_reflection/grpc_reflection/v1alpha:grpc_reflection",
-        "//src/python/grpcio_tests/tests/unit:test_common",
-        "//src/proto/grpc/testing:py_empty_proto",
-        "//src/proto/grpc/testing/proto2:empty2_extensions_proto",
-        requirement('protobuf'),
-    ],
-    imports=["../../",],
-)
-
diff --git a/src/python/grpcio_tests/tests/unit/_channel_args_test.py b/src/python/grpcio_tests/tests/unit/_channel_args_test.py
index 869c2f4d2f..dd1d2969a2 100644
--- a/src/python/grpcio_tests/tests/unit/_channel_args_test.py
+++ b/src/python/grpcio_tests/tests/unit/_channel_args_test.py
@@ -33,6 +33,14 @@ TEST_CHANNEL_ARGS = (
     ('arg6', TestPointerWrapper()),
 )
 
+INVALID_TEST_CHANNEL_ARGS = [
+    {
+        'foo': 'bar'
+    },
+    (('key',),),
+    'str',
+]
+
 
 class ChannelArgsTest(unittest.TestCase):
 
@@ -44,6 +52,14 @@ class ChannelArgsTest(unittest.TestCase):
             futures.ThreadPoolExecutor(max_workers=1),
             options=TEST_CHANNEL_ARGS)
 
+    def test_invalid_client_args(self):
+        for invalid_arg in INVALID_TEST_CHANNEL_ARGS:
+            self.assertRaises(
+                ValueError,
+                grpc.insecure_channel,
+                'localhost:8080',
+                options=invalid_arg)
+
 
 if __name__ == '__main__':
     unittest.main(verbosity=2)