On 03/02/14 18:06, Tomás Senart wrote:
> On Monday, February 3, 2014 5:56:37 PM UTC, Tomás Senart wrote:
> On Monday, February 3, 2014 5:40:06 PM UTC, Nick Craig-Wood wrote:
> IMHO locks look nicer when anonymous
> I generally agree with this whenever I am not writing a library, but
> that would expose the lock publicly which I don't want.
Good point
> Your Stop() methods are leaking go routines - from the docs
>
http://golang.org/pkg/time/#Ticker.Stop
> <
http://golang.org/pkg/time/#Ticker.Stop>
>
> Stop turns off a ticker. After Stop, no more ticks will be sent.
> Stop
> does not close the channel, to prevent a read from the channel
> succeeding incorrectly.
>
> So you'll want to close the channel too in order to stop the go
> routines
> probably. The documentation should state that a go routine is
> started
> and you'll need to call Stop() to stop it.
>
>
> Ohhh... Thanks for this one. I thought Stop closed the channel.
> I will change and add the documentation you suggested.
>
>
> It turns out that the provided channel is a receive only channel because
> there aren't actually any go-routines launched for the ticker.
>
http://golang.org/src/pkg/time/tick.go#L20
I was talking about the goroutine you start which runs this
func (b *Bucket) fill(capacity int64) {
for _ = range b.ticker.C {
if tokens := atomic.LoadInt64(&b.tokens); tokens < capacity {
atomic.AddInt64(&b.tokens, 1)
}
}
}
This go routine is never stopped - the for{} loop never exits. I was
thinking you could close the ticker to stop it but obviously not
> It isn't clear to me from the docs exactly what happens when there
> aren't enough tokens in the bucket. Do those tokens get taken
> out the
> bucket or not? I would hope not, but I'm not sure from this.
>
>
> If you have 0 tokens in the Bucket and you try to "take" any number
> of tokens, 0 tokens will be taken.
> If you have 10 tokens in the Bucket and you "take" 5 tokens then 5
> tokens will be taken.
> If you have 10 tokens in the Bucket and you try to "take" 15 then
> only 10 tokens will be taken.
>
> How do you suggest the documentation to reflect this better?
Why don't you put exactly what you wrote above in? It is a nice
explanation!
> // Take will take n tokens out of the bucket. If there aren't
> enough
> // tokens, the difference is returned. Otherwise n is returned.
> // This method is thread-safe.
> func (b *Bucket) Take(n int64) int64 {
>
> I would have though a better API would be something like this
>
>
> This wouldn't allow for partial success, as described above. If
> we're processing 1000 items and only 500 are allowed through, then
> the caller wants to process the first 500 differently from the last 500.
>
>
>
> // Take will attempt to take n tokens out of the bucket. If there
> // aren't enough in the bucket it returns false, otherwise it
> removes
> // the tokens from the bucket and returns true.
> // This method is thread-safe.
> func (b *Bucket) Take(n int64) bool {
Ah I was perhaps thinking of a different use. Say I have a channel of
1000 bytes per second, so I put 1000 tokens per second in to the bucket.
When I come to send a packet of 100 bytes say I try to get 100 tokens
from the bucket. I want to be able to send all the packet or none of it,
having a number of tokens < 100 returned is no use in that case.