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

tx throttler: healthcheck all cells if --tx-throttler-healthcheck-cells is undefined #12477

Merged
Merged
Show file tree
Hide file tree
Changes from 6 commits
Commits
Show all changes
33 commits
Select commit Hold shift + click to select a range
1fecb5d
WIP
timvaillancourt Feb 24, 2023
6beae70
vttablet tx throttler: healthcheck all cells if cells unspecified
timvaillancourt Feb 24, 2023
c8ad379
Fix test typo
timvaillancourt Feb 24, 2023
6ed1bac
Simplify test
timvaillancourt Feb 24, 2023
4c04f24
remove unnecessary 'empty healthCheckCells given' error
timvaillancourt Feb 25, 2023
08c1a1d
last tweak to simplify test
timvaillancourt Feb 25, 2023
0dd51b8
Update go/vt/vttablet/tabletserver/txthrottler/tx_throttler.go
timvaillancourt Mar 15, 2023
25bf05b
Merge branch 'main' into tx-throtter-default-all-cells
timvaillancourt Mar 15, 2023
62a9a26
Merge branch 'main' into tx-throtter-default-all-cells
timvaillancourt May 17, 2023
71ea246
Merge branch 'main' into tx-throtter-default-all-cells
timvaillancourt Jun 18, 2023
ed439e4
Move .GetKnownCells() call to .Open()
timvaillancourt Jun 19, 2023
d5c87cc
Update test
timvaillancourt Jun 19, 2023
84e0a4a
Improve err message
timvaillancourt Jun 19, 2023
9887e9f
Add code comment to explain 2nd cell check
timvaillancourt Jun 20, 2023
90d15b0
Add func fetchKnownCells()
timvaillancourt Jun 24, 2023
abc694d
Add test for fetchKnownCells
timvaillancourt Jun 24, 2023
1cf084d
Restart health stream if topo cells change
timvaillancourt Jun 24, 2023
29c496a
Update cell list when changed
timvaillancourt Jun 25, 2023
a0698dd
log.Fatalf -> Errorf
timvaillancourt Jun 25, 2023
8b287b8
Improve test
timvaillancourt Jun 25, 2023
0e8ae6f
use defer to stop ticker
timvaillancourt Jun 25, 2023
ab35c8b
PR suggestion
timvaillancourt Jun 27, 2023
2f5c44d
Add cellsFromTopo bool to txThrottlerState
timvaillancourt Jun 27, 2023
e7fa178
pass context.Context to .fetchKnownCells(...)
timvaillancourt Jun 28, 2023
d88c453
merge main
timvaillancourt Jul 25, 2023
e25e5ef
PR suggestion
timvaillancourt Jul 25, 2023
42bba79
fix defer-cancel() in loop
timvaillancourt Jul 25, 2023
e616a92
Past context to updateHealthCheckCells
timvaillancourt Jul 25, 2023
2a879ad
PR feedback
timvaillancourt Jul 27, 2023
4d8bff0
Dont update cells in config
timvaillancourt Jul 27, 2023
e7ac2c6
Add "the" to cli flag help + force ci to run again
timvaillancourt Jul 27, 2023
b70a1fa
Missing ts.config -> ts.config.healthCheckCells
timvaillancourt Jul 29, 2023
616f4a6
Fix unit test failure
timvaillancourt Jul 30, 2023
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
24 changes: 16 additions & 8 deletions go/vt/vttablet/tabletserver/txthrottler/tx_throttler.go
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,6 @@ limitations under the License.
package txthrottler

import (
"fmt"
"strings"
"sync"
"time"
Expand Down Expand Up @@ -114,10 +113,22 @@ func tryCreateTxThrottler(config *tabletenv.TabletConfig, topoServer *topo.Serve
return nil, err
}

// Clone tsv.TxThrottlerHealthCheckCells so that we don't assume tsv.TxThrottlerHealthCheckCells
// is immutable.
healthCheckCells := make([]string, len(config.TxThrottlerHealthCheckCells))
copy(healthCheckCells, config.TxThrottlerHealthCheckCells)
var healthCheckCells []string
if len(config.TxThrottlerHealthCheckCells) > 0 {
// Clone tsv.TxThrottlerHealthCheckCells so that we don't assume tsv.TxThrottlerHealthCheckCells
// is immutable.
healthCheckCells = make([]string, len(config.TxThrottlerHealthCheckCells))
copy(healthCheckCells, config.TxThrottlerHealthCheckCells)
timvaillancourt marked this conversation as resolved.
Show resolved Hide resolved
} else {
ctx, cancel := context.WithTimeout(context.Background(), topo.RemoteOperationTimeout)
defer cancel()

var err error
healthCheckCells, err = topoServer.GetKnownCells(ctx)
timvaillancourt marked this conversation as resolved.
Show resolved Hide resolved
if err != nil {
return nil, err
}
}

return newTxThrottler(&txThrottlerConfig{
enabled: true,
Expand Down Expand Up @@ -216,9 +227,6 @@ func newTxThrottler(config *txThrottlerConfig) (*TxThrottler, error) {
if err != nil {
return nil, err
}
if len(config.healthCheckCells) == 0 {
return nil, fmt.Errorf("empty healthCheckCells given. %+v", config)
}
}
return &TxThrottler{
config: config,
Expand Down
24 changes: 24 additions & 0 deletions go/vt/vttablet/tabletserver/txthrottler/tx_throttler_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@ import (
"time"

"github.com/golang/mock/gomock"
"github.com/stretchr/testify/assert"

"vitess.io/vitess/go/vt/discovery"
"vitess.io/vitess/go/vt/topo"
Expand Down Expand Up @@ -144,3 +145,26 @@ func TestEnabledThrottler(t *testing.T) {
}
throttler.Close()
}

func TestTryCreateTxThrottler(t *testing.T) {
allCells := []string{"cell1", "cell2", "cell3"}
ts := memorytopo.NewServer(allCells...)

config := tabletenv.NewDefaultConfig()
config.EnableTxThrottler = true

// Check tx throttler uses all known cells for healthchecks
// when TxThrottlerHealthCheckCells is empty/undef
{
txThrottler, err := tryCreateTxThrottler(config, ts)
assert.Nil(t, err)
assert.Equal(t, allCells, txThrottler.config.healthCheckCells)
}
// Check specified cells are used for healthchecks
{
config.TxThrottlerHealthCheckCells = []string{"cell1"}
txThrottler, err := tryCreateTxThrottler(config, ts)
assert.Nil(t, err)
assert.Equal(t, config.TxThrottlerHealthCheckCells, txThrottler.config.healthCheckCells)
}
}