Merge github.com:grpc/grpc into atomic-timers

3 files changed, 394 insertions, 0 deletions

diff --git a/doc/combiner-explainer.md b/doc/combiner-explainer.md
new file mode 100644
index 0000000000..9e9d077273
--- /dev/null
+++ b/doc/combiner-explainer.md
@@ -0,0 +1,158 @@
+# Combiner Explanation
+## Talk by ctiller, notes by vjpai
+
+Typical way of doing critical section
+
+```
+mu.lock()
+do_stuff()
+mu.unlock()
+```
+
+An alternative way of doing it is
+
+```
+class combiner {
+  run(f) {
+    mu.lock()
+    f()
+    mu.unlock()
+  }
+  mutex mu;
+}
+
+combiner.run(do_stuff)
+```
+
+If you have two threads calling combiner, there will be some kind of
+queuing in place. It's called `combiner` because you can pass in more
+than one do_stuff at once and they will run under a common `mu`.
+
+The implementation described above has the issue that you're blocking a thread
+for a period of time, and this is considered harmful because it's an application thread that you're blocking.
+
+Instead, get a new property:
+* Keep things running in serial execution
+* Don't ever sleep the thread
+* But maybe allow things to end up running on a different thread from where they were started
+* This means that `do_stuff` doesn't necessarily run to completion when `combiner.run` is invoked
+
+```
+class combiner {
+  mpscq q; // multi-producer single-consumer queue can be made non-blocking
+  state s; // is it empty or executing
+  
+  run(f) {
+    if (q.push(f)) { 
+      // q.push returns true if it's the first thing
+      while (q.pop(&f)) { // modulo some extra work to avoid races
+        f();
+      }
+    }
+  }
+}
+```
+
+The basic idea is that the first one to push onto the combiner
+executes the work and then keeps executing functions from the queue
+until the combiner is drained.
+
+Our combiner does some additional work, with the motivation of write-batching.
+
+We have a second tier of `run` called `run_finally`. Anything queued
+onto `run_finally` runs after we have drained the queue. That means
+that there is essentially a finally-queue. This is not guaranteed to
+be final, but it's best-effort. In the process of running the finally
+item, we might put something onto the main combiner queue and so we'll
+need to re-enter.
+
+`chttp2` runs all ops in the run state except if it sees a write it puts that into a finally. That way anything else that gets put into the combiner can add to that write.
+
+```
+class combiner {
+  mpscq q; // multi-producer single-consumer queue can be made non-blocking
+  state s; // is it empty or executing
+  queue finally; // you can only do run_finally when you are already running something from the combiner
+  
+  run(f) {
+    if (q.push(f)) { 
+      // q.push returns true if it's the first thing
+      loop:
+      while (q.pop(&f)) { // modulo some extra work to avoid races
+        f();
+      }
+      while (finally.pop(&f)) {
+        f();
+      }
+      goto loop;
+    }
+  }
+}
+```
+
+So that explains how combiners work in general. In gRPC, there is
+`start_batch(..., tag)` and then work only gets activated by somebody
+calling `cq::next` which returns a tag. This gives an API-level
+guarantee that there will be a thread doing polling to actually make
+work happen. However, some operations are not covered by a poller
+thread, such as cancellation that doesn't have a completion. Other
+callbacks that don't have a completion are the internal work that gets
+done before the batch gets completed. We need a condition called
+`covered_by_poller` that means that the item will definitely need some
+thread at some point to call `cq::next` . This includes those
+callbacks that directly cause a completion but also those that are
+indirectly required before getting a completion. If we can't tell for
+sure for a specific path, we have to assumed it is not covered by
+poller.
+
+The above combiner has the problem that it keeps draining for a
+potentially infinite amount of time and that can lead to a huge tail
+latency for some operations. So we can tweak it by returning to the application
+if we know that it is valid to do so:
+
+```
+while (q.pop(&f)) {
+  f();
+  if (control_can_be_returned && some_still_queued_thing_is_covered_by_poller) {
+    offload_combiner_work_to_some_other_thread();
+  }
+}
+```
+
+`offload` is more than `break`; it does `break` but also causes some
+other thread that is currently waiting on a poll to break out of its
+poll. This is done by setting up a per-polling-island work-queue
+(distributor) wakeup FD. The work-queue is the converse of the combiner; it
+tries to spray events onto as many threads as possible to get as much concurrency as possible.
+
+So `offload` really does:
+
+``` 
+  workqueue.run(continue_from_while_loop);
+  break;
+```
+
+This needs us to add another class variable for a `workqueue`
+(which is really conceptually a distributor).
+
+```
+workqueue::run(f) {
+  q.push(f)
+  eventfd.wakeup()
+}
+
+workqueue::readable() {
+  eventfd.consume();
+  q.pop(&f);
+  f();
+  if (!q.empty()) {
+    eventfd.wakeup(); // spray across as many threads as are waiting on this workqueue
+  }
+}
+```
+
+In principle, `run_finally` could get starved, but this hasn't
+happened in practice. If we were concerned about this, we could put a
+limit on how many things come off the regular `q` before the `finally`
+queue gets processed.
+
diff --git a/doc/core/grpc-error.md b/doc/core/grpc-error.md
new file mode 100644
index 0000000000..c05d1dd909
--- /dev/null
+++ b/doc/core/grpc-error.md
@@ -0,0 +1,160 @@
+# gRPC Error
+
+## Background
+
+`grpc_error` is the c-core's opaque representation of an error. It holds a
+collection of integers, strings, timestamps, and child errors that related to
+the final error.
+
+always present are:
+
+*   GRPC_ERROR_STR_FILE and GRPC_ERROR_INT_FILE_LINE - the source location where
+    the error was generated
+*   GRPC_ERROR_STR_DESCRIPTION - a human readable description of the error
+*   GRPC_ERROR_TIME_CREATED - a timestamp indicating when the error happened
+
+An error can also have children; these are other errors that are believed to
+have contributed to this one. By accumulating children, we can begin to root
+cause high level failures from low level failures, without having to derive
+execution paths from log lines.
+
+grpc_errors are refcounted objects, which means they need strict ownership
+semantics. An extra ref on an error can cause a memory leak, and a missing ref
+can cause a crash.
+
+This document serves as a detailed overview of grpc_error's ownership rules. It
+should help people use the errors, as well as help people debug refcount related
+errors.
+
+## Clarification of Ownership
+
+If a particular function is said to "own" an error, that means it has the
+responsibility of calling unref on the error. A function may have access to an
+error without ownership of it.
+
+This means the function may use the error, but must not call unref on it, since
+that will be done elsewhere in the code. A function that does not own an error
+may explicitly take ownership of it by manually calling GRPC_ERROR_REF.
+
+## Ownership Rules
+
+There are three rules of error ownership, which we will go over in detail.
+
+*   If `grpc_error` is returned by a function, the caller owns a ref to that
+    instance.
+*   If a `grpc_error` is passed to a `grpc_closure` callback function, then that
+    function does not own a ref to the error.
+*   if a `grpc_error` is passed to *any other function*, then that function
+    takes ownership of the error.
+
+### Rule 1
+
+> If `grpc_error` is returned by a function, the caller owns a ref to that
+> instance.*
+
+For example, in the following code block, error1 and error2 are owned by the
+current function.
+
+```C
+grpc_error* error1 = GRPC_ERROR_CREATE_FROM_STATIC_STRING("Some error occured");
+grpc_error* error2 = some_operation_that_might_fail(...);
+```
+
+The current function would have to explicitly call GRPC_ERROR_UNREF on the
+errors, or pass them along to a function that would take over the ownership.
+
+### Rule 2
+
+> If a `grpc_error` is passed to a `grpc_closure` callback function, then that
+> function does not own a ref to the error.
+
+A `grpc_closure` callback function is any function that has the signature:
+
+```C
+void (*cb)(grpc_exec_ctx *exec_ctx, void *arg, grpc_error *error);
+```
+
+This means that the error ownership is NOT transferred when a functions calls:
+
+```C
+c->cb(exec_ctx, c->cb_arg, err);
+```
+
+The caller is still responsible for unref-ing the error.
+
+However, the above line is currently being phased out! It is safer to invoke
+callbacks with `grpc_closure_run` and `grpc_closure_sched`. These functions are
+not callbacks, so they will take ownership of the error passed to them.
+
+```C
+grpc_error* error = GRPC_ERROR_CREATE_FROM_STATIC_STRING("Some error occured");
+grpc_closure_run(exec_ctx, cb, error);
+// current function no longer has ownership of the error
+```
+
+If you schedule or run a closure, but still need ownership of the error, then
+you must explicitly take a reference.
+
+```C
+grpc_error* error = GRPC_ERROR_CREATE_FROM_STATIC_STRING("Some error occured");
+grpc_closure_run(exec_ctx, cb, GRPC_ERROR_REF(error));
+// do some other things with the error
+GRPC_ERROR_UNREF(error);
+```
+
+Rule 2 is more important to keep in mind when **implementing** `grpc_closure`
+callback functions. You must keep in mind that you do not own the error, and
+must not unref it. More importantly, you cannot pass it to any function that
+would take ownership of the error, without explicitly taking ownership yourself.
+For example:
+
+```C
+void on_some_action(grpc_exec_ctx *exec_ctx, void *arg, grpc_error *error) {
+  // this would cause a crash, because some_function will unref the error,
+  // and the caller of this callback will also unref it.
+  some_function(error);
+
+  // this callback function must take ownership, so it can give that
+  // ownership to the function it is calling.
+  some_function(GRPC_ERROR_REF(error));
+}
+```
+
+### Rule 3
+
+> if a `grpc_error` is passed to *any other function*, then that function takes
+> ownership of the error.
+
+Take the following example:
+
+```C
+grpc_error* error = GRPC_ERROR_CREATE_FROM_STATIC_STRING("Some error occured");
+// do some things
+some_function(error);
+// can't use error anymore! might be gone.
+```
+
+When some_function is called, it takes over the ownership of the error, and it
+will eventually unref it. So the caller can no longer safely use the error.
+
+If the caller needed to keep using the error (or passing it to other functions),
+if would have to take on a reference to it. This is a common pattern seen.
+
+```C
+void func() {
+  grpc_error* error = GRPC_ERROR_CREATE_FROM_STATIC_STRING("Some error");
+  some_function(GRPC_ERROR_REF(error));
+  // do things
+  some_other_function(GRPC_ERROR_REF(error));
+  // do more things
+  some_last_function(error);
+}
+```
+
+The last call takes ownership and will eventually give the error its final
+unref.
+
+When **implementing** a function that takes an error (and is not a
+`grpc_closure` callback function), you must ensure the error is unref-ed either
+by doing it explicitly with GRPC_ERROR_UNREF, or by passing the error to a
+function that takes over the ownership.
diff --git a/doc/negative-http2-interop-test-descriptions.md b/doc/http2-interop-test-descriptions.md
index b64fe6a024..435a8de709 100644
--- a/doc/negative-http2-interop-test-descriptions.md
+++ b/doc/http2-interop-test-descriptions.md
@@ -193,3 +193,79 @@ Server Procedure:
   1. Sets MAX_CONCURRENT_STREAMS to one after the connection is made.
 
 *The assertion that the MAX_CONCURRENT_STREAMS limit is upheld occurs in the http2 library we used.*
