Skip to content

Commit 4d2b66a

Browse files
ericpromislowericpromislow
authored
[2.14] Backport PR 1081: Close the prepared SQL-statements after use. (#1150)
1 parent a08c83b commit 4d2b66a

2 files changed

Lines changed: 87 additions & 51 deletions

File tree

‎pkg/sqlcache/store/store.go‎

Lines changed: 22 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -184,8 +184,12 @@ func (s *Store) updateExternalInfo(tx db.TxClient, key string, externalUpdateInf
184184
}
185185
rawStmt := fmt.Sprintf(`UPDATE "%s_fields" SET "%s" = ? WHERE key = ?`,
186186
labelDep.SourceGVK, labelDep.TargetFinalFieldName)
187-
preparedStmt := s.Prepare(rawStmt)
188-
_, err = tx.Stmt(preparedStmt).Exec(finalTargetValue, sourceKey)
187+
err = func() error {
188+
preparedStmt := s.Prepare(rawStmt)
189+
defer preparedStmt.Close()
190+
_, err := tx.Stmt(preparedStmt).Exec(finalTargetValue, sourceKey)
191+
return err
192+
}()
189193
if err != nil {
190194
logrus.Infof("Error running %s(%s, %s): %s", rawStmt, finalTargetValue, sourceKey, err)
191195
continue
@@ -205,8 +209,11 @@ func (s *Store) updateExternalInfo(tx db.TxClient, key string, externalUpdateInf
205209
nonLabelDep.TargetFinalFieldName)
206210
// TODO: Try to fold the two blocks together
207211

208-
getStmt := s.Prepare(rawGetStmt)
209-
rows, err := s.QueryForRows(s.ctx, getStmt)
212+
rows, err := func() (db.Rows, error) {
213+
getStmt := s.Prepare(rawGetStmt)
214+
defer getStmt.Close()
215+
return s.QueryForRows(s.ctx, getStmt)
216+
}()
210217
if err != nil {
211218
if !isDBError(err) {
212219
logrus.Infof("Error getting external info for table %s, key %s: %v", nonLabelDep.TargetGVK, key, err)
@@ -230,8 +237,12 @@ func (s *Store) updateExternalInfo(tx db.TxClient, key string, externalUpdateInf
230237
}
231238
rawStmt := fmt.Sprintf(`UPDATE "%s_fields" SET "%s" = ? WHERE key = ?`,
232239
nonLabelDep.SourceGVK, nonLabelDep.TargetFinalFieldName)
233-
preparedStmt := s.Prepare(rawStmt)
234-
_, err = tx.Stmt(preparedStmt).Exec(finalTargetValue, sourceKey)
240+
err = func() error {
241+
preparedStmt := s.Prepare(rawStmt)
242+
defer preparedStmt.Close()
243+
_, err := tx.Stmt(preparedStmt).Exec(finalTargetValue, sourceKey)
244+
return err
245+
}()
235246
if err != nil {
236247
logrus.Infof("Error running %s(%s, %s): %s", rawStmt, finalTargetValue, sourceKey, err)
237248
continue
@@ -251,8 +262,11 @@ func (s *Store) updateExternalInfo(tx db.TxClient, key string, externalUpdateInf
251262
func (s *Store) overrideCheck(finalFieldName, sourceGVK, sourceKey, finalTargetValue string) (bool, error) {
252263
rawGetValueStmt := fmt.Sprintf(`SELECT f."%s" FROM "%s_fields" f WHERE f.key = ?`,
253264
finalFieldName, sourceGVK)
254-
getValueStmt := s.Prepare(rawGetValueStmt)
255-
rows, err := s.QueryForRows(s.ctx, getValueStmt, sourceKey)
265+
rows, err := func() (db.Rows, error) {
266+
getValueStmt := s.Prepare(rawGetValueStmt)
267+
defer getValueStmt.Close()
268+
return s.QueryForRows(s.ctx, getValueStmt, sourceKey)
269+
}()
256270
if err != nil {
257271
logrus.Debugf("Checking the field, got error %s", err)
258272
return false, err

‎pkg/sqlcache/store/store_test.go‎

Lines changed: 65 additions & 43 deletions
Original file line numberDiff line numberDiff line change
@@ -802,38 +802,43 @@ func TestAddWithOneUpdate(t *testing.T) {
802802
WHERE (lt0.label = "field.cattle.io/projectId") AND f."spec.displayName" != ex2."spec.displayName"`
803803
args1 := []any{}
804804
c.EXPECT().Prepare(WSIgnoringMatcher(rawStmt)).Return(preparedStmt)
805-
c.EXPECT().QueryForRows(gomock.Any(), gomock.Any(), args1)
805+
c.EXPECT().QueryForRows(gomock.Any(), preparedStmt, args1)
806806
preparedStmt.EXPECT().Close()
807807
c.EXPECT().ReadStringsN(gomock.Any(), 2).Return([][]string{{"lego.cattle.io/fields1", "moose1"}}, nil)
808808
// Override check:
809809
rawStmt2 := `SELECT f."spec.displayName" FROM "_v1_Namespace_fields" f WHERE f.key = ?`
810-
c.EXPECT().Prepare(WSIgnoringMatcher(rawStmt2))
811-
c.EXPECT().QueryForRows(gomock.Any(), gomock.Any(), gomock.Any())
810+
c.EXPECT().Prepare(WSIgnoringMatcher(rawStmt2)).Return(preparedStmt)
811+
c.EXPECT().QueryForRows(gomock.Any(), preparedStmt, gomock.Any())
812+
preparedStmt.EXPECT().Close()
812813
c.EXPECT().ReadStrings(gomock.Any())
813814

814815
rawStmt2a := `UPDATE "_v1_Namespace_fields" SET "spec.displayName" = ? WHERE key = ?`
815-
c.EXPECT().Prepare(rawStmt2a)
816-
txC.EXPECT().Stmt(gomock.Any()).Return(stmts)
816+
c.EXPECT().Prepare(rawStmt2a).Return(preparedStmt)
817+
txC.EXPECT().Stmt(preparedStmt).Return(stmts)
818+
preparedStmt.EXPECT().Close()
817819
stmts.EXPECT().Exec("moose1", "lego.cattle.io/fields1")
818820

819821
rawStmt3 := `SELECT DISTINCT f.key, ex2."spec.projectName" FROM "_v1_Pods_fields" f
820822
JOIN "provisioner.cattle.io_v3_Cluster_fields" ex2 ON f."field.cattle.io/fixer" = ex2."metadata.name"
821823
WHERE f."spec.projectName" != ex2."spec.projectName"`
822-
c.EXPECT().Prepare(WSIgnoringMatcher(rawStmt3))
824+
c.EXPECT().Prepare(WSIgnoringMatcher(rawStmt3)).Return(preparedStmt)
823825
args2 := []any{}
824-
c.EXPECT().QueryForRows(gomock.Any(), gomock.Any(), args2)
826+
c.EXPECT().QueryForRows(gomock.Any(), preparedStmt, args2)
827+
preparedStmt.EXPECT().Close()
825828

826829
c.EXPECT().ReadStringsN(gomock.Any(), 2).Return([][]string{{"lego.cattle.io/fields2", "moose2"}}, nil)
827830
// Override check:
828831
rawStmt2 = `SELECT f."spec.projectName" FROM "_v1_Pods_fields" f WHERE f.key = ?`
829-
c.EXPECT().Prepare(WSIgnoringMatcher(rawStmt2))
830-
c.EXPECT().QueryForRows(gomock.Any(), gomock.Any(), gomock.Any())
832+
c.EXPECT().Prepare(WSIgnoringMatcher(rawStmt2)).Return(preparedStmt)
833+
c.EXPECT().QueryForRows(gomock.Any(), preparedStmt, gomock.Any())
834+
preparedStmt.EXPECT().Close()
831835
c.EXPECT().ReadStrings(gomock.Any())
832836

833837
rawStmt4 := `UPDATE "_v1_Pods_fields" SET "spec.projectName" = ? WHERE key = ?`
834-
c.EXPECT().Prepare(rawStmt4)
835-
txC.EXPECT().Stmt(gomock.Any()).Return(stmts)
838+
c.EXPECT().Prepare(rawStmt4).Return(preparedStmt)
839+
txC.EXPECT().Stmt(preparedStmt).Return(stmts)
836840
stmts.EXPECT().Exec("moose2", "lego.cattle.io/fields2")
841+
preparedStmt.EXPECT().Close()
837842

838843
err := store.Add(testObject)
839844
assert.Nil(t, err)
@@ -870,19 +875,22 @@ func TestAddWithExternalUpdates(t *testing.T) {
870875
WHERE (lt0.label = "field.cattle.io/projectId") AND f."spec.displayName" != ex2."spec.displayName"`
871876
c.EXPECT().Prepare(WSIgnoringMatcher(rawStmt)).Return(preparedStmt)
872877
args1 := []any{}
873-
c.EXPECT().QueryForRows(gomock.Any(), gomock.Any(), args1)
878+
c.EXPECT().QueryForRows(gomock.Any(), preparedStmt, args1)
874879
preparedStmt.EXPECT().Close()
875880
c.EXPECT().ReadStringsN(gomock.Any(), 2).Return([][]string{{"lego.cattle.io/fields1", "moose1"}}, nil)
876881

882+
// Override check:
877883
rawStmt1b := `SELECT f."spec.displayName" FROM "_v1_Namespace_fields" f WHERE f.key = ?`
878-
c.EXPECT().Prepare(WSIgnoringMatcher(rawStmt1b))
884+
c.EXPECT().Prepare(WSIgnoringMatcher(rawStmt1b)).Return(preparedStmt)
879885
args1b := []any{"lego.cattle.io/fields1"}
880-
c.EXPECT().QueryForRows(gomock.Any(), gomock.Any(), args1b)
886+
c.EXPECT().QueryForRows(gomock.Any(), preparedStmt, args1b)
887+
preparedStmt.EXPECT().Close()
881888
c.EXPECT().ReadStrings(gomock.Any()).Return([]string{"flipper"}, nil)
882889

883890
rawStmt2 := `UPDATE "_v1_Namespace_fields" SET "spec.displayName" = ? WHERE key = ?`
884-
c.EXPECT().Prepare(rawStmt2)
885-
txC.EXPECT().Stmt(gomock.Any()).Return(stmts)
891+
c.EXPECT().Prepare(rawStmt2).Return(preparedStmt)
892+
txC.EXPECT().Stmt(preparedStmt).Return(stmts)
893+
preparedStmt.EXPECT().Close()
886894
stmts.EXPECT().Exec("moose1", "lego.cattle.io/fields1")
887895

888896
rawStmt3 := `SELECT DISTINCT f.key, ex2."spec.projectName"
@@ -892,19 +900,22 @@ func TestAddWithExternalUpdates(t *testing.T) {
892900
c.EXPECT().Prepare(WSIgnoringMatcher(rawStmt3)).Return(preparedStmt)
893901
args2 := []any{}
894902
c.EXPECT().QueryForRows(gomock.Any(), preparedStmt, args2)
895-
// preparedStmt.EXPECT().Close()
903+
preparedStmt.EXPECT().Close()
896904
c.EXPECT().ReadStringsN(gomock.Any(), 2).Return([][]string{{"lego.cattle.io/fields2", "moose2"}}, nil)
897905

906+
// Override check:
898907
rawStmt3b := `SELECT f."spec.projectName" FROM "_v1_Pods_fields" f WHERE f.key = ?`
899-
c.EXPECT().Prepare(WSIgnoringMatcher(rawStmt3b))
908+
c.EXPECT().Prepare(WSIgnoringMatcher(rawStmt3b)).Return(preparedStmt)
900909
args3b := []any{"lego.cattle.io/fields2"}
901-
c.EXPECT().QueryForRows(gomock.Any(), gomock.Any(), args3b)
910+
c.EXPECT().QueryForRows(gomock.Any(), preparedStmt, args3b)
911+
preparedStmt.EXPECT().Close()
902912
c.EXPECT().ReadStrings(gomock.Any()).Return([]string{"snorkel"}, nil)
903913

904914
rawStmt4 := `UPDATE "_v1_Pods_fields" SET "spec.projectName" = ? WHERE key = ?`
905-
c.EXPECT().Prepare(rawStmt4)
906-
txC.EXPECT().Stmt(gomock.Any()).Return(stmts)
915+
c.EXPECT().Prepare(rawStmt4).Return(preparedStmt)
916+
txC.EXPECT().Stmt(preparedStmt).Return(stmts)
907917
stmts.EXPECT().Exec("moose2", "lego.cattle.io/fields2")
918+
preparedStmt.EXPECT().Close()
908919

909920
err := store.Add(testObject)
910921
assert.Nil(t, err)
@@ -948,40 +959,46 @@ func TestAddWithSelfUpdates(t *testing.T) {
948959
WHERE (lt0.label = "field.cattle.io/projectId") AND f."spec.displayName" != ex2."spec.displayName"`
949960
c.EXPECT().Prepare(WSIgnoringMatcher(rawStmt)).Return(preparedStmt)
950961
args1 := []any{}
951-
c.EXPECT().QueryForRows(gomock.Any(), gomock.Any(), args1)
962+
c.EXPECT().QueryForRows(gomock.Any(), preparedStmt, args1)
952963
preparedStmt.EXPECT().Close()
953964
c.EXPECT().ReadStringsN(gomock.Any(), 2).Return([][]string{{"lego.cattle.io/fields1", "moose1"}}, nil)
954965

955966
rawStmt1b := `SELECT f."spec.displayName" FROM "_v1_Namespace_fields" f WHERE f.key = ?`
956-
c.EXPECT().Prepare(WSIgnoringMatcher(rawStmt1b))
967+
c.EXPECT().Prepare(WSIgnoringMatcher(rawStmt1b)).Return(preparedStmt)
957968
args1b := []any{"lego.cattle.io/fields1"}
958-
c.EXPECT().QueryForRows(gomock.Any(), gomock.Any(), args1b)
969+
c.EXPECT().QueryForRows(gomock.Any(), preparedStmt, args1b)
970+
preparedStmt.EXPECT().Close()
959971
c.EXPECT().ReadStrings(gomock.Any()).Return([]string{"flipper"}, nil)
960972

961973
rawStmt2 := `UPDATE "_v1_Namespace_fields" SET "spec.displayName" = ? WHERE key = ?`
962-
c.EXPECT().Prepare(rawStmt2)
963-
txC.EXPECT().Stmt(gomock.Any()).Return(stmts)
974+
c.EXPECT().Prepare(rawStmt2).Return(preparedStmt)
975+
txC.EXPECT().Stmt(preparedStmt).Return(stmts)
976+
preparedStmt.EXPECT().Close()
964977
stmts.EXPECT().Exec("moose1", "lego.cattle.io/fields1")
965978

966979
rawStmt3 := `SELECT DISTINCT f.key, ex2."spec.projectName"
967980
FROM "_v1_Pods_fields" f JOIN "provisioner.cattle.io_v3_Cluster_fields" ex2
968981
ON f."field.cattle.io/fixer" = ex2."metadata.name"
969982
WHERE f."spec.projectName" != ex2."spec.projectName"`
970-
c.EXPECT().Prepare(WSIgnoringMatcher(rawStmt3))
983+
c.EXPECT().Prepare(WSIgnoringMatcher(rawStmt3)).Return(preparedStmt)
971984
args2 := []any{}
972-
c.EXPECT().QueryForRows(gomock.Any(), gomock.Any(), args2)
985+
c.EXPECT().QueryForRows(gomock.Any(), preparedStmt, args2)
986+
preparedStmt.EXPECT().Close()
973987
c.EXPECT().ReadStringsN(gomock.Any(), 2).Return([][]string{{"field.cattle.io/fixer", "moose1"}}, nil)
974988

989+
// Override check:
975990
rawStmt3b := `SELECT f."spec.projectName" FROM "_v1_Pods_fields" f WHERE f.key = ?`
976-
c.EXPECT().Prepare(WSIgnoringMatcher(rawStmt3b))
991+
c.EXPECT().Prepare(WSIgnoringMatcher(rawStmt3b)).Return(preparedStmt)
977992
args3b := []any{"field.cattle.io/fixer"}
978-
c.EXPECT().QueryForRows(gomock.Any(), gomock.Any(), args3b)
993+
c.EXPECT().QueryForRows(gomock.Any(), preparedStmt, args3b)
994+
preparedStmt.EXPECT().Close()
979995
c.EXPECT().ReadStrings(gomock.Any()).Return([]string{"snorkel"}, nil)
980996

981997
rawStmt4 := `UPDATE "_v1_Pods_fields" SET "spec.projectName" = ? WHERE key = ?`
982-
c.EXPECT().Prepare(rawStmt4)
983-
txC.EXPECT().Stmt(gomock.Any()).Return(stmts)
998+
c.EXPECT().Prepare(rawStmt4).Return(preparedStmt)
999+
txC.EXPECT().Stmt(preparedStmt).Return(stmts)
9841000
stmts.EXPECT().Exec("moose1", "field.cattle.io/fixer")
1001+
preparedStmt.EXPECT().Close()
9851002

9861003
err := store.Add(testObject)
9871004
assert.Nil(t, err)
@@ -1037,37 +1054,42 @@ func TestAddWithBothUpdates(t *testing.T) {
10371054
})
10381055
c.EXPECT().Prepare(WSIgnoringMatcher(rawStmt)).Return(preparedStmt)
10391056
args1 := []any{}
1040-
c.EXPECT().QueryForRows(gomock.Any(), gomock.Any(), args1)
1057+
c.EXPECT().QueryForRows(gomock.Any(), preparedStmt, args1)
10411058
preparedStmt.EXPECT().Close()
10421059
c.EXPECT().ReadStringsN(gomock.Any(), 2).Return([][]string{{"lego.cattle.io/fields1", "moose1"}}, nil)
10431060
// Override check:
10441061
rawStmt2 := `SELECT f."spec.displayName" FROM "_v1_Namespace_fields" f WHERE f.key = ?`
1045-
c.EXPECT().Prepare(WSIgnoringMatcher(rawStmt2))
1062+
c.EXPECT().Prepare(WSIgnoringMatcher(rawStmt2)).Return(preparedStmt)
10461063
args1b := []any{"lego.cattle.io/fields1"}
1047-
c.EXPECT().QueryForRows(gomock.Any(), gomock.Any(), args1b)
1064+
c.EXPECT().QueryForRows(gomock.Any(), preparedStmt, args1b)
1065+
preparedStmt.EXPECT().Close()
10481066
c.EXPECT().ReadStrings(gomock.Any())
10491067

10501068
rawStmt2a := `UPDATE "_v1_Namespace_fields" SET "spec.displayName" = ? WHERE key = ?`
1051-
c.EXPECT().Prepare(rawStmt2a)
1052-
txC.EXPECT().Stmt(gomock.Any()).Return(stmts)
1069+
c.EXPECT().Prepare(rawStmt2a).Return(preparedStmt)
1070+
txC.EXPECT().Stmt(preparedStmt).Return(stmts)
1071+
preparedStmt.EXPECT().Close()
10531072
stmts.EXPECT().Exec("moose1", "lego.cattle.io/fields1")
10541073

1055-
c.EXPECT().Prepare(WSIgnoringMatcher(rawStmt3))
1074+
c.EXPECT().Prepare(WSIgnoringMatcher(rawStmt3)).Return(preparedStmt)
10561075
args2 := []any{}
1057-
c.EXPECT().QueryForRows(gomock.Any(), gomock.Any(), args2)
1076+
c.EXPECT().QueryForRows(gomock.Any(), preparedStmt, args2)
1077+
preparedStmt.EXPECT().Close()
10581078

10591079
c.EXPECT().ReadStringsN(gomock.Any(), 2).Return([][]string{{"field.cattle.io/fixer", "moose1"}}, nil)
10601080
// Override check:
10611081
rawStmt2 = `SELECT f."spec.projectName" FROM "_v1_Pods_fields" f WHERE f.key = ?`
1062-
c.EXPECT().Prepare(WSIgnoringMatcher(rawStmt2))
1082+
c.EXPECT().Prepare(WSIgnoringMatcher(rawStmt2)).Return(preparedStmt)
10631083
args3b := []any{"field.cattle.io/fixer"}
1064-
c.EXPECT().QueryForRows(gomock.Any(), gomock.Any(), args3b)
1084+
c.EXPECT().QueryForRows(gomock.Any(), preparedStmt, args3b)
1085+
preparedStmt.EXPECT().Close()
10651086
c.EXPECT().ReadStrings(gomock.Any())
10661087

10671088
rawStmt4 := `UPDATE "_v1_Pods_fields" SET "spec.projectName" = ? WHERE key = ?`
1068-
c.EXPECT().Prepare(rawStmt4)
1069-
txC.EXPECT().Stmt(gomock.Any()).Return(stmts)
1089+
c.EXPECT().Prepare(rawStmt4).Return(preparedStmt)
1090+
txC.EXPECT().Stmt(preparedStmt).Return(stmts)
10701091
stmts.EXPECT().Exec("moose1", "field.cattle.io/fixer")
1092+
preparedStmt.EXPECT().Close()
10711093
// And again for the other object
10721094
}
10731095

0 commit comments

Comments
 (0)