code review 170290043: x/talks/2014/gotham-context: talk for GothamGo 2014 in NYC.

x/talks/2014/gotham-context: talk for GothamGo 2014 in NYC.

[bcmills, 2014-11-13 19:53:14]

Overall LGTM.

Mostly, I'd put more emphasis on "call" over "request".

https://codereview.appspot.com/170290043/diff/100001/2014/gotham-context.slide
File 2014/gotham-context.slide (right):

https://codereview.appspot.com/170290043/diff/100001/2014/gotham-context.slid...
2014/gotham-context.slide:106: A `Context` carries a cancelation signal and
request-scoped values to all functions running on behalf of the same task.
Maybe s/request-scoped/call-scoped/ throughout?

People writing APIs have an unfortunate tendency to think "there's no request
associated with my API, so I don't need to plumb in a Context".  And then they
overuse context.Background() or context.TODO() and things break or hang.

"call-scoped" has a nice double-entendre that keeps it relevant - the initial
Context values for an incoming RPC are scoped to the RPC call, but values added
to child Contexts are scoped to the *function* call.

https://codereview.appspot.com/170290043/diff/100001/2014/gotham-context.slid...
2014/gotham-context.slide:136: Contexts form a tree, any subtree of which can be
canceled.
Maybe add something like: "The tree mirrors the tree of function calls in the
program."

(That's the phrasing I use when explaining what data should or should not be
carried in a Context value.)

https://codereview.appspot.com/170290043/diff/100001/2014/gotham-context.slid...
2014/gotham-context.slide:180: .code gotham-context/first-context.go
/START1/,/STOP1/
The usual caveats about goroutine pressure under load apply to this example - it
either needs a global bound on unreclaimed replicas (e.g. a semaphore) or an
explicit "join" operation before returning.  (The fact that cancellation is only
informational has deep implications.)

I get that it's a simplified example, but it would at least be nice to mention
the ways it which it has been simplified.

[Sameer Ajmani, 2014-11-13 20:32:41]

On 2014/11/13 19:53:14, bcmills wrote:
> Overall LGTM.
> 
> Mostly, I'd put more emphasis on "call" over "request".
> 
> https://codereview.appspot.com/170290043/diff/100001/2014/gotham-context.slide
> File 2014/gotham-context.slide (right):
> 
>
https://codereview.appspot.com/170290043/diff/100001/2014/gotham-context.slid...
> 2014/gotham-context.slide:106: A `Context` carries a cancelation signal and
> request-scoped values to all functions running on behalf of the same task.
> Maybe s/request-scoped/call-scoped/ throughout?

I don't think call-scoped conveys the flow across goroutines and processes I
depict i the Cancelation is transitive slide.
Perhaps request-scoped doesn't, either, but "request scoped" is a widely-used
term.

> 
> People writing APIs have an unfortunate tendency to think "there's no request
> associated with my API, so I don't need to plumb in a Context".  And then they
> overuse context.Background() or context.TODO() and things break or hang.
> 
> "call-scoped" has a nice double-entendre that keeps it relevant - the initial
> Context values for an incoming RPC are scoped to the RPC call, but values
added
> to child Contexts are scoped to the *function* call.
> 
>
https://codereview.appspot.com/170290043/diff/100001/2014/gotham-context.slid...
> 2014/gotham-context.slide:136: Contexts form a tree, any subtree of which can
be
> canceled.
> Maybe add something like: "The tree mirrors the tree of function calls in the
> program."

Again, not quite true when you consider goroutines and RPCs.  Or at least, you
need a generous notion of "calls".

> 
> (That's the phrasing I use when explaining what data should or should not be
> carried in a Context value.)
> 
>
https://codereview.appspot.com/170290043/diff/100001/2014/gotham-context.slid...
> 2014/gotham-context.slide:180: .code gotham-context/first-context.go
> /START1/,/STOP1/
> The usual caveats about goroutine pressure under load apply to this example -
it
> either needs a global bound on unreclaimed replicas (e.g. a semaphore) or an
> explicit "join" operation before returning.  (The fact that cancellation is
only
> informational has deep implications.)
> 
> I get that it's a simplified example, but it would at least be nice to mention
> the ways it which it has been simplified.

