Navigation Menu

Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add advisory locking to mongodb #448

Merged
merged 3 commits into from Sep 26, 2020
Merged

Conversation

andyN42
Copy link
Contributor

@andyN42 andyN42 commented Sep 21, 2020

Resolves #435

This implements advisory locking for mongodb. The migrate_advisory_lock collection will have a unique index on the locking_key collection. Migrate processes will attempt to claim the lock by inserting the same unique value into the collection. If succesful, they have the lock, if not, they wait for up to 15s using an exponential backoff.

Note that the collection name and backoff max time are configurable with the following query params:

URL Query WithInstance Config Description
x-advisory-locking true Feature flag for advisory locking, if set to false, disable advisory locking
x-advisory-lock-collection migrate_advisory_lock The name of the collection to use for advisory locking
x-advisory-lock-timout 15 The max time in seconds that the advisory lock will wait if the db is already locked.
x-advisory-lock-timout-interval 10 The max timeout in seconds interval that the advisory lock will wait if the db is already locked.

@andyN42 andyN42 force-pushed the locking branch 2 times, most recently from 0f9cfff to 0ad2f21 Compare September 21, 2020 19:01
@coveralls
Copy link

coveralls commented Sep 21, 2020

Pull Request Test Coverage Report for Build 870

  • 95 of 134 (70.9%) changed or added relevant lines in 1 file are covered.
  • No unchanged relevant lines lost coverage.
  • Overall coverage increased (+0.4%) to 54.246%

Changes Missing Coverage Covered Lines Changed/Added Lines %
database/mongodb/mongodb.go 95 134 70.9%
Totals Coverage Status
Change from base Build 863: 0.4%
Covered Lines: 2868
Relevant Lines: 5287

💛 - Coveralls

Copy link
Member

@dhui dhui left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the PR!

database/mongodb/mongodb.go Outdated Show resolved Hide resolved
database/mongodb/mongodb.go Outdated Show resolved Hide resolved
database/mongodb/mongodb.go Show resolved Hide resolved
database/mongodb/mongodb.go Outdated Show resolved Hide resolved
database/mongodb/mongodb.go Outdated Show resolved Hide resolved
database/mongodb/mongodb_test.go Show resolved Hide resolved
database/mongodb/mongodb.go Outdated Show resolved Hide resolved
database/mongodb/mongodb.go Outdated Show resolved Hide resolved
database/mongodb/mongodb.go Outdated Show resolved Hide resolved
database/mongodb/mongodb.go Outdated Show resolved Hide resolved
rename constants
rename query params
add new timeout interval query param
add new feature flag
use timeout context
database/mongodb/mongodb.go Outdated Show resolved Hide resolved
database/mongodb/mongodb.go Outdated Show resolved Hide resolved
@andyN42
Copy link
Contributor Author

andyN42 commented Sep 25, 2020

@dhui I am seeing intermittent cassandra test failures, unrelated to my changes. Is this a known issue?

https://travis-ci.com/github/golang-migrate/migrate/jobs/391121339

    --- PASS: TestMigrate/cassandra:3.0 (26.99s)
    --- PASS: TestMigrate/cassandra:3.11 (27.69s)
FAIL
coverage: 49.3% of statements
exit status 1
FAIL	github.com/golang-migrate/migrate/v4/database/cassandra	78.845s
?   	github.com/golang-migrate/migrate/v4/database/clickhouse	[no test files]
=== RUN   Test

Edit: force pushed whitespace changes to my branch twice. First time, same issue, second time, build passed

Copy link
Member

@dhui dhui left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for addressing the prev round of feedback.

Re: failing tests
Yeah, unfortunately, some of the integration tests fail intermittently... e.g. docker pull fails or container fails to start 😞
Force pushing is the best way to re-trigger a build.

database/mongodb/mongodb.go Show resolved Hide resolved
database/mongodb/mongodb.go Show resolved Hide resolved
})
if err != nil {
return nil, err
}
return mc, nil
}

//Parse the url param, convert it to boolean
// returns error if param invalid. returns defaultValue if param not present
func parseBoolean(urlParam string, defaultValue bool) (bool, error) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice, thanks for the refactors!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Implement Locking with MongoDB
4 participants