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

Race fix: Acquire lock in fetchAndLoadDatabase before calling _fetchAndLoadDatabase #6638

Merged
merged 1 commit into from
Jan 11, 2024

Conversation

bbrks
Copy link
Member

@bbrks bbrks commented Jan 11, 2024

This race appears to have cropped up now we're running more tests with Rosmar.

fetchAndLoadDatabase wasn't acquiring the lock before falling into _fetchAndLoadDatabase which seems obviously wrong. I've scanned usages of both methods and the locking seems correct based on _ prefix usage (i.e. we're not going to be accidentally double locking)

22:05:17 ==================
22:05:17 
WARNING: DATA RACE
22:05:17 Read at 0x00c001e25e98 by goroutine 37168:
22:05:17   github.com/couchbase/sync_gateway/rest.(*ServerContext)._applyConfig()
22:05:17       /home/couchbase/jenkins/workspace/sgw-unix-build/3.2.0/community/sync_gateway/rest/config.go:1830 +0x375
22:05:17   github.com/couchbase/sync_gateway/rest.(*ServerContext)._applyConfigs()
22:05:17       /home/couchbase/jenkins/workspace/sgw-unix-build/3.2.0/community/sync_gateway/rest/config.go:1770 +0x17c
22:05:17   github.com/couchbase/sync_gateway/rest.(*ServerContext).fetchAndLoadConfigs()
22:05:17       /home/couchbase/jenkins/workspace/sgw-unix-build/3.2.0/community/sync_gateway/rest/config.go:1446 +0xa64
22:05:17   github.com/couchbase/sync_gateway/rest.(*ServerContext).initializeBootstrapConnection.func1()
22:05:17       /home/couchbase/jenkins/workspace/sgw-unix-build/3.2.0/community/sync_gateway/rest/server_context.go:2093 +0x325
22:05:17 
22:05:17 Previous write at 0x00c001e25e98 by goroutine 37166:
22:05:17   github.com/couchbase/sync_gateway/rest.(*ServerContext)._getOrAddDatabaseFromConfig()
22:05:17       /home/couchbase/jenkins/workspace/sgw-unix-build/3.2.0/community/sync_gateway/rest/server_context.go:911 +0x3f46
22:05:17   github.com/couchbase/sync_gateway/rest.(*ServerContext)._reloadDatabaseWithConfig()
22:05:17       /home/couchbase/jenkins/workspace/sgw-unix-build/3.2.0/community/sync_gateway/rest/server_context.go:438 +0x159
22:05:17   github.com/couchbase/sync_gateway/rest.(*ServerContext)._applyConfig()
22:05:17       /home/couchbase/jenkins/workspace/sgw-unix-build/3.2.0/community/sync_gateway/rest/config.go:1853 +0x6f9
22:05:17   github.com/couchbase/sync_gateway/rest.(*ServerContext)._applyConfigs()
22:05:17       /home/couchbase/jenkins/workspace/sgw-unix-build/3.2.0/community/sync_gateway/rest/config.go:1770 +0x17c
22:05:17   github.com/couchbase/sync_gateway/rest.(*ServerContext)._fetchAndLoadDatabase()
22:05:17       /home/couchbase/jenkins/workspace/sgw-unix-build/3.2.0/community/sync_gateway/rest/config.go:1472 +0x1cd
22:05:17   github.com/couchbase/sync_gateway/rest.(*ServerContext).fetchAndLoadDatabase()
22:05:17       /home/couchbase/jenkins/workspace/sgw-unix-build/3.2.0/community/sync_gateway/rest/config.go:1462 +0x255
22:05:17   github.com/couchbase/sync_gateway/rest.(*ServerContext).GetInactiveDatabase()
22:05:17       /home/couchbase/jenkins/workspace/sgw-unix-build/3.2.0/community/sync_gateway/rest/server_context.go:294 +0x256
22:05:17   github.com/couchbase/sync_gateway/rest.(*ServerContext).GetDatabase()
22:05:17       /home/couchbase/jenkins/workspace/sgw-unix-build/3.2.0/community/sync_gateway/rest/server_context.go:256 +0xd7
22:05:17   github.com/couchbase/sync_gateway/rest/adminapitest.TestDbConfigPersistentSGVersions.func1.1()
22:05:17       /home/couchbase/jenkins/workspace/sgw-unix-build/3.2.0/community/sync_gateway/rest/adminapitest/admin_api_test.go:3356 +0x84
22:05:17   github.com/couchbase/sync_gateway/rest.WaitAndAssertCondition()
22:05:17       /home/couchbase/jenkins/workspace/sgw-unix-build/3.2.0/community/sync_gateway/rest/utilities_testing.go:2283 +0x116
22:05:17   github.com/couchbase/sync_gateway/rest/adminapitest.TestDbConfigPersistentSGVersions.func1()
22:05:17       /home/couchbase/jenkins/workspace/sgw-unix-build/3.2.0/community/sync_gateway/rest/adminapitest/admin_api_test.go:3355 +0x14d
22:05:17   github.com/couchbase/sync_gateway/rest/adminapitest.TestDbConfigPersistentSGVersions()
22:05:17       /home/couchbase/jenkins/workspace/sgw-unix-build/3.2.0/community/sync_gateway/rest/adminapitest/admin_api_test.go:3369 +0x80b
22:05:17   testing.tRunner()
22:05:17       /home/couchbase/cbdeps/go1.21.4/src/testing/testing.go:1595 +0x238
22:05:17   testing.(*T).Run.func1()
22:05:17       /home/couchbase/cbdeps/go1.21.4/src/testing/testing.go:1648 +0x44
22:05:17 
22:05:17 Goroutine 37168 (running) created at:
22:05:17   github.com/couchbase/sync_gateway/rest.(*ServerContext).initializeBootstrapConnection()
22:05:17       /home/couchbase/jenkins/workspace/sgw-unix-build/3.2.0/community/sync_gateway/rest/server_context.go:2082 +0x784
22:05:17   github.com/couchbase/sync_gateway/rest.SetupServerContext()
22:05:17       /home/couchbase/jenkins/workspace/sgw-unix-build/3.2.0/community/sync_gateway/rest/config.go:1387 +0x50b
22:05:17   github.com/couchbase/sync_gateway/rest.StartServerWithConfig()
22:05:17       /home/couchbase/jenkins/workspace/sgw-unix-build/3.2.0/community/sync_gateway/rest/utilities_testing_bootstrap.go:144 +0x86
22:05:17   github.com/couchbase/sync_gateway/rest/adminapitest.TestDbConfigPersistentSGVersions()
22:05:17       /home/couchbase/jenkins/workspace/sgw-unix-build/3.2.0/community/sync_gateway/rest/adminapitest/admin_api_test.go:3323 +0x1f2
22:05:17   testing.tRunner()
22:05:17       /home/couchbase/cbdeps/go1.21.4/src/testing/testing.go:1595 +0x238
22:05:17   testing.(*T).Run.func1()
22:05:17       /home/couchbase/cbdeps/go1.21.4/src/testing/testing.go:1648 +0x44
22:05:17 
22:05:17 Goroutine 37166 (running) created at:
22:05:17   testing.(*T).Run()
22:05:17       /home/couchbase/cbdeps/go1.21.4/src/testing/testing.go:1648 +0x82a
22:05:17   testing.runTests.func1()
22:05:17       /home/couchbase/cbdeps/go1.21.4/src/testing/testing.go:2054 +0x84
22:05:17   testing.tRunner()
22:05:17       /home/couchbase/cbdeps/go1.21.4/src/testing/testing.go:1595 +0x238
22:05:17   testing.runTests()
22:05:17       /home/couchbase/cbdeps/go1.21.4/src/testing/testing.go:2052 +0x896
22:05:17   testing.(*M).Run()
22:05:17       /home/couchbase/cbdeps/go1.21.4/src/testing/testing.go:1925 +0xb57
22:05:17   github.com/couchbase/sync_gateway/base.TestBucketPoolMain()
22:05:17       /home/couchbase/jenkins/workspace/sgw-unix-build/3.2.0/community/sync_gateway/base/main_test_bucket_pool.go:689 +0x424
22:05:17   github.com/couchbase/sync_gateway/db.TestBucketPoolWithIndexes()
22:05:17       /home/couchbase/jenkins/workspace/sgw-unix-build/3.2.0/community/sync_gateway/db/util_testing.go:510 +0x71
22:05:17   github.com/couchbase/sync_gateway/rest/adminapitest.TestMain()
22:05:17       /home/couchbase/jenkins/workspace/sgw-unix-build/3.2.0/community/sync_gateway/rest/adminapitest/main_test.go:24 +0x1d
22:05:17   main.main()
22:05:17       _testmain.go:235 +0x307
22:05:17 ==================

Integration Tests

@torcolvin torcolvin merged commit d3cda06 into master Jan 11, 2024
17 checks passed
@torcolvin torcolvin deleted the fix_fetchAndLoadDatabase_race branch January 11, 2024 15:57
@bbrks
Copy link
Member Author

bbrks commented Jan 31, 2024

Retrospective Jira ticket: CBG-3743

gregns1 pushed a commit that referenced this pull request Feb 6, 2024
torcolvin pushed a commit that referenced this pull request Feb 6, 2024
…etchAndLoadDatabase (#6638) (#6672)

Co-authored-by: Ben Brooks <ben.brooks@couchbase.com>
bbrks added a commit that referenced this pull request Mar 28, 2024
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.

2 participants