I knew you'd bring this one up :-)  I debated adding a waitgroup or draining the
channel,
but explaining that change would require more time than I have.  Per our last
discussion on this,
I'm awaiting your blog post on well-conditioned Go servers (with numbers to back
it up) :-)

[bcmills, 2014-11-13 20:44:27]

On Thu, Nov 13, 2014 at 3:32 PM, <sameer@golang.org> wrote:

> Reviewers: bcmills,
>
> Message:
> On 2014/11/13 19:53:14, bcmills wrote:
>
>> Overall LGTM.
>>
>
>  Mostly, I'd put more emphasis on "call" over "request".
>>
>
>
> https://codereview.appspot.com/170290043/diff/100001/
> 2014/gotham-context.slide
>
>> File 2014/gotham-context.slide (right):
>>
>
>
> https://codereview.appspot.com/170290043/diff/100001/
> 2014/gotham-context.slide#newcode106
>
>> 2014/gotham-context.slide:106: A `Context` carries a cancelation
>>
> signal and
>
>> request-scoped values to all functions running on behalf of the same
>>
> task.
>
>> Maybe s/request-scoped/call-scoped/ throughout?
>>
>
> I don't think call-scoped conveys the flow across goroutines and
> processes I depict i the Cancelation is transitive slide.
> Perhaps request-scoped doesn't, either, but "request scoped" is a
> widely-used term.


Well, you need the double-entendre for it to work - across goroutines is
"function calls", and across processes is "RPC calls".

The trouble with "request-scoped" is that it makes people think they
shouldn't use Context when they can't identify the "request".



>  People writing APIs have an unfortunate tendency to think "there's no
>>
> request
>
>> associated with my API, so I don't need to plumb in a Context".  And
>>
> then they
>
>> overuse context.Background() or context.TODO() and things break or
>>
> hang.
>
>  "call-scoped" has a nice double-entendre that keeps it relevant - the
>>
> initial
>
>> Context values for an incoming RPC are scoped to the RPC call, but
>>
> values added
>
>> to child Contexts are scoped to the *function* call.
>>
>
>
> https://codereview.appspot.com/170290043/diff/100001/
> 2014/gotham-context.slide#newcode136
>
>> 2014/gotham-context.slide:136: Contexts form a tree, any subtree of
>>
> which can be
>
>> canceled.
>> Maybe add something like: "The tree mirrors the tree of function calls
>>
> in the
>
>> program."
>>
>
> Again, not quite true when you consider goroutines and RPCs.  Or at
> least, you need a generous notion of "calls".


The spec is pleasantly generous (emphasis mine):
'A "go" statement starts the execution of a _function call_ as an
independent concurrent thread of control.'


 (That's the phrasing I use when explaining what data should or should
>>
> not be
>
>> carried in a Context value.)
>>
>
>
> https://codereview.appspot.com/170290043/diff/100001/
> 2014/gotham-context.slide#newcode180
>
>> 2014/gotham-context.slide:180: .code gotham-context/first-context.go
>> /START1/,/STOP1/
>> The usual caveats about goroutine pressure under load apply to this
>>
> example - it
>
>> either needs a global bound on unreclaimed replicas (e.g. a semaphore)
>>
> or an
>
>> explicit "join" operation before returning.  (The fact that
>>
> cancellation is only
>
>> informational has deep implications.)
>>
>
>  I get that it's a simplified example, but it would at least be nice to
>>
> mention
>
>> the ways it which it has been simplified.
>>
>
> I knew you'd bring this one up :-)  I debated adding a waitgroup or
> draining the channel,
> but explaining that change would require more time than I have.  Per our
> last discussion on this,
> I'm awaiting your blog post on well-conditioned Go servers (with numbers
> to back it up) :-)
>
> Description:
> gotham-context: talk for GothamGo 2014 in NYC.
>
> Please review this at https://codereview.appspot.com/170290043/
>
> Affected files (+520, -0 lines):
>   A 2014/gotham-context.slide
>   A 2014/gotham-context/after.go
>   A 2014/gotham-context/before.go
>   A 2014/gotham-context/eg.go
>   A 2014/gotham-context/first.go
>   A 2014/gotham-context/first-context.go
>   A 2014/gotham-context/interface.go
>   A 2014/gotham-context/transitive.svg
>
>
>

