Skip to content

Commit

Permalink
Ensure db connections are closed when disabling plugin
Browse files Browse the repository at this point in the history
Summary:
We were seeing some issues with db connections. Taking a look at recent plugin code shows a potential area where the connection may not be getting closed properly.
Defer in these cases aren't as ideal, since there are multiple db requests in a function. We just need to make sure we close the row if we're returning early due to an error.

Test Plan: existing tests pass

Reviewers: vihang

Reviewed By: vihang

Signed-off-by: Michelle Nguyen <[email protected]>

Differential Revision: https://phab.corp.pixielabs.ai/D11704

GitOrigin-RevId: 8d650e5
  • Loading branch information
aimichelle authored and copybaranaut committed Jun 29, 2022
1 parent d2edbc3 commit 4e66e86
Showing 1 changed file with 7 additions and 4 deletions.
11 changes: 7 additions & 4 deletions src/cloud/plugin/controllers/server.go
Original file line number Diff line number Diff line change
Expand Up @@ -282,6 +282,7 @@ func (s *Server) createPresetScripts(ctx context.Context, txn *sqlx.Tx, orgID uu
return status.Errorf(codes.Internal, "Failed to fetch plugin")
}

defer rows.Close()
var plugin RetentionPlugin
if rows.Next() {
err := rows.StructScan(&plugin)
Expand Down Expand Up @@ -312,7 +313,7 @@ func (s *Server) disableOrgRetention(ctx context.Context, txn *sqlx.Tx, orgID uu
if err != nil {
return status.Errorf(codes.Internal, "Failed to fetch scripts")
}

defer rows.Close()
for rows.Next() {
var id uuid.UUID
err = rows.Scan(&id)
Expand All @@ -334,7 +335,7 @@ func (s *Server) disableOrgRetention(ctx context.Context, txn *sqlx.Tx, orgID uu
if err != nil {
return status.Errorf(codes.Internal, "Failed to fetch custom export scripts")
}

defer rows.Close()
for rows.Next() {
var id uuid.UUID
err = rows.Scan(&id)
Expand Down Expand Up @@ -367,7 +368,7 @@ func (s *Server) updatePresetScripts(ctx context.Context, txn *sqlx.Tx, orgID uu
if err != nil {
return status.Errorf(codes.Internal, "Failed to fetch plugin")
}

defer rows.Close()
var plugin RetentionPlugin
if rows.Next() {
err := rows.StructScan(&plugin)
Expand Down Expand Up @@ -585,6 +586,7 @@ func (s *Server) UpdateOrgRetentionPluginConfig(ctx context.Context, req *plugin
var customExportURL *string
var insecureTLS bool
enabled := false
defer rows.Close()
if rows.Next() {
enabled = true
err := rows.Scan(&origVersion, &origConfig, &customExportURL, &insecureTLS)
Expand All @@ -603,6 +605,7 @@ func (s *Server) UpdateOrgRetentionPluginConfig(ctx context.Context, req *plugin
if err != nil {
return nil, status.Errorf(codes.Internal, "Failed to fetch plugin")
}
defer rows.Close()
var allowCustomExportURL bool
var allowInsecureTLS bool
if rows.Next() {
Expand Down Expand Up @@ -967,7 +970,7 @@ func (s *Server) UpdateRetentionScript(ctx context.Context, req *pluginpb.Update
if err != nil {
return nil, status.Errorf(codes.Internal, "Failed to fetch script")
}

defer rows.Close()
if !rows.Next() {
return nil, status.Error(codes.NotFound, "script not found")
}
Expand Down

0 comments on commit 4e66e86

Please sign in to comment.