Golang: throttle (time delay) function is not working in goroutine (works fine in main thread)

So I am writing a utility to query an API at work, and they throttle to 20 calls every 10 seconds. Easy, I'll just throttle my calls to at least .5 seconds passed since last call. My Throttle utility worked fine until I tried to use goroutines.

Right now I am using a struct/method combo:

func (c *CTKAPI) Throttle() {
if c.Debug{fmt.Println("\t\t\tEntering Throttle()")}
for { //in case something else makes a call while we're sleeping, we need to re-check
    if t := time.Now().Sub(c.LastCallTime); t < c.ThrottleTime {
        if c.Debug{fmt.Printf("\t\t\tThrottle: Sleeping %v\n", c.ThrottleTime - t)}
        time.Sleep(c.ThrottleTime - t)
    } else {
        if c.Debug{fmt.Println("\t\t\tThrottle: Released.")}
        break
    }
}
c.LastCallTime = time.Now()
if c.Debug{fmt.Println("\t\t\tExiting Throttle()")}

}

And then I call whatever.Throttle() before each call in each goroutine to make sure i've waited at least a half second before launching my next call.

But that seems to be unreliable and gives unpredictable results. Is there a more elegant way of throttling concurrent requests?

-Mike

Because you're introducing a data race, multiple routines are accessing / changing c.LastCallTime.

You use time.Tick instead or make c.LastCallTime an int64 (c.LastCallTime = time.Now().Unix()) and use atomic.LoadInt64/StoreInt64 to check it.

There is actually a much easier way to do this: create a time ticker.

package main

import (
	"fmt"
    "sync"
    "time"
)

func main() {
    rateLimit := time.Tick(500 * time.Millisecond)
    <-rateLimit

    var wg sync.WaitGroup
    for i := 0; i < 10; i++ {
	    wg.Add(1)
	    go func(i int) {
		    <-rateLimit
		    fmt.Println("Hello", i)
		    wg.Done()
	    }(i)
    }
    wg.Wait()
}

Answering my own question, as I happened across a few solutions late last night.
The simplest answer for me was to use a sync.Mutex variable to lock and unlock the throttle function to make sure I wasn't accidentally hitting it at the same time. Another option would have been to move my throttling service to be housed in its own goroutine function (thus eliminating concurrent calls) and communicating throttled/OK with channels, but for this application the Mutex was a cleaner solution.
Here is the simplified version of the working code for those looking for a similar solution:

package main
import (
"fmt"
"time"
"sync"
)
type tStruct struct {
delay time.Duration
last time.Time
lock sync.Mutex //this will be our locking variable
}
func (t *tStruct) createT() *tStruct {
return &tStruct {
delay: 500*time.Millisecond,
last: time.Now(),
}
}
func (t *tStruct) throttle(th int) {
//we lock our function, and any other routine calling this function will block.
t.lock.Lock()
//and we'll defer an unlock, so when we exit the throttle, we'll be ready for another call.
defer t.lock.Unlock()
fmt.Printf("\tThread %v Entering Throttle Check.\n", th)
defer fmt.Printf("\tThread %v Leaving Throttle Check.\n", th)
for {
p := time.Now().Sub(t.last)
if p < t.delay {
fmt.Printf("\tThread %v Sleeping %v.\n", th, t.delay-p)
time.Sleep(t.delay-p)
} else {
fmt.Printf("\tThread %v No longer Throttled.\n", th)
t.last = time.Now()
break
}
}
}
func (t *tStruct) worker(rch <-chan string, sch chan<- string, th int) {
fmt.Printf("Thread %v starting up.\n", th)
defer fmt.Printf("Thread %v Dead.\n", th)
sch <-"READY"
for {
r := <-rch
fmt.Printf("Thread %v received %v\n", th, r)
switch r {
case "STOP":
fmt.Printf("Thread %v returning.\n", th)
sch <-"QUITTING"
return
default:
fmt.Printf("Thread %v processing %v.\n", th, r)
t.throttle(th)
fmt.Printf("Thread %v done with %v.\n", th, r)
sch <-"OK"
}
}
}
func main() {
ts := tStruct{}
ts.delay = 500*time.Millisecond
ts.last = time.Now()
sch := make(chan string)
rch := make(chan string)
tC := 3
tA := 0
fmt.Println("Starting Threads")
for i:=1; i<(tC+1); i++ {
go ts.worker(sch, rch, i)
r := <-rch
if r=="READY" {
tA++
} else {
fmt.Println("ERROR not READY")
}
}
fmt.Println("Feeding All Threads")
for i:=1; i<(tC+1); i++ {
sch <- "WORK"
}
fmt.Println("Listening for threads")
for tA > 0{
r := <-rch
switch r {
case "QUITTING":
tA--
fmt.Println("main received QUITTING")
continue
case "OK":
fmt.Println("main received OK")
sch <-"STOP"
continue
default:
fmt.Println("Shouldn't be here!!!")
}
}
}

Your new code is better. As mentioned in another answer you had a race. Go has a built in race detector go build -race. It's an amazing tool and would have found the race for you with a good unit test.

I believe one of your initial assumptions is flawed. By pacing all API calls you eliminate the chance for any bursting. In your scheme every API call takes a latency hit even when it might not have to. Unless you are sure that every API call is going to hit the throttle there is a better way.

Start a time.NewTicker to 10 seconds and initialize a counter to 0. Increment the counter for each API request. If the counter gets to 20 sleep the goroutines until the timer goes off. When the timer goes off reset the counter and continue the sleeping goroutines.

I've always wanted to code an API rate limiter so I coded it up and you can see it here:
https://github.com/tildeleb/limiter/blob/master/limiter.go

Except for the example, it's untested. Any feedback, please create an issue on github.