Commentary

4 files changed, 28 insertions, 13 deletions

diff --git a/src/core/channel/channel_stack.h b/src/core/channel/channel_stack.h
index 1279fec080..5d33ab5b42 100644
--- a/src/core/channel/channel_stack.h
+++ b/src/core/channel/channel_stack.h
@@ -214,7 +214,8 @@ void grpc_call_stack_set_pollset(grpc_exec_ctx *exec_ctx,
 /* Destroy a call stack */
 void grpc_call_stack_destroy(grpc_exec_ctx *exec_ctx, grpc_call_stack *stack);
 
-/* Ignore set pollset */
+/* Ignore set pollset - used by filters to implement the set_pollset method
+   if they don't care about pollsets at all. Does nothing. */
 void grpc_call_stack_ignore_set_pollset(grpc_exec_ctx *exec_ctx,
                                         grpc_call_element *elem,
                                         grpc_pollset *pollset);
diff --git a/src/core/channel/subchannel_call_holder.h b/src/core/channel/subchannel_call_holder.h
index 3dd43c9c3f..bda051c566 100644
--- a/src/core/channel/subchannel_call_holder.h
+++ b/src/core/channel/subchannel_call_holder.h
@@ -36,6 +36,10 @@
 
 #include "src/core/client_config/subchannel.h"
 
+/** Pick a subchannel for grpc_subchannel_call_holder;
+    Return 1 if subchannel is available immediately (in which case on_ready
+    should not be called), or 0 otherwise (in which case on_ready should be
+    called when the subchannel is available) */
 typedef int (*grpc_subchannel_call_holder_pick_subchannel)(
     grpc_exec_ctx *exec_ctx, void *arg, grpc_metadata_batch *initial_metadata,
     grpc_subchannel **subchannel, grpc_closure *on_ready);
@@ -46,10 +50,21 @@ typedef enum {
   GRPC_SUBCHANNEL_CALL_HOLDER_CREATING_CALL
 } grpc_subchannel_call_holder_creation_phase;
 
+/** Wrapper for holding a pointer to grpc_subchannel_call, and the
+    associated machinery to create such a pointer.
+    Handles queueing of stream ops until a call object is ready, waiting
+    for initial metadata before trying to create a call object,
+    and handling cancellation gracefully.
+
+    Both the channel and uchannel filter use this as their call_data. */
 typedef struct grpc_subchannel_call_holder {
-  /* either 0 for no call, 1 for cancelled, or a pointer to a
-     grpc_subchannel_call */
+  /** either 0 for no call, 1 for cancelled, or a pointer to a
+      grpc_subchannel_call */
   gpr_atm subchannel_call;
+  /** Helper function to choose the subchannel on which to create
+      the call object. Channel filter delegates to the load
+      balancing policy (once it's ready); uchannel returns
+      immediately */
   grpc_subchannel_call_holder_pick_subchannel pick_subchannel;
   void *pick_subchannel_arg;
 
diff --git a/src/core/client_config/subchannel.c b/src/core/client_config/subchannel.c
index e911a46faf..49c2cf9a19 100644
--- a/src/core/client_config/subchannel.c
+++ b/src/core/client_config/subchannel.c
@@ -22,7 +22,7 @@
  * LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR
  * A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT
  * OWNER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL,
- * SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMA`S (INCLUDING, BUT NOT
+ * SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT
  * LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE,
  * DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY
  * THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT
@@ -395,12 +395,12 @@ static void start_connect(grpc_exec_ctx *exec_ctx, grpc_subchannel *c) {
 
 static void continue_creating_call(grpc_exec_ctx *exec_ctx, void *arg,
                                    int iomgr_success) {
-  int call_creation_status;
+  int call_creation_finished_ok;
   waiting_for_connect *w4c = arg;
   grpc_subchannel_del_interested_party(exec_ctx, w4c->subchannel, w4c->pollset);
-  call_creation_status = grpc_subchannel_create_call(
+  call_creation_finished_ok = grpc_subchannel_create_call(
       exec_ctx, w4c->subchannel, w4c->pollset, w4c->target, w4c->notify);
-  GPR_ASSERT(call_creation_status == 1);
+  GPR_ASSERT(call_creation_finished_ok == 1);
   w4c->notify->cb(exec_ctx, w4c->notify->cb_arg, iomgr_success);
   GRPC_SUBCHANNEL_UNREF(exec_ctx, w4c->subchannel, "waiting_for_connect");
   gpr_free(w4c);
diff --git a/src/core/client_config/subchannel.h b/src/core/client_config/subchannel.h
index 4769a6de68..1fefa1888a 100644
--- a/src/core/client_config/subchannel.h
+++ b/src/core/client_config/subchannel.h
@@ -77,12 +77,11 @@ void grpc_subchannel_call_unref(grpc_exec_ctx *exec_ctx,
 
 /** construct a subchannel call (possibly asynchronously).
  *
- * If the returned status is \a GRPC_SUBCHANNEL_CALL_CREATE_READY, the call will
- * return immediately and \a target will point to a connected \a subchannel_call
- * instance. Note that \a notify will \em not be invoked in this case.
- * Otherwise, if the returned status is GRPC_SUBCHANNEL_CALL_CREATE_PENDING, the
- * subchannel call will be created asynchronously, invoking the \a notify
- * callback upon completion. */
+ * If the returned status is 1, the call will return immediately and \a target 
+ * will point to a connected \a subchannel_call instance. Note that \a notify 
+ * will \em not be invoked in this case.
+ * Otherwise, if the returned status is 0, the subchannel call will be created 
+ * asynchronously, invoking the \a notify callback upon completion. */
 int grpc_subchannel_create_call(grpc_exec_ctx *exec_ctx,
                                 grpc_subchannel *subchannel,
                                 grpc_pollset *pollset, gpr_atm *target,