From 18dd02bc3122e792f69ceaa5baf688075fa7e1ac Mon Sep 17 00:00:00 2001 From: Steve Ramage Date: Wed, 27 Oct 2021 09:12:01 -0700 Subject: [PATCH 1/7] Resolves #647 - Fixes typos in Mongo advisory locking parameters --- database/mongodb/README.md | 4 ++-- database/mongodb/mongodb.go | 13 +++++++++++-- 2 files changed, 13 insertions(+), 4 deletions(-) diff --git a/database/mongodb/README.md b/database/mongodb/README.md index 32b736fae..bbebe02cd 100644 --- a/database/mongodb/README.md +++ b/database/mongodb/README.md @@ -15,8 +15,8 @@ | `x-transaction-mode` | `TransactionMode` | If set to `true` wrap commands in [transaction](https://docs.mongodb.com/manual/core/transactions). Available only for replica set. Driver is using [strconv.ParseBool](https://golang.org/pkg/strconv/#ParseBool) for parsing| | `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. | +| `x-advisory-lock-timeout` | `15` | The max time in seconds that migrate will wait to acquire a lock before failing. | +| `x-advisory-lock-timeout-interval` | `10` | The max time in seconds between attempts to acquire the advisory lock, the lock is attempted to be acquired using an exponential backoff algorithm. | | `dbname` | `DatabaseName` | The name of the database to connect to | | `user` | | The user to sign in as. Can be omitted | | `password` | | The user's password. Can be omitted | diff --git a/database/mongodb/mongodb.go b/database/mongodb/mongodb.go index 3e18fd44b..9fe39cb8b 100644 --- a/database/mongodb/mongodb.go +++ b/database/mongodb/mongodb.go @@ -137,7 +137,16 @@ func (m *Mongo) Open(dsn string) (database.Driver, error) { if err != nil { return nil, err } - maxLockingIntervals, err := parseInt(unknown.Get("x-advisory-lock-timout-interval"), DefaultLockTimeoutInterval) + + lockTimeout := unknown.Get("x-advisory-lock-timeout-interval") + + if lockTimeout == "" { + // The initial release had a typo for this argument but for backwards compatibility sake, we will keep supporting it. + lockTimeout = unknown.Get("x-advisory-lock-timout-interval") + } + + maxLockCheckInterval, err := parseInt(lockTimeout, DefaultLockTimeoutInterval) + if err != nil { return nil, err } @@ -157,7 +166,7 @@ func (m *Mongo) Open(dsn string) (database.Driver, error) { CollectionName: lockCollection, Timeout: lockingTimout, Enabled: advisoryLockingFlag, - Interval: maxLockingIntervals, + Interval: maxLockCheckInterval, }, }) if err != nil { From b63f099c7f263d451a50dd5630e63ae1613ee1d5 Mon Sep 17 00:00:00 2001 From: Steve Ramage Date: Thu, 4 Nov 2021 14:46:17 -0700 Subject: [PATCH 2/7] PR Feedback for #647 - Error out when both params set --- database/mongodb/mongodb.go | 17 ++++++++++++----- 1 file changed, 12 insertions(+), 5 deletions(-) diff --git a/database/mongodb/mongodb.go b/database/mongodb/mongodb.go index 9fe39cb8b..9b7a47a2b 100644 --- a/database/mongodb/mongodb.go +++ b/database/mongodb/mongodb.go @@ -37,7 +37,8 @@ const contextWaitTimeout = 5 * time.Second // how long to wait for var ( ErrNoDatabaseName = fmt.Errorf("no database name") - ErrNilConfig = fmt.Errorf("no config") + ErrNilConfig = fmt.Errorf("no config") + ErrTypoAndNotNonTypoUsed = fmt.Errorf("both x-advisory-lock-timeout-interval and x-advisory-lock-timout-interval were specified") ) type Mongo struct { @@ -138,11 +139,17 @@ func (m *Mongo) Open(dsn string) (database.Driver, error) { return nil, err } - lockTimeout := unknown.Get("x-advisory-lock-timeout-interval") + lockTimeoutIntervalValue := unknown.Get("x-advisory-lock-timeout-interval") + // The initial release had a typo for this argument but for backwards compatibility sake, we will keep supporting it + // and we will error out if both values are set. + lockTimeoutIntervalValueFromTypo := unknown.Get("x-advisory-lock-timout-interval") - if lockTimeout == "" { - // The initial release had a typo for this argument but for backwards compatibility sake, we will keep supporting it. - lockTimeout = unknown.Get("x-advisory-lock-timout-interval") + lockTimeout := lockTimeoutIntervalValue + + if (lockTimeoutIntervalValue != "" && lockTimeoutIntervalValueFromTypo != "") { + return nil, ErrTypoAndNotNonTypoUsed + } else if (lockTimeoutIntervalValueFromTypo != "") { + lockTimeout = lockTimeoutIntervalValueFromTypo } maxLockCheckInterval, err := parseInt(lockTimeout, DefaultLockTimeoutInterval) From fa529960f45bba569afc288f3bf6ba9b0cb9e5ea Mon Sep 17 00:00:00 2001 From: Steve Ramage Date: Thu, 4 Nov 2021 20:43:04 -0700 Subject: [PATCH 3/7] Fixes lint issues with #647 --- CONTRIBUTING.md | 2 +- database/mongodb/mongodb.go | 6 +++--- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index 84fb8238a..e94c65ddd 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -11,7 +11,7 @@ 1. Write awesome code ... 1. `make test` to run all tests against all database versions 1. Push code and open Pull Request - + :wq Some more helpful commands: * You can specify which database/ source tests to run: diff --git a/database/mongodb/mongodb.go b/database/mongodb/mongodb.go index 9b7a47a2b..b6c3a5caa 100644 --- a/database/mongodb/mongodb.go +++ b/database/mongodb/mongodb.go @@ -36,7 +36,7 @@ const LockIndexName = "lock_unique_key" // the name of the inde const contextWaitTimeout = 5 * time.Second // how long to wait for the request to mongo to block/wait for. var ( - ErrNoDatabaseName = fmt.Errorf("no database name") + ErrNoDatabaseName = fmt.Errorf("no database name") ErrNilConfig = fmt.Errorf("no config") ErrTypoAndNotNonTypoUsed = fmt.Errorf("both x-advisory-lock-timeout-interval and x-advisory-lock-timout-interval were specified") ) @@ -146,9 +146,9 @@ func (m *Mongo) Open(dsn string) (database.Driver, error) { lockTimeout := lockTimeoutIntervalValue - if (lockTimeoutIntervalValue != "" && lockTimeoutIntervalValueFromTypo != "") { + if lockTimeoutIntervalValue != "" && lockTimeoutIntervalValueFromTypo != "" { return nil, ErrTypoAndNotNonTypoUsed - } else if (lockTimeoutIntervalValueFromTypo != "") { + } else if lockTimeoutIntervalValueFromTypo != "" { lockTimeout = lockTimeoutIntervalValueFromTypo } From 2fe1563b8c2cab07e80d0e1739f6d5db809a6ecd Mon Sep 17 00:00:00 2001 From: Steve Ramage Date: Thu, 4 Nov 2021 21:43:09 -0700 Subject: [PATCH 4/7] Additional PR Fixes #647 --- CONTRIBUTING.md | 1 - database/mongodb/mongodb.go | 6 +++--- 2 files changed, 3 insertions(+), 4 deletions(-) diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index e94c65ddd..42213413f 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -11,7 +11,6 @@ 1. Write awesome code ... 1. `make test` to run all tests against all database versions 1. Push code and open Pull Request - :wq Some more helpful commands: * You can specify which database/ source tests to run: diff --git a/database/mongodb/mongodb.go b/database/mongodb/mongodb.go index b6c3a5caa..599518cd5 100644 --- a/database/mongodb/mongodb.go +++ b/database/mongodb/mongodb.go @@ -37,8 +37,8 @@ const contextWaitTimeout = 5 * time.Second // how long to wait for var ( ErrNoDatabaseName = fmt.Errorf("no database name") - ErrNilConfig = fmt.Errorf("no config") - ErrTypoAndNotNonTypoUsed = fmt.Errorf("both x-advisory-lock-timeout-interval and x-advisory-lock-timout-interval were specified") + ErrNilConfig = fmt.Errorf("no config") + ErrLockTimeoutConfigConflict = fmt.Errorf("both x-advisory-lock-timeout-interval and x-advisory-lock-timout-interval were specified") ) type Mongo struct { @@ -147,7 +147,7 @@ func (m *Mongo) Open(dsn string) (database.Driver, error) { lockTimeout := lockTimeoutIntervalValue if lockTimeoutIntervalValue != "" && lockTimeoutIntervalValueFromTypo != "" { - return nil, ErrTypoAndNotNonTypoUsed + return nil, ErrLockTimeoutConfigConflict } else if lockTimeoutIntervalValueFromTypo != "" { lockTimeout = lockTimeoutIntervalValueFromTypo } From 4ba43183fd2a5b76de443531f0e6f4f28d69dbdf Mon Sep 17 00:00:00 2001 From: Steve Ramage Date: Thu, 4 Nov 2021 21:45:28 -0700 Subject: [PATCH 5/7] Lint fixes again #647 --- database/mongodb/mongodb.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/database/mongodb/mongodb.go b/database/mongodb/mongodb.go index 599518cd5..00f0fd1be 100644 --- a/database/mongodb/mongodb.go +++ b/database/mongodb/mongodb.go @@ -36,7 +36,7 @@ const LockIndexName = "lock_unique_key" // the name of the inde const contextWaitTimeout = 5 * time.Second // how long to wait for the request to mongo to block/wait for. var ( - ErrNoDatabaseName = fmt.Errorf("no database name") + ErrNoDatabaseName = fmt.Errorf("no database name") ErrNilConfig = fmt.Errorf("no config") ErrLockTimeoutConfigConflict = fmt.Errorf("both x-advisory-lock-timeout-interval and x-advisory-lock-timout-interval were specified") ) From 868d692e241f46c39e0255494e8fead64c6c715d Mon Sep 17 00:00:00 2001 From: Steve Ramage Date: Thu, 4 Nov 2021 21:48:18 -0700 Subject: [PATCH 6/7] One more tweak to CONTRIBUTING.md --- CONTRIBUTING.md | 1 + 1 file changed, 1 insertion(+) diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index 42213413f..0bfe3df77 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -11,6 +11,7 @@ 1. Write awesome code ... 1. `make test` to run all tests against all database versions 1. Push code and open Pull Request + Some more helpful commands: * You can specify which database/ source tests to run: From dd504065f8a4510f5e68021fff22e2a8baafab4c Mon Sep 17 00:00:00 2001 From: Steve Ramage Date: Thu, 4 Nov 2021 21:48:51 -0700 Subject: [PATCH 7/7] One more tweak to CONTRIBUTING.md, again --- CONTRIBUTING.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index 0bfe3df77..84fb8238a 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -11,7 +11,7 @@ 1. Write awesome code ... 1. `make test` to run all tests against all database versions 1. Push code and open Pull Request - + Some more helpful commands: * You can specify which database/ source tests to run: