Skip to content

Commit

Permalink
slack-vitess-r15.0.5: fix races in Unit Test (Race) CI, fix "old"…
Browse files Browse the repository at this point in the history
… reparent CIs (#356)
  • Loading branch information
timvaillancourt committed Jul 9, 2024
1 parent d991fe9 commit 93ac79f
Show file tree
Hide file tree
Showing 10 changed files with 37 additions and 16 deletions.
2 changes: 1 addition & 1 deletion go/flags/endtoend/vtbackup.txt
Original file line number Diff line number Diff line change
Expand Up @@ -217,7 +217,7 @@ Flags:
--topo_consul_lock_delay duration LockDelay for consul session. (default 15s)
--topo_consul_lock_session_checks string List of checks for consul session. (default "serfHealth")
--topo_consul_lock_session_ttl string TTL for consul session.
--topo_consul_max_conns_per_host int Maximum number of consul connections per host.
--topo_consul_max_conns_per_host int Maximum number of consul connections per host. (default 250)
--topo_consul_max_idle_conns int Maximum number of idle consul connections. (default 100)
--topo_consul_watch_poll_duration duration time of the long poll for watch queries. (default 30s)
--topo_etcd_lease_ttl int Lease TTL for locks and leader election. The client will use KeepAlive to keep the lease going. (default 30)
Expand Down
2 changes: 1 addition & 1 deletion go/flags/endtoend/vtctld.txt
Original file line number Diff line number Diff line change
Expand Up @@ -155,7 +155,7 @@ Flags:
--topo_consul_lock_delay duration LockDelay for consul session. (default 15s)
--topo_consul_lock_session_checks string List of checks for consul session. (default "serfHealth")
--topo_consul_lock_session_ttl string TTL for consul session.
--topo_consul_max_conns_per_host int Maximum number of consul connections per host.
--topo_consul_max_conns_per_host int Maximum number of consul connections per host. (default 250)
--topo_consul_max_idle_conns int Maximum number of idle consul connections. (default 100)
--topo_consul_watch_poll_duration duration time of the long poll for watch queries. (default 30s)
--topo_etcd_lease_ttl int Lease TTL for locks and leader election. The client will use KeepAlive to keep the lease going. (default 30)
Expand Down
2 changes: 1 addition & 1 deletion go/flags/endtoend/vtgate.txt
Original file line number Diff line number Diff line change
Expand Up @@ -208,7 +208,7 @@ Flags:
--topo_consul_lock_delay duration LockDelay for consul session. (default 15s)
--topo_consul_lock_session_checks string List of checks for consul session. (default "serfHealth")
--topo_consul_lock_session_ttl string TTL for consul session.
--topo_consul_max_conns_per_host int Maximum number of consul connections per host.
--topo_consul_max_conns_per_host int Maximum number of consul connections per host. (default 250)
--topo_consul_max_idle_conns int Maximum number of idle consul connections. (default 100)
--topo_consul_watch_poll_duration duration time of the long poll for watch queries. (default 30s)
--topo_etcd_lease_ttl int Lease TTL for locks and leader election. The client will use KeepAlive to keep the lease going. (default 30)
Expand Down
2 changes: 1 addition & 1 deletion go/flags/endtoend/vtorc.txt
Original file line number Diff line number Diff line change
Expand Up @@ -93,7 +93,7 @@ Flags:
--topo_consul_lock_delay duration LockDelay for consul session. (default 15s)
--topo_consul_lock_session_checks string List of checks for consul session. (default "serfHealth")
--topo_consul_lock_session_ttl string TTL for consul session.
--topo_consul_max_conns_per_host int Maximum number of consul connections per host.
--topo_consul_max_conns_per_host int Maximum number of consul connections per host. (default 250)
--topo_consul_max_idle_conns int Maximum number of idle consul connections. (default 100)
--topo_consul_watch_poll_duration duration time of the long poll for watch queries. (default 30s)
--topo_etcd_lease_ttl int Lease TTL for locks and leader election. The client will use KeepAlive to keep the lease going. (default 30)
Expand Down
2 changes: 1 addition & 1 deletion go/flags/endtoend/vttablet.txt
Original file line number Diff line number Diff line change
Expand Up @@ -359,7 +359,7 @@ Flags:
--topo_consul_lock_delay duration LockDelay for consul session. (default 15s)
--topo_consul_lock_session_checks string List of checks for consul session. (default "serfHealth")
--topo_consul_lock_session_ttl string TTL for consul session.
--topo_consul_max_conns_per_host int Maximum number of consul connections per host.
--topo_consul_max_conns_per_host int Maximum number of consul connections per host. (default 250)
--topo_consul_max_idle_conns int Maximum number of idle consul connections. (default 100)
--topo_consul_watch_poll_duration duration time of the long poll for watch queries. (default 30s)
--topo_etcd_lease_ttl int Lease TTL for locks and leader election. The client will use KeepAlive to keep the lease going. (default 30)
Expand Down
2 changes: 1 addition & 1 deletion go/flags/endtoend/vttestserver.txt
Original file line number Diff line number Diff line change
Expand Up @@ -131,7 +131,7 @@ Flags:
--topo_consul_lock_delay duration LockDelay for consul session. (default 15s)
--topo_consul_lock_session_checks string List of checks for consul session. (default "serfHealth")
--topo_consul_lock_session_ttl string TTL for consul session.
--topo_consul_max_conns_per_host int Maximum number of consul connections per host.
--topo_consul_max_conns_per_host int Maximum number of consul connections per host. (default 250)
--topo_consul_max_idle_conns int Maximum number of idle consul connections. (default 100)
--topo_consul_watch_poll_duration duration time of the long poll for watch queries. (default 30s)
--topo_zk_auth_file string auth to use when connecting to the zk topo server, file contents should be <scheme>:<auth>, e.g., digest:user:pass
Expand Down
13 changes: 13 additions & 0 deletions go/internal/flag/flag.go
Original file line number Diff line number Diff line change
Expand Up @@ -29,10 +29,13 @@ import (
"os"
"reflect"
"strings"
"sync"

flag "github.com/spf13/pflag"
)

var flagsMu sync.Mutex

// Parse wraps the standard library's flag.Parse to perform some sanity checking
// and issue deprecation warnings in advance of our move to pflag.
//
Expand All @@ -42,6 +45,8 @@ import (
//
// See VEP-4, phase 1 for details: https://github.com/vitessio/enhancements/blob/c766ea905e55409cddeb666d6073cd2ac4c9783e/veps/vep-4.md#phase-1-preparation
func Parse(fs *flag.FlagSet) {
flagsMu.Lock()
defer flagsMu.Unlock()
PreventGlogVFlagFromClobberingVersionFlagShorthand(fs)
fs.AddGoFlagSet(goflag.CommandLine)

Expand Down Expand Up @@ -70,6 +75,8 @@ func Parse(fs *flag.FlagSet) {

// IsFlagProvided returns if the given flag has been provided by the user explicitly or not
func IsFlagProvided(name string) bool {
flagsMu.Lock()
defer flagsMu.Unlock()
fl := flag.Lookup(name)
if fl != nil {
return fl.Changed
Expand Down Expand Up @@ -167,6 +174,8 @@ func filterTestFlags() ([]string, []string) {
// handle `go test` flags correctly. We need to separately parse the test flags using goflags. Additionally flags
// like test.Short() require that goflag.Parse() is called first.
func ParseFlagsForTest() {
flagsMu.Lock()
defer flagsMu.Unlock()
// We need to split up the test flags and the regular app pflags.
// Then hand them off the std flags and pflags parsers respectively.
args, testFlags := filterTestFlags()
Expand Down Expand Up @@ -198,6 +207,8 @@ func Parsed() bool {
// standard library `flag` CommandLine. If found in the latter, it is converted
// to a pflag.Flag first. If found in neither, this function returns nil.
func Lookup(name string) *flag.Flag {
flagsMu.Lock()
defer flagsMu.Unlock()
if f := flag.Lookup(name); f != nil {
return f
}
Expand All @@ -213,6 +224,8 @@ func Lookup(name string) *flag.Flag {
// removed. If no double-dash was specified on the command-line, this is
// equivalent to flag.Args() from the standard library flag package.
func Args() (args []string) {
flagsMu.Lock()
defer flagsMu.Unlock()
doubleDashIdx := -1
for i, arg := range flag.Args() {
if arg == "--" {
Expand Down
4 changes: 0 additions & 4 deletions go/vt/servenv/exporter.go
Original file line number Diff line number Diff line change
Expand Up @@ -106,10 +106,6 @@ type Exporter struct {
mu sync.Mutex
}

func init() {
HTTPHandle("/debug/vars", expvar.Handler())
}

// NewExporter creates a new Exporter with name as namespace.
// label is the name of the additional dimension for the stats vars.
func NewExporter(name, label string) *Exporter {
Expand Down
23 changes: 17 additions & 6 deletions go/vt/topo/consultopo/server.go
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@ import (
"time"

"github.com/hashicorp/consul/api"
"github.com/hashicorp/go-cleanhttp"
"github.com/spf13/pflag"

"vitess.io/vitess/go/vt/log"
Expand All @@ -37,25 +38,32 @@ import (

var (
consulAuthClientStaticFile string
consulConfig = api.DefaultConfig()
// serfHealth is the default check from consul
consulLockSessionChecks = "serfHealth"
consulLockSessionTTL string
consulLockDelay = 15 * time.Second
consulLockDelay = 15 * time.Second
consulMaxConnsPerHost int = 250 // do not use client default of 0/unlimited
consulMaxIdleConns int
consulIdleConnTimeout time.Duration
)

func init() {
servenv.RegisterFlagsForTopoBinaries(registerServerFlags)
}

func registerServerFlags(fs *pflag.FlagSet) {
// cleanhttp.DefaultPooledTransport() is used by the consul api client
// as an *http.Transport. We call it here just to get the default
// values the consul api client will inherit from it later.
defaultConsulPooledTransport := cleanhttp.DefaultPooledTransport()

fs.StringVar(&consulAuthClientStaticFile, "consul_auth_static_file", consulAuthClientStaticFile, "JSON File to read the topos/tokens from.")
fs.StringVar(&consulLockSessionChecks, "topo_consul_lock_session_checks", consulLockSessionChecks, "List of checks for consul session.")
fs.StringVar(&consulLockSessionTTL, "topo_consul_lock_session_ttl", consulLockSessionTTL, "TTL for consul session.")
fs.DurationVar(&consulLockDelay, "topo_consul_lock_delay", consulLockDelay, "LockDelay for consul session.")
fs.IntVar(&consulConfig.Transport.MaxConnsPerHost, "topo_consul_max_conns_per_host", consulConfig.Transport.MaxConnsPerHost, "Maximum number of consul connections per host.")
fs.IntVar(&consulConfig.Transport.MaxIdleConns, "topo_consul_max_idle_conns", consulConfig.Transport.MaxIdleConns, "Maximum number of idle consul connections.")
fs.DurationVar(&consulConfig.Transport.IdleConnTimeout, "topo_consul_idle_conn_timeout", consulConfig.Transport.IdleConnTimeout, "Maximum amount of time to pool idle connections.")
fs.IntVar(&consulMaxConnsPerHost, "topo_consul_max_conns_per_host", consulMaxConnsPerHost, "Maximum number of consul connections per host.")
fs.IntVar(&consulMaxIdleConns, "topo_consul_max_idle_conns", defaultConsulPooledTransport.MaxIdleConns, "Maximum number of idle consul connections.")
fs.DurationVar(&consulIdleConnTimeout, "topo_consul_idle_conn_timeout", defaultConsulPooledTransport.IdleConnTimeout, "Maximum amount of time to pool idle connections.")
}

// ClientAuthCred credential to use for consul clusters
Expand Down Expand Up @@ -134,8 +142,11 @@ func NewServer(cell, serverAddr, root string) (*Server, error) {
if err != nil {
return nil, err
}
cfg := consulConfig
cfg := api.DefaultConfig()
cfg.Address = serverAddr
cfg.Transport.MaxConnsPerHost = consulMaxConnsPerHost
cfg.Transport.MaxIdleConns = consulMaxIdleConns
cfg.Transport.IdleConnTimeout = consulIdleConnTimeout
if creds != nil {
if creds[cell] != nil {
cfg.Token = creds[cell].ACLToken
Expand Down
1 change: 1 addition & 0 deletions go/vt/vttest/topoctl.go
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,7 @@ func (ctl *Topoctl) Setup() error {
if err != nil {
return err
}
defer topoServer.Close()

log.Infof("Creating cells if they don't exist in the provided topo server %s %s %s", ctl.TopoImplementation, ctl.TopoGlobalServerAddress, ctl.TopoGlobalRoot)
// Create cells if it doesn't exist to be idempotent. Should work when we share the same topo server across multiple local clusters.
Expand Down

0 comments on commit 93ac79f

Please sign in to comment.