Skip to content

Commit 3ae1c3c

Browse files
craig[bot]RaduBerinde
craig[bot]
and
RaduBerinde
committed
Merge #143804
143804: *: resolve remaining defer-in-loops r=RaduBerinde a=RaduBerinde Fixes: #137605 Release note: None Co-authored-by: Radu Berinde <[email protected]>
2 parents c712dba + e92993f commit 3ae1c3c

File tree

28 files changed

+389
-419
lines changed

28 files changed

+389
-419
lines changed

‎pkg/acceptance/cluster/dockercluster.go‎

Lines changed: 6 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -772,11 +772,12 @@ func (l *DockerCluster) stop(ctx context.Context) {
772772
fmt.Sprintf("stderr.%s.log", strings.Replace(
773773
timeutil.Now().Format(time.RFC3339), ":", "_", -1)))
774774
maybePanic(os.MkdirAll(filepath.Dir(file), 0755))
775-
w, err := os.Create(file)
776-
maybePanic(err)
777-
//nolint:deferloop TODO(#137605)
778-
defer w.Close()
779-
maybePanic(n.Logs(ctx, w))
775+
func() {
776+
w, err := os.Create(file)
777+
maybePanic(err)
778+
defer w.Close()
779+
maybePanic(n.Logs(ctx, w))
780+
}()
780781
log.Infof(ctx, "node %d: stderr at %s", i, file)
781782
if crashed {
782783
log.Infof(ctx, "~~~ node %d CRASHED ~~~~", i)

‎pkg/acceptance/localcluster/cluster.go‎

Lines changed: 10 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -725,13 +725,6 @@ func (n *Node) waitUntilLive(dur time.Duration) error {
725725
n.pgURL = pgURL.String()
726726
}()
727727

728-
var uiURL *url.URL
729-
730-
//nolint:deferloop TODO(#137605)
731-
defer func() {
732-
log.Infof(ctx, "process %d started (db: %s ui: %s)", pid, pgURL, uiURL)
733-
}()
734-
735728
// We're basically running, but (at least) the decommissioning test sometimes starts
736729
// up servers that can already be draining when they get here. For that reason, leave
737730
// the admin port undefined if we don't manage to get it.
@@ -744,21 +737,17 @@ func (n *Node) waitUntilLive(dur time.Duration) error {
744737
n.db = makeDB(n.pgURL, n.Cfg.NumWorkers, n.Cfg.DB)
745738
}()
746739

747-
{
748-
var uiStr string
749-
if err := n.db.QueryRow(
750-
`SELECT value FROM crdb_internal.node_runtime_info WHERE component='UI' AND field = 'URL'`,
751-
).Scan(&uiStr); err != nil {
752-
log.Infof(ctx, "%v", err)
753-
return nil
754-
}
755-
756-
_, uiURL, err = portFromURL(uiStr)
757-
if err != nil {
758-
log.Infof(ctx, "%v", err)
759-
// TODO(tschottdorf): see above.
760-
}
740+
var uiStr string
741+
var uiURL *url.URL
742+
if err := n.db.QueryRow(
743+
`SELECT value FROM crdb_internal.node_runtime_info WHERE component='UI' AND field = 'URL'`,
744+
).Scan(&uiStr); err != nil {
745+
log.Infof(ctx, "%v", err)
746+
} else if _, uiURL, err = portFromURL(uiStr); err != nil {
747+
log.Infof(ctx, "%v", err)
748+
// TODO(tschottdorf): see above.
761749
}
750+
log.Infof(ctx, "process %d started (db: %s ui: %s)", pid, pgURL, uiURL)
762751
return nil
763752
}
764753
return errors.Errorf("node %+v was unable to join cluster within %s", n.Cfg, dur)

‎pkg/backup/backupencryption/encryption.go‎

Lines changed: 2 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -176,14 +176,10 @@ func ValidateKMSURIsAgainstFullBackup(
176176
return nil, err
177177
}
178178

179-
//nolint:deferloop TODO(#137605)
180-
defer func() {
181-
_ = kms.Close()
182-
}()
183-
184179
// Depending on the KMS specific implementation, this may or may not contact
185180
// the remote KMS.
186181
id := kms.MasterKeyID()
182+
_ = kms.Close()
187183

188184
encryptedDataKey, err := kmsMasterKeyIDToDataKey.getEncryptedDataKey(PlaintextMasterKeyID(id))
189185
if err != nil {
@@ -457,10 +453,8 @@ func ReadEncryptionOptions(
457453
if err != nil {
458454
return nil, errors.Wrap(err, encryptionReadErrorMsg)
459455
}
460-
//nolint:deferloop TODO(#137605)
461-
defer r.Close(ctx)
462-
463456
encInfoBytes, err := ioctx.ReadAll(ctx, r)
457+
_ = r.Close(ctx)
464458
if err != nil {
465459
return nil, errors.Wrap(err, encryptionReadErrorMsg)
466460
}

‎pkg/backup/revision_reader.go‎

Lines changed: 35 additions & 38 deletions
Original file line numberDiff line numberDiff line change
@@ -50,51 +50,48 @@ func GetAllRevisions(
5050
exportResp := resp.(*kvpb.ExportResponse)
5151
var res []VersionedValues
5252
for _, file := range exportResp.Files {
53-
iterOpts := storage.IterOptions{
54-
KeyTypes: storage.IterKeyTypePointsOnly,
55-
LowerBound: file.Span.Key,
56-
UpperBound: file.Span.EndKey,
57-
}
58-
iter, err := storage.NewMemSSTIterator(file.SST, true, iterOpts)
59-
if err != nil {
60-
return err
61-
}
62-
//nolint:deferloop TODO(#137605)
63-
defer func() {
64-
if iter != nil {
65-
iter.Close()
53+
err := func() error {
54+
iterOpts := storage.IterOptions{
55+
KeyTypes: storage.IterKeyTypePointsOnly,
56+
LowerBound: file.Span.Key,
57+
UpperBound: file.Span.EndKey,
6658
}
67-
}()
68-
iter.SeekGE(storage.MVCCKey{Key: startKey})
59+
iter, err := storage.NewMemSSTIterator(file.SST, true, iterOpts)
60+
if err != nil {
61+
return err
62+
}
63+
defer iter.Close()
64+
iter.SeekGE(storage.MVCCKey{Key: startKey})
6965

70-
for ; ; iter.Next() {
71-
if valid, err := iter.Valid(); !valid || err != nil {
66+
for ; ; iter.Next() {
67+
if valid, err := iter.Valid(); !valid || err != nil {
68+
if err != nil {
69+
return err
70+
}
71+
break
72+
} else if iter.UnsafeKey().Key.Compare(endKey) >= 0 {
73+
break
74+
}
75+
key := iter.UnsafeKey()
76+
keyCopy := make([]byte, len(key.Key))
77+
copy(keyCopy, key.Key)
78+
key.Key = keyCopy
79+
v, err := iter.UnsafeValue()
7280
if err != nil {
7381
return err
7482
}
75-
break
76-
} else if iter.UnsafeKey().Key.Compare(endKey) >= 0 {
77-
break
78-
}
79-
key := iter.UnsafeKey()
80-
keyCopy := make([]byte, len(key.Key))
81-
copy(keyCopy, key.Key)
82-
key.Key = keyCopy
83-
v, err := iter.UnsafeValue()
84-
if err != nil {
85-
return err
86-
}
87-
value := make([]byte, len(v))
88-
copy(value, v)
89-
if len(res) == 0 || !res[len(res)-1].Key.Equal(key.Key) {
90-
res = append(res, VersionedValues{Key: key.Key})
83+
value := make([]byte, len(v))
84+
copy(value, v)
85+
if len(res) == 0 || !res[len(res)-1].Key.Equal(key.Key) {
86+
res = append(res, VersionedValues{Key: key.Key})
87+
}
88+
res[len(res)-1].Values = append(res[len(res)-1].Values, roachpb.Value{Timestamp: key.Timestamp, RawBytes: value})
9189
}
92-
res[len(res)-1].Values = append(res[len(res)-1].Values, roachpb.Value{Timestamp: key.Timestamp, RawBytes: value})
90+
return nil
91+
}()
92+
if err != nil {
93+
return err
9394
}
94-
95-
// Close and nil out the iter to release the underlying resources.
96-
iter.Close()
97-
iter = nil
9895
}
9996

10097
select {

‎pkg/backup/show.go‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1223,11 +1223,10 @@ func backupShowerFileSetup() backupShower {
12231223
if err != nil {
12241224
return nil, err
12251225
}
1226-
//nolint:deferloop TODO(#137605)
1227-
defer it.Close()
12281226
var idx int
12291227
for ; ; it.Next() {
12301228
if ok, err := it.Valid(); err != nil {
1229+
it.Close()
12311230
return nil, err
12321231
} else if !ok {
12331232
break
@@ -1260,6 +1259,7 @@ func backupShowerFileSetup() backupShower {
12601259
})
12611260
idx++
12621261
}
1262+
it.Close()
12631263
}
12641264
return rows, nil
12651265
},

‎pkg/ccl/changefeedccl/event_processing.go‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -702,9 +702,9 @@ func (c *parallelEventConsumer) workerLoop(
702702
return nil
703703
case <-c.termCh:
704704
c.mu.Lock()
705-
//nolint:deferloop TODO(#137605)
706-
defer c.mu.Unlock()
707-
return c.mu.termErr
705+
termErr := c.mu.termErr
706+
c.mu.Unlock()
707+
return termErr
708708
case e := <-c.workerCh[id]:
709709
if err := consumer.ConsumeEvent(ctx, e); err != nil {
710710
return c.setWorkerError(err)

‎pkg/ccl/changefeedccl/schemafeed/schematestutils/schema_test_utils.go‎

Lines changed: 47 additions & 43 deletions
Original file line numberDiff line numberDiff line change
@@ -166,54 +166,58 @@ func FetchDescVersionModificationTime(
166166
t.Fatal(pErr.GoError())
167167
}
168168
for _, file := range res.(*kvpb.ExportResponse).Files {
169-
it, err := storage.NewMemSSTIterator(file.SST, false /* verify */, storage.IterOptions{
170-
KeyTypes: storage.IterKeyTypePointsAndRanges,
171-
LowerBound: keys.MinKey,
172-
UpperBound: keys.MaxKey,
173-
})
174-
if err != nil {
175-
t.Fatal(err)
176-
}
177-
//nolint:deferloop TODO(#137605)
178-
defer it.Close()
179-
for it.SeekGE(storage.NilKey); ; it.Next() {
180-
if ok, err := it.Valid(); err != nil {
181-
t.Fatal(err)
182-
} else if !ok {
183-
continue
184-
}
185-
k := it.UnsafeKey()
186-
if _, hasRange := it.HasPointAndRange(); hasRange {
187-
t.Fatalf("unexpected MVCC range key at %s", k)
188-
}
189-
remaining, _, _, err := s.Codec().DecodeIndexPrefix(k.Key)
190-
if err != nil {
191-
t.Fatal(err)
192-
}
193-
_, tableID, err := encoding.DecodeUvarintAscending(remaining)
169+
ts, found := func() (hlc.Timestamp, bool) {
170+
it, err := storage.NewMemSSTIterator(file.SST, false /* verify */, storage.IterOptions{
171+
KeyTypes: storage.IterKeyTypePointsAndRanges,
172+
LowerBound: keys.MinKey,
173+
UpperBound: keys.MaxKey,
174+
})
194175
if err != nil {
195176
t.Fatal(err)
196177
}
197-
if tableID != uint64(dropColTblID) {
198-
continue
199-
}
200-
unsafeValue, err := it.UnsafeValue()
201-
require.NoError(t, err)
202-
if unsafeValue == nil {
203-
t.Fatal(errors.New(`value was dropped or truncated`))
204-
}
205-
value := roachpb.Value{RawBytes: unsafeValue, Timestamp: k.Timestamp}
206-
b, err := descbuilder.FromSerializedValue(&value)
207-
if err != nil {
208-
t.Fatal(err)
209-
}
210-
require.NotNil(t, b)
211-
if b.DescriptorType() == catalog.Table {
212-
tbl := b.BuildImmutable().(catalog.TableDescriptor)
213-
if int(tbl.GetVersion()) == version {
214-
return tbl.GetModificationTime()
178+
defer it.Close()
179+
for it.SeekGE(storage.NilKey); ; it.Next() {
180+
if ok, err := it.Valid(); err != nil {
181+
t.Fatal(err)
182+
} else if !ok {
183+
return hlc.Timestamp{}, false
184+
}
185+
k := it.UnsafeKey()
186+
if _, hasRange := it.HasPointAndRange(); hasRange {
187+
t.Fatalf("unexpected MVCC range key at %s", k)
188+
}
189+
remaining, _, _, err := s.Codec().DecodeIndexPrefix(k.Key)
190+
if err != nil {
191+
t.Fatal(err)
192+
}
193+
_, tableID, err := encoding.DecodeUvarintAscending(remaining)
194+
if err != nil {
195+
t.Fatal(err)
196+
}
197+
if tableID != uint64(dropColTblID) {
198+
continue
199+
}
200+
unsafeValue, err := it.UnsafeValue()
201+
require.NoError(t, err)
202+
if unsafeValue == nil {
203+
t.Fatal(errors.New(`value was dropped or truncated`))
204+
}
205+
value := roachpb.Value{RawBytes: unsafeValue, Timestamp: k.Timestamp}
206+
b, err := descbuilder.FromSerializedValue(&value)
207+
if err != nil {
208+
t.Fatal(err)
209+
}
210+
require.NotNil(t, b)
211+
if b.DescriptorType() == catalog.Table {
212+
tbl := b.BuildImmutable().(catalog.TableDescriptor)
213+
if int(tbl.GetVersion()) == version {
214+
return tbl.GetModificationTime(), true
215+
}
215216
}
216217
}
218+
}()
219+
if found {
220+
return ts
217221
}
218222
}
219223
t.Fatal(errors.New(`couldn't find table desc for given version`))

‎pkg/cli/clisqlshell/sql.go‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -2268,14 +2268,14 @@ func (c *cliState) doRunShell(state cliStateEnum, cmdIn, cmdOut, cmdErr *os.File
22682268
}
22692269
switch state {
22702270
case cliStart:
2271-
//nolint:deferloop TODO(#137605)
2271+
//nolint:deferloop
22722272
defer func() {
22732273
if err := c.closeOutputFile(); err != nil {
22742274
fmt.Fprintf(cmdErr, "warning: closing output file: %v\n", err)
22752275
}
22762276
}()
22772277
cleanupFn, err := c.configurePreShellDefaults(cmdIn, cmdOut, cmdErr)
2278-
//nolint:deferloop TODO(#137605)
2278+
//nolint:deferloop
22792279
defer cleanupFn()
22802280
if err != nil {
22812281
return err

‎pkg/cli/debug.go‎

Lines changed: 19 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -878,24 +878,25 @@ func runDebugGCCmd(cmd *cobra.Command, args []string) error {
878878
}
879879

880880
for _, desc := range descs {
881-
snap := db.NewSnapshot()
882-
//nolint:deferloop TODO(#137605)
883-
defer snap.Close()
884-
now := hlc.Timestamp{WallTime: timeutil.Now().UnixNano()}
885-
thresh := gc.CalculateThreshold(now, gcTTL)
886-
info, err := gc.Run(
887-
context.Background(),
888-
&desc, snap,
889-
now, thresh,
890-
gc.RunOptions{
891-
LockAgeThreshold: lockAgeThreshold,
892-
MaxLocksPerIntentCleanupBatch: lockBatchSize,
893-
TxnCleanupThreshold: txnCleanupThreshold,
894-
},
895-
gcTTL, gc.NoopGCer{},
896-
func(_ context.Context, _ []roachpb.Lock) error { return nil },
897-
func(_ context.Context, _ *roachpb.Transaction) error { return nil },
898-
)
881+
info, err := func() (gc.Info, error) {
882+
snap := db.NewSnapshot()
883+
defer snap.Close()
884+
now := hlc.Timestamp{WallTime: timeutil.Now().UnixNano()}
885+
thresh := gc.CalculateThreshold(now, gcTTL)
886+
return gc.Run(
887+
context.Background(),
888+
&desc, snap,
889+
now, thresh,
890+
gc.RunOptions{
891+
LockAgeThreshold: lockAgeThreshold,
892+
MaxLocksPerIntentCleanupBatch: lockBatchSize,
893+
TxnCleanupThreshold: txnCleanupThreshold,
894+
},
895+
gcTTL, gc.NoopGCer{},
896+
func(_ context.Context, _ []roachpb.Lock) error { return nil },
897+
func(_ context.Context, _ *roachpb.Transaction) error { return nil },
898+
)
899+
}()
899900
if err != nil {
900901
return err
901902
}

‎pkg/cli/debug_recover_loss_of_quorum.go‎

Lines changed: 2 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -819,10 +819,8 @@ func applyRecoveryToLocalStore(
819819
}
820820
stores[i] = store
821821
batch := store.NewBatch()
822-
//nolint:deferloop TODO(#137605)
823-
defer store.Close()
824-
//nolint:deferloop TODO(#137605)
825-
defer batch.Close()
822+
defer store.Close() //nolint:deferloop
823+
defer batch.Close() //nolint:deferloop
826824

827825
storeIdent, err := kvstorage.ReadStoreIdent(ctx, store)
828826
if err != nil {

0 commit comments

Comments
 (0)