+
+### data_frame_padding
+
+This test verifies that the client can correctly receive padded http2 data
+frames. It also stresses the client's flow control (there is a high chance
+that the sender will deadlock if the client's flow control logic doesn't
+correctly account for padding).
+
+Client Procedure:
+(Note this is the same procedure as in the "large_unary" gRPC interop tests.
+Clients should use their "large_unary" gRPC interop test implementations.)
+Procedure:
+ 1. Client calls UnaryCall with:
+
+    ```
+    {
+      response_size: 314159
+      payload:{
+        body: 271828 bytes of zeros
+      }
+    }
+    ```
+
+Client asserts:
+* call was successful
+* response payload body is 314159 bytes in size
+* clients are free to assert that the response payload body contents are zero
+  and comparing the entire response message against a golden response
+
+Server Procedure:
+  1. Reply to the client's request with a `SimpleResponse`, with a payload
+  body length of `SimpleRequest.response_size`. But send it across specific
+  http2 data frames as follows:
+    * Each http2 data frame contains a 5 byte payload and 255 bytes of padding.
+
+  * Note the 5 byte payload and 255 byte padding are partly arbitrary,
+  and other numbers are also ok. With 255 bytes of padding for each 5 bytes of
+  payload containing actual gRPC message, the 300KB response size will
+  multiply into around 15 megabytes of flow control debt, which should stress
+  flow control accounting.
+
+### no_df_padding_sanity_test
+
+This test verifies that the client can correctly receive a series of small
+data frames. Note that this test is intentionally a slight variation of
+"data_frame_padding", with the only difference being that this test doesn't use data
+frame padding when the response is sent. This test is primarily meant to
+prove correctness of the http2 server implementation and highlight failures
+of the "data_frame_padding" test.
+
+Client Procedure:
+(Note this is the same procedure as in the "large_unary" gRPC interop tests.
+Clients should use their "large_unary" gRPC interop test implementations.)
+Procedure:
+ 1. Client calls UnaryCall with:
+
+    ```
+    {
+      response_size: 314159
+      payload:{
+        body: 271828 bytes of zeros
+      }
+    }
+    ```
+
+Client asserts:
+* call was successful
+* response payload body is 314159 bytes in size
+* clients are free to assert that the response payload body contents are zero
+  and comparing the entire response message against a golden response
+
+Server Procedure:
+  1. Reply to the client's request with a `SimpleResponse`, with a payload
+  body length of `SimpleRequest.response_size`. But send it across series of
+  http2 data frames that contain 5 bytes of "payload" and zero bytes of
+  "padding" (the padding flags on the data frames should not be set).