[Sameer Ajmani, 2014-11-14 15:57:46]

On Thu, Nov 13, 2014 at 3:43 PM, Bryan Mills <bcmills@google.com> wrote:

>
>
> On Thu, Nov 13, 2014 at 3:32 PM, <sameer@golang.org> wrote:
>
>> Reviewers: bcmills,
>>
>> Message:
>> On 2014/11/13 19:53:14, bcmills wrote:
>>
>>> Overall LGTM.
>>>
>>
>>  Mostly, I'd put more emphasis on "call" over "request".
>>>
>>
>>
>> https://codereview.appspot.com/170290043/diff/100001/
>> 2014/gotham-context.slide
>>
>>> File 2014/gotham-context.slide (right):
>>>
>>
>>
>> https://codereview.appspot.com/170290043/diff/100001/
>> 2014/gotham-context.slide#newcode106
>>
>>> 2014/gotham-context.slide:106: A `Context` carries a cancelation
>>>
>> signal and
>>
>>> request-scoped values to all functions running on behalf of the same
>>>
>> task.
>>
>>> Maybe s/request-scoped/call-scoped/ throughout?
>>>
>>
>> I don't think call-scoped conveys the flow across goroutines and
>> processes I depict i the Cancelation is transitive slide.
>> Perhaps request-scoped doesn't, either, but "request scoped" is a
>> widely-used term.
>
>
> Well, you need the double-entendre for it to work - across goroutines is
> "function calls", and across processes is "RPC calls".
>
> The trouble with "request-scoped" is that it makes people think they
> shouldn't use Context when they can't identify the "request".
>

Agreed.  I'll make the point when speaking that this is useful not just for
request scopes but for any blocking call that wants to support cancelation
or timeouts.


>
>
>
>>  People writing APIs have an unfortunate tendency to think "there's no
>>>
>> request
>>
>>> associated with my API, so I don't need to plumb in a Context".  And
>>>
>> then they
>>
>>> overuse context.Background() or context.TODO() and things break or
>>>
>> hang.
>>
>>  "call-scoped" has a nice double-entendre that keeps it relevant - the
>>>
>> initial
>>
>>> Context values for an incoming RPC are scoped to the RPC call, but
>>>
>> values added
>>
>>> to child Contexts are scoped to the *function* call.
>>>
>>
>>
>> https://codereview.appspot.com/170290043/diff/100001/
>> 2014/gotham-context.slide#newcode136
>>
>>> 2014/gotham-context.slide:136: Contexts form a tree, any subtree of
>>>
>> which can be
>>
>>> canceled.
>>> Maybe add something like: "The tree mirrors the tree of function calls
>>>
>> in the
>>
>>> program."
>>>
>>
>> Again, not quite true when you consider goroutines and RPCs.  Or at
>> least, you need a generous notion of "calls".
>
>
> The spec is pleasantly generous (emphasis mine):
> 'A "go" statement starts the execution of a _function call_ as an
> independent concurrent thread of control.'
>

Yeah, but I don't think that's what people will think "or goroutines or
RPCs" when I say function calls.  I need to be explicit in the talk.


>
>
>  (That's the phrasing I use when explaining what data should or should
>>>
>> not be
>>
>>> carried in a Context value.)
>>>
>>
>>
>> https://codereview.appspot.com/170290043/diff/100001/
>> 2014/gotham-context.slide#newcode180
>>
>>> 2014/gotham-context.slide:180: .code gotham-context/first-context.go
>>> /START1/,/STOP1/
>>> The usual caveats about goroutine pressure under load apply to this
>>>
>> example - it
>>
>>> either needs a global bound on unreclaimed replicas (e.g. a semaphore)
>>>
>> or an
>>
>>> explicit "join" operation before returning.  (The fact that
>>>
>> cancellation is only
>>
>>> informational has deep implications.)
>>>
>>
>>  I get that it's a simplified example, but it would at least be nice to
>>>
>> mention
>>
>>> the ways it which it has been simplified.
>>>
>>
>> I knew you'd bring this one up :-)  I debated adding a waitgroup or
>> draining the channel,
>> but explaining that change would require more time than I have.  Per our
>> last discussion on this,
>> I'm awaiting your blog post on well-conditioned Go servers (with numbers
>> to back it up) :-)
>>
>> Description:
>> gotham-context: talk for GothamGo 2014 in NYC.
>>
>> Please review this at https://codereview.appspot.com/170290043/
>>
>> Affected files (+520, -0 lines):
>>   A 2014/gotham-context.slide
>>   A 2014/gotham-context/after.go
>>   A 2014/gotham-context/before.go
>>   A 2014/gotham-context/eg.go
>>   A 2014/gotham-context/first.go
>>   A 2014/gotham-context/first-context.go
>>   A 2014/gotham-context/interface.go
>>   A 2014/gotham-context/transitive.svg
>>
>>
>>
>

