8000 Fix socket leak, clean up single use postgres databases (#2413) · coder/coder@a82c0eb · GitHub
[go: up one dir, main page]
Skip to content

    • Commit a82c0eb

    Browse files
    spikecurtisspikecurtis
    authored
    Fix socket leak, clean up single use postgres databases (#2413)
    * Fix socket leak, clean up single use postgres databases Signed-off-by: Spike Curtis <spike@coder.com> * Move migrate close defer until after we know it is not nil Signed-off-by: Spike Curtis <spike@coder.com>
    1 parent eab5c15 commit a82c0eb

    File tree

    4 files changed

    +38
    -9
    lines changed

    4 files changed

    +38
    -9
    lines changed

    ‎Makefile

    Lines changed: 2 additions & 2 deletions
    Original file line numberDiff line numberDiff line change
    @@ -115,9 +115,9 @@ test: test-clean
    115115
    .PHONY: test-postgres
    116116
    test-postgres: test-clean
    117117
    DB=ci gotestsum --junitfile="gotests.xml" --packages="./..." -- \
    118-
    -covermode=atomic -coverprofile="gotests.coverage" -timeout=10m \
    118+
    -covermode=atomic -coverprofile="gotests.coverage" -timeout=30m \
    119119
    -coverpkg=./...,github.com/coder/coder/codersdk \
    120-
    -count=1 -parallel=1 -race -failfast
    120+
    -count=1 -race -failfast
    121121

    122122

    123123
    .PHONY: test-postgres-docker

    ‎cli/list_test.go

    Lines changed: 6 additions & 4 deletions
    Original file line numberDiff line numberDiff line change
    @@ -5,7 +5,7 @@ import (
    55
    "testing"
    66
    "time"
    77

    8-
    "github.com/stretchr/testify/require"
    8+
    "github.com/stretchr/testify/assert"
    99

    1010
    "github.com/coder/coder/cli/clitest"
    1111
    "github.com/coder/coder/coderd/coderdtest"
    @@ -30,13 +30,15 @@ func TestList(t *testing.T) {
    3030
    pty := ptytest.New(t)
    3131
    cmd.SetIn(pty.Input())
    3232
    cmd.SetOut(pty.Output())
    33-
    errC := make(chan error)
    33+
    done := make(chan any)
    3434
    go func() {
    35-
    errC <- cmd.ExecuteContext(ctx)
    35+
    errC := cmd.ExecuteContext(ctx)
    36+
    assert.NoError(t, errC)
    37+
    close(done)
    3638
    }()
    3739
    pty.ExpectMatch(workspace.Name)
    3840
    pty.ExpectMatch("Running")
    3941
    cancelFunc()
    40-
    require.NoError(t, <-errC)
    42+
    <-done
    4143
    })
    4244
    }

    ‎coderd/database/migrate.go

    Lines changed: 23 additions & 2 deletions
    Original file line numberDiff line numberDiff line change
    @@ -1,6 +1,7 @@
    11
    package database
    22

    33
    import (
    4+
    "context"
    45
    "database/sql"
    56
    "embed"
    67
    "errors"
    @@ -17,12 +18,21 @@ import (
    1718
    var migrations embed.FS
    1819

    1920
    func migrateSetup(db *sql.DB) (source.Driver, *migrate.Migrate, error) {
    21+
    ctx := context.Background()
    2022
    sourceDriver, err := iofs.New(migrations, "migrations")
    2123
    if err != nil {
    2224
    return nil, nil, xerrors.Errorf("create iofs: %w", err)
    2325
    }
    2426

    25-
    dbDriver, err := postgres.WithInstance(db, &postgres.Config{})
    27+
    // there is a postgres.WithInstance() method that takes the DB instance,
    28+
    // but, when you close the resulting Migrate, it closes the DB, which
    29+
    // we don't want. Instead, create just a connection that will get closed
    30+
    // when migration is done.
    31+
    conn, err := db.Conn(ctx)
    32+
    if err != nil {
    33+
    return nil, nil, xerrors.Errorf("postgres connection: %w", err)
    34+
    }
    35+
    dbDriver, err := postgres.WithConnection(ctx, conn, &postgres.Config{})
    2636
    if err != nil {
    2737
    return nil, nil, xerrors.Errorf("wrap postgres connection: %w", err)
    2838
    }
    @@ -36,11 +46,22 @@ func migrateSetup(db *sql.DB) (source.Driver, *migrate.Migrate, error) {
    3646
    }
    3747

    3848
    // MigrateUp runs SQL migrations to ensure the database schema is up-to-date.
    39-
    func MigrateUp(db *sql.DB) error {
    49+
    func MigrateUp(db *sql.DB) (retErr error) {
    4050
    _, m, err := migrateSetup(db)
    4151
    if err != nil {
    4252
    return xerrors.Errorf("migrate setup: %w", err)
    4353
    }
    54+
    defer func() {
    55+
    srcErr, dbErr := m.Close()
    56+
    if retErr != nil {
    57+
    return
    58+
    }
    59+
    if dbErr != nil {
    60+
    retErr = dbErr
    61+
    return
    62+
    }
    63+
    retErr = srcErr
    64+
    }()
    4465

    4566
    err = m.Up()
    4667
    if err != nil {

    ‎coderd/database/postgres/postgres.go

    Lines changed: 7 additions & 1 deletion
    Original file line numberDiff line numberDiff line change
    @@ -44,7 +44,13 @@ func Open() (string, func(), error) {
    4444
    return "", nil, xerrors.Errorf("create db: %w", err)
    4545
    }
    4646

    47-
    return "postgres://postgres:postgres@127.0.0.1:5432/" + dbName + "?sslmode=disable", func() {}, nil
    47+
    deleteDB := func() {
    48+
    ddb, _ := sql.Open("postgres", dbURL)
    49+
    defer ddb.Close()
    50+
    _, _ = ddb.Exec("DROP DATABASE " + dbName)
    51+
    }
    52+
    53+
    return "postgres://postgres:postgres@127.0.0.1:5432/" + dbName + "?sslmode=disable", deleteDB, nil
    4854
    }
    4955

    5056
    pool, err := dockertest.NewPool("")

    0 commit comments

    Comments
     (0)
    0