Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: do not retry failed speed test, use deprecated random seeder due to issue on windows #83

Merged
merged 2 commits into from
Jul 17, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 6 additions & 0 deletions config/config.go
Original file line number Diff line number Diff line change
Expand Up @@ -5,13 +5,15 @@ import (
"flag"
"fmt"
"io"
"math/rand"
"net"
"net/http"
"os"
"path/filepath"
"strconv"
"strings"
"sync"
"time"

"github.com/imup-io/client/util"
log "golang.org/x/exp/slog"
Expand All @@ -21,6 +23,7 @@ import (
var ImUpAPIHost = "https://api.imup.io"

var (
seedRandom sync.Once
setupFlags sync.Once

allowlistedIPs *string
Expand Down Expand Up @@ -162,6 +165,9 @@ func New() (Reloadable, error) {
defer mu.Unlock()
// do not instantiate a new copy of config, use the package level global
cfg = &config{}
seedRandom.Do(func() {
rand.Seed(time.Now().UTC().UnixNano())
})

setupFlags.Do(func() {
allowlistedIPs = flag.String("allowlisted-ips", "", "comma separated list of CIDR strings to match against host IP that determines whether speed and connectivity testing will be run, default is allow all")
Expand Down
1 change: 0 additions & 1 deletion helpers.go
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,6 @@ import (

// https://github.com/m-lab/ndt-server/blob/master/spec/ndt7-protocol.md#requirements-for-non-interactive-clients
func sleepTime() time.Duration {
rand := rand.New(rand.NewSource(int64(time.Now().Nanosecond())))
t := rand.ExpFloat64() * 21600
if t < 2160 {
t = 2160
Expand Down
3 changes: 0 additions & 3 deletions http_client.go
Original file line number Diff line number Diff line change
Expand Up @@ -27,9 +27,6 @@ func exactJitterBackoff(min, max time.Duration, attemptNum int, resp *nethttp.Re
return min
}

// Seed rand; doing this every time is fine
rand := rand.New(rand.NewSource(int64(time.Now().Nanosecond())))

// Pick a random number that lies somewhere between the min and max and
// multiply by the attemptNum. attemptNum starts at zero so we always
// increment here. We first get a random percentage, then apply that to the
Expand Down
35 changes: 3 additions & 32 deletions speedtesting/speedtest.go
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,6 @@ import (
"context"
"errors"
"fmt"
"math/rand"
"net/url"
"runtime"
"sync"
Expand Down Expand Up @@ -47,7 +46,7 @@ type Options struct {
// if set takes precedence over ndt7 locate API as well as the serviceURL
Server string

// is set takes precendece over ndt7 locate API
// if set takes precedence over ndt7 locate API
ServiceURL *url.URL
}

Expand All @@ -57,7 +56,8 @@ func Run(ctx context.Context, opts Options) (*SpeedTestResult, error) {
defer mu.Unlock()

startTime := time.Now().UnixNano()
result, err := opts.speedTest(ctx, 0)
// TODO: fix error handling for windows speed testing
result, err := RunSpeedTest(ctx, &opts)
endTime := time.Now().UnixNano()

if err != nil && !errors.Is(err, context.Canceled) {
Expand All @@ -73,32 +73,3 @@ func Run(ctx context.Context, opts Options) (*SpeedTestResult, error) {

return result, nil
}

// speed test recursively attempts to get a speed test result utilizing the ndt7 back-off spec and a max retry
func (opts *Options) speedTest(ctx context.Context, retries int) (*SpeedTestResult, error) {
s, err := RunSpeedTest(ctx, opts)
if err != nil && !errors.Is(err, context.Canceled) && retries < 10 {
retries += 1
// https://github.com/m-lab/ndt-server/blob/master/spec/ndt7-protocol.md#requirements-for-non-interactive-clients
for mean := 60.0; mean <= 960.0; mean *= 2.0 {
stdev := 0.05 * mean
seconds := rand.NormFloat64()*stdev + mean

wg := sync.WaitGroup{}
wg.Add(1)
go func() {
defer wg.Done()
select {
case <-ctx.Done():
case <-time.After(time.Duration(seconds * float64(time.Second))):
}
}()

wg.Wait()
return opts.speedTest(ctx, retries)

}
}

return s, err
}
3 changes: 3 additions & 0 deletions speedtesting/speedtest_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,9 @@ import (

func TestSpeedTest(t *testing.T) {
h, srv := NewNDT7Server(t)
// https://github.com/Microsoft/hcsshim/issues/108
time.Sleep(2 * time.Second)

defer os.RemoveAll(h.DataDir)
defer srv.Close()

Expand Down