[Sameer Ajmani, 2014-11-15 17:28:41]

Hello bcmills@google.com (cc: golang-codereviews@googlegroups.com),

I'd like you to review this change to
https://code.google.com/p/go.talks

[Sameer Ajmani, 2014-11-15 17:30:49]

On 2014/11/15 17:28:41, Sameer Ajmani wrote:
> Hello mailto:bcmills@google.com (cc:
mailto:golang-codereviews@googlegroups.com),
> 
> I'd like you to review this change to
> https://code.google.com/p/go.talks

Added adg@ for submit.
The talk is in the form I presented.

[Sameer Ajmani, 2014-11-15 17:31:56]

On 2014/11/15 17:30:49, Sameer Ajmani wrote:
> On 2014/11/15 17:28:41, Sameer Ajmani wrote:
> > Hello mailto:bcmills@google.com (cc:
> mailto:golang-codereviews@googlegroups.com),
> > 
> > I'd like you to review this change to
> > https://code.google.com/p/go.talks
> 
> Added adg@ for submit.
> The talk is in the form I presented.

FYI my hgrc says:
  default = https://code.google.com/p/go.talks

let me know whether I should change this now to:
  default = https://golang.org/x/talks

[minux, 2014-11-15 19:26:48]

On Nov 15, 2014 12:31 PM, <sameer@golang.org> wrote:
> FYI my hgrc says:
>   default = https://code.google.com/p/go.talks
> let me know whether I should change this now to:
>   default = https://golang.org/x/talks
there is no need to change (until we migrate to git)

[Sameer Ajmani, 2014-11-19 11:26:20]

Ping adg for talks repo commit.
On Nov 15, 2014 12:30 PM, <sameer@golang.org> wrote:

> On 2014/11/15 17:28:41, Sameer Ajmani wrote:
>
>> Hello mailto:bcmills@google.com (cc:
>>
> mailto:golang-codereviews@googlegroups.com),
>
>  I'd like you to review this change to
>> https://code.google.com/p/go.talks
>>
>
> Added adg@ for submit.
> The talk is in the form I presented.
>
>
> https://codereview.appspot.com/170290043/
>

[adg1, 2014-11-21 02:39:32]

You must add

// +build OMIT

to the top of each of your example programs

[Sameer Ajmani, 2014-11-25 21:36:34]

On 2014/11/21 02:39:32, adg1 wrote:
> You must add
> 
> // +build OMIT
> 
> to the top of each of your example programs

Done.

[adg1, 2014-11-25 21:58:44]

LGTM

[Sameer Ajmani, 2014-11-28 03:44:47]

Do I have submitter privileges for the talks repo now?
On Nov 25, 2014 4:58 PM, <adg@google.com> wrote:

> LGTM
>
> https://codereview.appspot.com/170290043/
>

[adg1, 2014-11-28 03:53:29]

If you can submit to the go repo you can submit here.

On Fri Nov 28 2014 at 2:44:45 PM Sameer Ajmani <sameer@golang.org> wrote:

> Do I have submitter privileges for the talks repo now?
> On Nov 25, 2014 4:58 PM, <adg@google.com> wrote:
>
>> LGTM
>>
>> https://codereview.appspot.com/170290043/
>>
>

[Sameer Ajmani, 2014-12-01 16:22:57]

*** Submitted as
https://code.google.com/p/go/source/detail?r=b88095ff947b&repo=talks ***

x/talks/2014/gotham-context: talk for GothamGo 2014 in NYC.

LGTM=bcmills, adg
R=bcmills, adg, minux
CC=golang-codereviews
https://codereview.appspot.com/170290043