Use db.Conn to fix postgres lock/unlock

[Teddy-Schmitz]

Hi, Inspired by #299, this fixes #296 for the postgres driver using the same method.

[jayd3e]

Thanks for this PR. I incorporated the fix into my codebase, however I'm still seeing the following error:

2017/12/07 01:15:27 errored while attempting to up sync the database:can't acquire lock

My migration Up/Down functions look like the following:

package db

import (
    "log"

    "github.com/mattes/migrate"
    _ "github.com/mattes/migrate/database/postgres"
    _ "github.com/mattes/migrate/source/file"
    "github.com/rainhq/rain/core"
)

func UpSync(config core.Configuration) {
    migration, err := migrate.New("file://"+config.MigrationsPath, config.Database.Url())
    if err != nil {
        log.Fatal(err)
    }
    defer migration.Close()

    if err := migration.Up(); err != nil {
        if err != migrate.ErrNoChange {
            migration.Close()
            log.Fatal("errored while attempting to up sync the database:", err)
        }
    }
}

func DownSync(config core.Configuration) {
    migration, err := migrate.New("file://"+config.MigrationsPath, config.Database.Url())
    if err != nil {
        log.Fatal(err)
    }
    defer migration.Close()

    if err := migration.Down(); err != nil {
        if err != migrate.ErrNoChange {
            migration.Close()
            log.Fatal("errored while attempting to down sync the database:", err)
        }
    }
}

Lastly, I do both a DownSync/UpSync operation before each test like the following:

g.Describe("GET /accounts", func() {
    g.BeforeEach(func() {
                db.UpSync(config)
                db.DownSync(config)
        })

    g.It("Should return a Forbidden response if unauthorized", func() {
        _, err := apiClient.Accounts()
        g.Assert(err == nil).IsFalse()

        assertions.assertClientError(err, 403, 403, "Forbidden")
    })

I'm pulling my hair out over this issue, as I've been working on it for most of the day. I initially tried a number of workarounds that involved using NewWithDatabaseInstance and closing the sql.DB instance after the migration was complete. This did not work unfortunately. I have now resorted to using a forked version of migrate with your fix applied as I mentioned, and I'm still seeing the error. Any thoughts?

[jayd3e]

One more piece of info is this has always worked on my local machine, but I'm trying to get it to work on Travis-CI. I'm not seeing the locking issues locally at all.

[jayd3e]

As a final note, I don’t always receive the error. The error gets thrown in one very specific spot consistently.

[Teddy-Schmitz]

You are using go1.9?

[jayd3e]

Correct 👍🏼

[Teddy-Schmitz]

hmm im not sure what to tell you really, postgres cant release the lock unless the request comes from the same connection. You can edit the code a bit to check the response from postgres during that one spot that always fails and see if its releasing the lock or not.

in postgres.go change this

func (p *Postgres) Unlock() error {
    if !p.isLocked {
        return nil
    }

    aid, err := database.GenerateAdvisoryLockId(p.config.DatabaseName)
    if err != nil {
        return err
    }

    query := `SELECT pg_advisory_unlock($1)`
    if _, err := p.db.ExecContext(context.Background(), query, aid); err != nil {
        return &database.Error{OrigErr: err, Query: []byte(query)}
    }
    p.isLocked = false
    return nil
}

to this

func (p *Postgres) Unlock() error {
    if !p.isLocked {
        return nil
    }

    aid, err := database.GenerateAdvisoryLockId(p.config.DatabaseName)
    if err != nil {
        return err
    }

    query := `SELECT pg_advisory_unlock($1)`
    var success bool
    if err := p.db.QueryRowContext(context.Background(), query, aid).Scan(&success); err != nil {
        return &database.Error{OrigErr: err, Query: []byte(query)}
    }

    fmt.Printf("Unlock was %v", success)
    p.isLocked = false
    return nil
}

and you can inspect the success bool to see if it was successful in unlocking. Maybe your accidentally calling the migration twice or hit some other race condition.

[jayd3e]

Interesting turn of events. It appears to always be returning true, even though running the actual migration fails when attempting to create the lock.

 19 tests complete (83 ms)
--- PASS: TestWithdrawTransactionService (0.09s)
PASS
Unlock was true
ok      github.com/org/project/service  5.635s
2017/12/09 03:50:54 errored while attempting to up sync the database:can't acquire lock
FAIL    github.com/org/project/server   0.020s
Unlock was true

[Teddy-Schmitz]

honestly the only thing i can think of is your somehow running the migration twice or your tests are running concurrently. Maybe pause execution of your test right before the error and go into postgres and see what connection is holding the lock.

[jayd3e]

That's a brilliant idea. Will try and report back 👍🏼

[dhui]

Fixed in forked release v3.2.0