Skip to content

Commit 8e01681

Browse files
committed
feat: use options instead of many arguments
Signed-off-by: Manan Gupta <manan@planetscale.com>
1 parent d453821 commit 8e01681

File tree

3 files changed

+20
-17
lines changed

3 files changed

+20
-17
lines changed

go/vt/vtctl/reparentutil/planned_reparenter.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -182,7 +182,7 @@ func (pr *PlannedReparenter) preflightChecks(
182182
}
183183

184184
event.DispatchUpdate(ev, "electing a primary candidate")
185-
opts.NewPrimaryAlias, err = ElectNewPrimary(ctx, pr.tmc, &ev.ShardInfo, tabletMap, innodbBufferPoolData, opts.NewPrimaryAlias, opts.AvoidPrimaryAlias, opts.WaitReplicasTimeout, opts.TolerableReplLag, opts.durability, opts.AllowCrossCellPromotion, pr.logger)
185+
opts.NewPrimaryAlias, err = ElectNewPrimary(ctx, pr.tmc, &ev.ShardInfo, tabletMap, innodbBufferPoolData, opts, pr.logger)
186186
if err != nil {
187187
return true, err
188188
}

go/vt/vtctl/reparentutil/util.go

Lines changed: 9 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -69,12 +69,7 @@ func ElectNewPrimary(
6969
shardInfo *topo.ShardInfo,
7070
tabletMap map[string]*topo.TabletInfo,
7171
innodbBufferPoolData map[string]int,
72-
newPrimaryAlias *topodatapb.TabletAlias,
73-
avoidPrimaryAlias *topodatapb.TabletAlias,
74-
waitReplicasTimeout time.Duration,
75-
tolerableReplLag time.Duration,
76-
durability Durabler,
77-
allowCrossCellPromotion bool,
72+
opts *PlannedReparentOptions,
7873
// (TODO:@ajm188) it's a little gross we need to pass this, maybe embed in the context?
7974
logger logutil.Logger,
8075
) (*topodatapb.TabletAlias, error) {
@@ -99,16 +94,16 @@ func ElectNewPrimary(
9994
reasonsToInvalidate := strings.Builder{}
10095
for _, tablet := range tabletMap {
10196
switch {
102-
case newPrimaryAlias != nil:
97+
case opts.NewPrimaryAlias != nil:
10398
// If newPrimaryAlias is provided, then that is the only valid tablet, even if it is not of type replica or in a different cell.
104-
if !topoproto.TabletAliasEqual(tablet.Alias, newPrimaryAlias) {
99+
if !topoproto.TabletAliasEqual(tablet.Alias, opts.NewPrimaryAlias) {
105100
reasonsToInvalidate.WriteString(fmt.Sprintf("\n%v does not match the new primary alias provided", topoproto.TabletAliasString(tablet.Alias)))
106101
continue
107102
}
108-
case !allowCrossCellPromotion && primaryCell != "" && tablet.Alias.Cell != primaryCell:
103+
case !opts.AllowCrossCellPromotion && primaryCell != "" && tablet.Alias.Cell != primaryCell:
109104
reasonsToInvalidate.WriteString(fmt.Sprintf("\n%v is not in the same cell as the previous primary", topoproto.TabletAliasString(tablet.Alias)))
110105
continue
111-
case avoidPrimaryAlias != nil && topoproto.TabletAliasEqual(tablet.Alias, avoidPrimaryAlias):
106+
case opts.AvoidPrimaryAlias != nil && topoproto.TabletAliasEqual(tablet.Alias, opts.AvoidPrimaryAlias):
112107
reasonsToInvalidate.WriteString(fmt.Sprintf("\n%v matches the primary alias to avoid", topoproto.TabletAliasString(tablet.Alias)))
113108
continue
114109
case tablet.Tablet.Type != topodatapb.TabletType_REPLICA:
@@ -123,18 +118,18 @@ func ElectNewPrimary(
123118
// then we don't need to find the position of the said tablet for sorting.
124119
// We can just return the tablet quickly.
125120
// This check isn't required, but it saves us an RPC call that is otherwise unnecessary.
126-
if len(candidates) == 1 && tolerableReplLag == 0 {
121+
if len(candidates) == 1 && opts.TolerableReplLag == 0 {
127122
return candidates[0].Alias, nil
128123
}
129124

130125
for _, tablet := range candidates {
131126
tb := tablet
132127
errorGroup.Go(func() error {
133128
// find and store the positions for the tablet
134-
pos, replLag, err := findPositionAndLagForTablet(groupCtx, tb, logger, tmc, waitReplicasTimeout)
129+
pos, replLag, err := findPositionAndLagForTablet(groupCtx, tb, logger, tmc, opts.WaitReplicasTimeout)
135130
mu.Lock()
136131
defer mu.Unlock()
137-
if err == nil && (tolerableReplLag == 0 || tolerableReplLag >= replLag) {
132+
if err == nil && (opts.TolerableReplLag == 0 || opts.TolerableReplLag >= replLag) {
138133
validTablets = append(validTablets, tb)
139134
tabletPositions = append(tabletPositions, pos)
140135
innodbBufferPool = append(innodbBufferPool, innodbBufferPoolData[topoproto.TabletAliasString(tb.Alias)])
@@ -156,7 +151,7 @@ func ElectNewPrimary(
156151
}
157152

158153
// sort the tablets for finding the best primary
159-
err = sortTabletsForReparent(validTablets, tabletPositions, innodbBufferPool, durability)
154+
err = sortTabletsForReparent(validTablets, tabletPositions, innodbBufferPool, opts.durability)
160155
if err != nil {
161156
return nil, err
162157
}

go/vt/vtctl/reparentutil/util_test.go

Lines changed: 10 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -71,7 +71,7 @@ func TestElectNewPrimary(t *testing.T) {
7171
tmc *chooseNewPrimaryTestTMClient
7272
shardInfo *topo.ShardInfo
7373
tabletMap map[string]*topo.TabletInfo
74-
innodbBufferPoolData map[string]int
74+
innodbBufferPoolData map[string]int
7575
newPrimaryAlias *topodatapb.TabletAlias
7676
avoidPrimaryAlias *topodatapb.TabletAlias
7777
tolerableReplLag time.Duration
@@ -853,7 +853,15 @@ zone1-0000000100 is not a replica`,
853853
t.Run(tt.name, func(t *testing.T) {
854854
t.Parallel()
855855

856-
actual, err := ElectNewPrimary(ctx, tt.tmc, tt.shardInfo, tt.tabletMap, tt.innodbBufferPoolData, tt.newPrimaryAlias, tt.avoidPrimaryAlias, time.Millisecond*50, tt.tolerableReplLag, durability, tt.allowCrossCellPromotion, logger)
856+
options := &PlannedReparentOptions{
857+
NewPrimaryAlias: tt.newPrimaryAlias,
858+
AvoidPrimaryAlias: tt.avoidPrimaryAlias,
859+
TolerableReplLag: tt.tolerableReplLag,
860+
durability: durability,
861+
AllowCrossCellPromotion: tt.allowCrossCellPromotion,
862+
WaitReplicasTimeout: time.Millisecond * 50,
863+
}
864+
actual, err := ElectNewPrimary(ctx, tt.tmc, tt.shardInfo, tt.tabletMap, tt.innodbBufferPoolData, options, logger)
857865
if len(tt.errContains) > 0 {
858866
for _, errC := range tt.errContains {
859867
assert.ErrorContains(t, err, errC)

0 commit comments

Comments
 (0)