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

[staticcheck] Cleanup deprecations #13898

Merged
merged 6 commits into from
Aug 31, 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
1 change: 0 additions & 1 deletion examples/compose/client.go
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,6 @@ var (

func main() {
pflag.Parse()
rand.Seed(time.Now().UnixNano())

// Connect to vtgate.
db, err := vitessdriver.Open(*server, "@primary")
Expand Down
1 change: 0 additions & 1 deletion go/cmd/vtclient/vtclient.go
Original file line number Diff line number Diff line change
Expand Up @@ -197,7 +197,6 @@ func run() (*results, error) {
if maxSeqID > minSeqID {
go func() {
if useRandom {
rand.Seed(time.Now().UnixNano())
for {
seqChan <- rand.Intn(maxSeqID-minSeqID) + minSeqID
}
Expand Down
3 changes: 0 additions & 3 deletions go/cmd/vtgate/vtgate.go
Original file line number Diff line number Diff line change
Expand Up @@ -18,9 +18,7 @@ package main

import (
"context"
"math/rand"
"strings"
"time"

"github.com/spf13/pflag"

Expand Down Expand Up @@ -56,7 +54,6 @@ func registerFlags(fs *pflag.FlagSet) {
var resilientServer *srvtopo.ResilientServer

func init() {
rand.Seed(time.Now().UnixNano())
servenv.RegisterDefaultFlags()
servenv.RegisterFlags()
servenv.RegisterGRPCServerFlags()
Expand Down
14 changes: 5 additions & 9 deletions go/netutil/netutil.go
Original file line number Diff line number Diff line change
Expand Up @@ -29,10 +29,6 @@ import (
"time"
)

func init() {
rand.Seed(time.Now().UnixNano())
}

// byPriorityWeight sorts records by ascending priority and weight.
type byPriorityWeight []*net.SRV

Expand All @@ -48,7 +44,7 @@ func (addrs byPriorityWeight) Less(i, j int) bool {
// shuffleByWeight shuffles SRV records by weight using the algorithm
// described in RFC 2782.
// NOTE(msolo) This is disabled when the weights are zero.
func (addrs byPriorityWeight) shuffleByWeight() {
func (addrs byPriorityWeight) shuffleByWeight(rand *rand.Rand) {
sum := 0
for _, addr := range addrs {
sum += int(addr.Weight)
Expand All @@ -72,21 +68,21 @@ func (addrs byPriorityWeight) shuffleByWeight() {
}
}

func (addrs byPriorityWeight) sortRfc2782() {
func (addrs byPriorityWeight) sortRfc2782(rand *rand.Rand) {
sort.Sort(addrs)
i := 0
for j := 1; j < len(addrs); j++ {
if addrs[i].Priority != addrs[j].Priority {
addrs[i:j].shuffleByWeight()
addrs[i:j].shuffleByWeight(rand)
i = j
}
}
addrs[i:].shuffleByWeight()
addrs[i:].shuffleByWeight(rand)
}

// SortRfc2782 reorders SRV records as specified in RFC 2782.
func SortRfc2782(srvs []*net.SRV) {
byPriorityWeight(srvs).sortRfc2782()
byPriorityWeight(srvs).sortRfc2782(rand.New(rand.NewSource(time.Now().UTC().UnixNano())))
}

// SplitHostPort is an alternative to net.SplitHostPort that also parses the
Expand Down
10 changes: 4 additions & 6 deletions go/netutil/netutil_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@ import (
"testing"
)

func checkDistribution(t *testing.T, data []*net.SRV, margin float64) {
func checkDistribution(t *testing.T, rand *rand.Rand, data []*net.SRV, margin float64) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Given that the new golang behavior is to auto-seed the random generator on startup, why would we want to pass a rand * Rand argument to these functions? Wouldn't we just want them to generate random values as opposed to deterministic ones?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

there were a few test cases that relied on rand.Seed(1) before the start of the test, so we update this function to take a rand instance which is either seeded to 1 or to time.Now() (aka "random value")

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm just hesitant to require this additional argument in the function signature. Is it possible to modify the tests to not rely on a reproducible ordering?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I hear you, but I am hesitant to change it, and my brief read (without fully digging in/understanding, so admitting that up front) seems to be that it is definitionally dependent on randomness: https://github.com/vitessio/vitess/pull/42/files#diff-b0fbcaba3872685e058eab7c65e0397e27dab0b0df92694e46fbcb67be911f28

Copy link
Contributor

@shlomi-noach shlomi-noach Aug 31, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, forget it, I notice that these functions are only called internally in the same file (and in the unit test file), so not worth bothering.

sum := 0
for _, srv := range data {
sum += int(srv.Weight)
Expand All @@ -36,7 +36,7 @@ func checkDistribution(t *testing.T, data []*net.SRV, margin float64) {
for j := 0; j < count; j++ {
d := make([]*net.SRV, len(data))
copy(d, data)
byPriorityWeight(d).shuffleByWeight()
byPriorityWeight(d).shuffleByWeight(rand)
key := d[0].Target
results[key] = results[key] + 1
}
Expand All @@ -54,12 +54,11 @@ func checkDistribution(t *testing.T, data []*net.SRV, margin float64) {
}

func testUniformity(t *testing.T, size int, margin float64) {
rand.Seed(1)
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

oh duh, here's my problem

data := make([]*net.SRV, size)
for i := 0; i < size; i++ {
data[i] = &net.SRV{Target: fmt.Sprintf("%c", 'a'+i), Weight: 1}
}
checkDistribution(t, data, margin)
checkDistribution(t, rand.New(rand.NewSource(1)), data, margin)
}

func TestUniformity(t *testing.T) {
Expand All @@ -70,13 +69,12 @@ func TestUniformity(t *testing.T) {
}

func testWeighting(t *testing.T, margin float64) {
rand.Seed(1)
data := []*net.SRV{
{Target: "a", Weight: 60},
{Target: "b", Weight: 30},
{Target: "c", Weight: 10},
}
checkDistribution(t, data, margin)
checkDistribution(t, rand.New(rand.NewSource(1)), data, margin)
}

func TestWeighting(t *testing.T) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -251,7 +251,6 @@ func (bt *BufferingTest) createCluster() (*cluster.LocalProcessCluster, int) {
if err := clusterInstance.StartVtgate(); err != nil {
return nil, 1
}
rand.Seed(time.Now().UnixNano())
return clusterInstance, 0
}

Expand Down
1 change: 0 additions & 1 deletion go/test/endtoend/vreplication/cluster_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -307,7 +307,6 @@ func init() {
if os.Getenv("VREPLICATION_E2E_DEBUG") != "" {
debugMode = true
}
rand.Seed(time.Now().UTC().UnixNano())
originalVtdataroot = os.Getenv("VTDATAROOT")
var mainVtDataRoot string
if debugMode {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -23,10 +23,8 @@ package keyspacewatches
import (
"database/sql"
"fmt"
"math/rand"
"os"
"testing"
"time"

_ "github.com/go-sql-driver/mysql"
"github.com/stretchr/testify/require"
Expand Down Expand Up @@ -115,7 +113,6 @@ func createCluster(extraVTGateArgs []string) (*cluster.LocalProcessCluster, int)
Host: clusterInstance.Hostname,
Port: clusterInstance.VtgateMySQLPort,
}
rand.Seed(time.Now().UnixNano())
return clusterInstance, 0
}

Expand Down
4 changes: 3 additions & 1 deletion go/tools/asthelpergen/asthelpergen.go
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,8 @@ import (
"strings"

"github.com/dave/jennifer/jen"
"golang.org/x/text/cases"
"golang.org/x/text/language"
"golang.org/x/tools/go/packages"

"vitess.io/vitess/go/tools/codegen"
Expand Down Expand Up @@ -304,7 +306,7 @@ func printableTypeName(t types.Type) string {
case *types.Named:
return t.Obj().Name()
case *types.Basic:
return strings.Title(t.Name()) // nolint
return cases.Title(language.AmericanEnglish).String(t.Name())
case *types.Interface:
return t.String()
default:
Expand Down
12 changes: 9 additions & 3 deletions go/vt/mysqlctl/grpcmysqlctlclient/client.go
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@ import (

"google.golang.org/grpc"
"google.golang.org/grpc/codes"
"google.golang.org/grpc/credentials/insecure"
"google.golang.org/grpc/status"

"vitess.io/vitess/go/vt/grpcclient"
Expand All @@ -41,9 +42,14 @@ type client struct {

func factory(network, addr string) (mysqlctlclient.MysqlctlClient, error) {
// create the RPC client
cc, err := grpcclient.Dial(addr, grpcclient.FailFast(false), grpc.WithInsecure(), grpc.WithDialer(func(addr string, timeout time.Duration) (net.Conn, error) { //nolint:staticcheck
return net.DialTimeout(network, addr, timeout)
}))
cc, err := grpcclient.Dial(
addr,
grpcclient.FailFast(false),
grpc.WithTransportCredentials(insecure.NewCredentials()),
grpc.WithContextDialer(func(ctx context.Context, addr string,
) (net.Conn, error) {
return new(net.Dialer).DialContext(ctx, network, addr)
}))
if err != nil {
return nil, err
}
Expand Down
5 changes: 0 additions & 5 deletions go/vt/tableacl/testlib/testlib.go
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,6 @@ import (
"fmt"
"math/rand"
"testing"
"time"

querypb "vitess.io/vitess/go/vt/proto/query"
tableaclpb "vitess.io/vitess/go/vt/proto/tableacl"
Expand Down Expand Up @@ -127,7 +126,3 @@ func checkAccess(config *tableaclpb.Config, tableName string, role tableacl.Role
}
return nil
}

func init() {
rand.Seed(time.Now().UnixNano())
}
8 changes: 4 additions & 4 deletions go/vt/tlstest/tlstest.go
Original file line number Diff line number Diff line change
Expand Up @@ -348,8 +348,8 @@ func RevokeCertAndRegenerateCRL(root, parent, name string) {
log.Fatal(err)
}

revoked := crlList.RevokedCertificates
revoked = append(revoked, pkix.RevokedCertificate{
revoked := crlList.RevokedCertificateEntries
revoked = append(revoked, x509.RevocationListEntry{
SerialNumber: certificate.SerialNumber,
RevocationTime: time.Now(),
})
Expand All @@ -365,8 +365,8 @@ func RevokeCertAndRegenerateCRL(root, parent, name string) {

var crlNumber big.Int
newCrl, err := x509.CreateRevocationList(rand.Reader, &x509.RevocationList{
RevokedCertificates: revoked,
Number: crlNumber.Add(crlList.Number, big.NewInt(1)),
RevokedCertificateEntries: revoked,
Number: crlNumber.Add(crlList.Number, big.NewInt(1)),
}, caCert, caKey.(crypto.Signer))
if err != nil {
log.Fatal(err)
Expand Down
5 changes: 0 additions & 5 deletions go/vt/topo/memorytopo/memorytopo.go
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,6 @@ import (
"math/rand"
"strings"
"sync"
"time"

"vitess.io/vitess/go/vt/log"
"vitess.io/vitess/go/vt/topo"
Expand Down Expand Up @@ -349,7 +348,3 @@ func (f *Factory) recursiveDelete(n *node) {
f.recursiveDelete(parent)
}
}

func init() {
rand.Seed(time.Now().UnixNano())
}
2 changes: 0 additions & 2 deletions go/vt/topotools/shard_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,6 @@ import (
"math/rand"
"sync"
"testing"
"time"

topodatapb "vitess.io/vitess/go/vt/proto/topodata"
"vitess.io/vitess/go/vt/topo/memorytopo"
Expand Down Expand Up @@ -109,7 +108,6 @@ func TestGetOrCreateShard(t *testing.T) {
// and do massive parallel GetOrCreateShard
keyspace := "test_keyspace"
wg := sync.WaitGroup{}
rand.Seed(time.Now().UnixNano())
for i := 0; i < 100; i++ {
wg.Add(1)
go func(i int) {
Expand Down
5 changes: 0 additions & 5 deletions go/vt/vttablet/tabletserver/gc/tablegc.go
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,6 @@ import (
"context"
"fmt"
"math"
"math/rand"
"sort"
"sync"
"sync/atomic"
Expand Down Expand Up @@ -84,10 +83,6 @@ type transitionRequest struct {
uuid string
}

func init() {
rand.Seed(time.Now().UnixNano())
}

// TableGC is the main entity in the table garbage collection mechanism.
// This service "garbage collects" tables:
// - it checks for magically-named tables (e.g. _vt_EVAC_f6338b2af8af11eaa210f875a4d24e90_20200920063522)
Expand Down
5 changes: 0 additions & 5 deletions go/vt/vttablet/tabletserver/tabletserver_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,6 @@ import (
"errors"
"fmt"
"io"
"math/rand"
"net/http"
"net/http/httptest"
"os"
Expand Down Expand Up @@ -2683,7 +2682,3 @@ func addTabletServerSupportedQueries(db *fakesqldb.DB) {
}},
})
}

func init() {
rand.Seed(time.Now().UnixNano())
}
4 changes: 0 additions & 4 deletions go/vt/vttablet/tabletserver/throttle/throttler.go
Original file line number Diff line number Diff line change
Expand Up @@ -110,10 +110,6 @@ const (
ThrottleCheckSelf
)

func init() {
rand.Seed(time.Now().UnixNano())
}

// Throttler is the main entity in the throttling mechanism. This service runs, probes, collects data,
// aggregates, reads inventory, provides information, etc.
type Throttler struct {
Expand Down
18 changes: 8 additions & 10 deletions go/vt/vttablet/tabletserver/vstreamer/packet_size_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,8 @@ import (
"math/rand"
"testing"
"time"

"github.com/stretchr/testify/assert"
)

type polynomial []float64
Expand All @@ -33,7 +35,7 @@ func (p polynomial) fit(x float64) float64 {
return y
}

func simulate(t *testing.T, ps PacketSizer, base, mustSend int, interpolate func(float64) float64) (time.Duration, int) {
func simulate(t *testing.T, rand *rand.Rand, ps PacketSizer, base, mustSend int, interpolate func(float64) float64) (time.Duration, int) {
t.Helper()

var elapsed time.Duration
Expand Down Expand Up @@ -90,25 +92,21 @@ func TestPacketSizeSimulation(t *testing.T) {
for _, tc := range cases {
t.Run(tc.name, func(t *testing.T) {
seed := time.Now().UnixNano()
rand.Seed(seed)
rand := rand.New(rand.NewSource(seed))

// Simulate a replication using the given polynomial and the dynamic packet sizer
ps1 := newDynamicPacketSizer(tc.baseSize)
elapsed1, sent1 := simulate(t, ps1, tc.baseSize, tc.baseSize*1000, tc.p.fit)
elapsed1, sent1 := simulate(t, rand, ps1, tc.baseSize, tc.baseSize*1000, tc.p.fit)

// Simulate the same polynomial using a fixed packet size
ps2 := newFixedPacketSize(tc.baseSize)
elapsed2, sent2 := simulate(t, ps2, tc.baseSize, tc.baseSize*1000, tc.p.fit)
elapsed2, sent2 := simulate(t, rand, ps2, tc.baseSize, tc.baseSize*1000, tc.p.fit)

// the simulation for dynamic packet sizing should always be faster then the fixed packet,
// and should also send fewer packets in total
delta := elapsed1 - elapsed2
if delta > tc.error {
t.Errorf("packet-adjusted simulation is %v slower than fixed approach, seed %d", delta, seed)
}
if sent1 > sent2 {
t.Errorf("packet-adjusted simulation sent more packets (%d) than fixed approach (%d), seed %d", sent1, sent2, seed)
}
assert.LessOrEqualf(t, delta, tc.error, "packet-adjusted simulation is %v slower than fixed approach", delta)
assert.LessOrEqualf(t, sent1, sent2, "packet-adjusted simulation sent more packets (%d) than fixed approach (%d)", sent1, sent2)
// t.Logf("dynamic = (%v, %d), fixed = (%v, %d)", elapsed1, sent1, elapsed2, sent2)
})
}
Expand Down
5 changes: 0 additions & 5 deletions go/vt/vttest/environment.go
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,6 @@ import (
"os"
"path"
"strings"
"time"

"vitess.io/vitess/go/vt/proto/vttest"

Expand Down Expand Up @@ -301,10 +300,6 @@ func defaultEnvFactory() (Environment, error) {
return NewLocalTestEnv("", 0)
}

func init() {
rand.Seed(time.Now().UnixNano())
}

// NewDefaultEnv is an user-configurable callback that returns a new Environment
// instance with the default settings.
// This callback is only used in cases where the user hasn't explicitly set
Expand Down
2 changes: 1 addition & 1 deletion go/vt/vttls/crl.go
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,7 @@ func certIsRevoked(cert *x509.Certificate, crl *x509.RevocationList) bool {
log.Warningf("The current Certificate Revocation List (CRL) is past expiry date and must be updated. Revoked certificates will still be rejected in this state.")
}

for _, revoked := range crl.RevokedCertificates {
for _, revoked := range crl.RevokedCertificateEntries {
if cert.SerialNumber.Cmp(revoked.SerialNumber) == 0 {
return true
}
Expand Down
Loading