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

Fixed race condition with IAM Postgres Driver #2776

Merged
merged 1 commit into from Oct 4, 2019

Conversation

@mr337
Copy link
Contributor

commented Oct 4, 2019

Description

This PR addresses the issue of having a shared state. This was detected by running go test with the -race option. More info here

The use of a mutex fixed the opportunity to read and write at the same time. Since the context of the mutex is kept extremely small there is no expected impact or deadlocks.

Reviewer Notes

None

Setup

Add any steps or code to run in this section to help others prepare to run your code:

cd pkg/iampostgres/
go test -race -v .

Code Review Verification Steps

  • Tested in the Experimental environment (for changes to containers, app startup, or connection to data stores)
  • Request review from a member of a different team.
  • Have the Pivotal acceptance criteria been met for this change?

References

@mr337 mr337 requested review from chrisgilmerproj and gidjin Oct 4, 2019
Copy link
Contributor

left a comment

🚀 - Nice work!

@mr337 mr337 force-pushed the lh-rdsiam-race branch from bba95e0 to b528a25 Oct 4, 2019
@codecov

This comment has been minimized.

Copy link

commented Oct 4, 2019

Codecov Report

Merging #2776 into master will increase coverage by 0.1%.
The diff coverage is 100%.

@@           Coverage Diff            @@
##           master   #2776     +/-   ##
========================================
+ Coverage    57.6%   57.6%   +0.1%     
========================================
  Files         278     278             
  Lines       12567   12573      +6     
========================================
+ Hits         7233    7239      +6     
  Misses       4586    4586             
  Partials      748     748
Impacted Files Coverage Δ
pkg/iampostgres/iampostgres.go 74.5% <100%> (+4.2%) ⬆️
@mr337

This comment has been minimized.

Copy link
Contributor Author

commented Oct 4, 2019

Working great in experimental.

@mr337 mr337 force-pushed the lh-rdsiam-race branch from fb1e9e0 to b528a25 Oct 4, 2019
@mr337 mr337 force-pushed the lh-rdsiam-race branch from b528a25 to 09f14e9 Oct 4, 2019
@mr337 mr337 merged commit f4c833e into master Oct 4, 2019
18 checks passed
18 checks passed
ci/circleci: acceptance_tests_experimental Your tests passed on CircleCI!
Details
ci/circleci: acceptance_tests_local Your tests passed on CircleCI!
Details
ci/circleci: acceptance_tests_staging Your tests passed on CircleCI!
Details
ci/circleci: build_app Your tests passed on CircleCI!
Details
ci/circleci: build_migrations Your tests passed on CircleCI!
Details
ci/circleci: build_storybook_app Your tests passed on CircleCI!
Details
ci/circleci: build_tasks Your tests passed on CircleCI!
Details
ci/circleci: build_tools Your tests passed on CircleCI!
Details
ci/circleci: check_generated_code Your tests passed on CircleCI!
Details
ci/circleci: client_test Your tests passed on CircleCI!
Details
ci/circleci: integration_tests Your tests passed on CircleCI!
Details
ci/circleci: pre_deps_golang Your tests passed on CircleCI!
Details
ci/circleci: pre_deps_yarn Your tests passed on CircleCI!
Details
ci/circleci: pre_test Your tests passed on CircleCI!
Details
ci/circleci: server_test Your tests passed on CircleCI!
Details
ci/circleci: server_test_coverage Your tests passed on CircleCI!
Details
codecov/patch/go 100% of diff hit (target 57.4%)
Details
codecov/project/go 57.4% (+0.1%) compared to 20214bc
Details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
2 participants
You can’t perform that action at this time.