diff options
Diffstat (limited to 'doc')
-rw-r--r-- | doc/combiner-explainer.md | 158 | ||||
-rw-r--r-- | doc/core/grpc-error.md | 160 | ||||
-rw-r--r-- | doc/g_stands_for.md | 1 | ||||
-rw-r--r-- | doc/http2-interop-test-descriptions.md (renamed from doc/negative-http2-interop-test-descriptions.md) | 76 |
4 files changed, 395 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/g_stands_for.md b/doc/g_stands_for.md index 53a1fdf193..d2fc7a50f9 100644 --- a/doc/g_stands_for.md +++ b/doc/g_stands_for.md @@ -7,3 +7,4 @@ future), and the corresponding version numbers that used them: - 1.0 'g' stands for 'gRPC' - 1.1 'g' stands for 'good' - 1.2 'g' stands for 'green' +- 1.3 'g' stands for 'gentle' 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). |