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.

Sign up for GitHub

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

Jump to bottom

Retry connection acquisition in PostgresStorageBackend.createDataSource #10758

Closed
wants to merge 1 commit into from
Closed

Retry connection acquisition in PostgresStorageBackend.createDataSource #10758

wants to merge 1 commit into from

Conversation

tudor-da
Copy link
Contributor

@tudor-da tudor-da commented Sep 2, 2021

CHANGELOG_BEGIN
CHANGELOG_END

Pull Request Checklist

  • Read and understand the contribution guidelines
  • Include appropriate tests
  • Set a descriptive title and thorough description
  • Add a reference to the issue this PR will solve, if appropriate
  • Include changelog additions in one or more commit message bodies between the CHANGELOG_BEGIN and CHANGELOG_END tags
  • Normal production system change, include purpose of change in description

NOTE: CI is not automatically run on non-members pull-requests for security
reasons. The reviewer will have to comment with /AzurePipelines run to
trigger the build.

@tudor-da tudor-da requested a review from a team as a code owner September 2, 2021 17:47
@tudor-da tudor-da force-pushed the tudor/pg-storage-backend-retry-connection-acquisition branch from b71b4a6 to 0326f8a Compare September 2, 2021 17:52
kamil-da
kamil-da reviewed Sep 2, 2021
View reviewed changes
...ntegration-api/src/main/scala/platform/store/backend/postgresql/PostgresStorageBackend.scala
@@ -305,4 +306,27 @@ private[backend] object PostgresStorageBackend
override def lock(id: Int): DBLockStorageBackend.LockId = PGLockId(id.toLong)

override def dbLockSupported: Boolean = true

// TODO Cleanup: Refactor into or re-use an existing utility
private def tryAcquireConnection(dataSource: DataSource, numberOfAttempts: Int = 600)(implicit
Copy link
Contributor

Choose a reason for hiding this comment

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

Why the number of attempts is so high? Is it because of waiting for db availability?

@kamil-da kamil-da mentioned this pull request Sep 2, 2021
Merged
6 tasks
@nmarton-da
Copy link
Contributor

I would prefer to not do retries there, because:

  • createDatasource functionality is lightweight, and not used in places where execution expected to take longer (direct call in factories instead of tuneling execution through Resource/ResourceOwner)
  • this is not solving the issue for other db types

Please see #10759 for a quick fix, but if severity allows I would fix it later.
cc: @mziolekda @rautenrieth-da

@tudor-da tudor-da closed this Sep 2, 2021
@tudor-da tudor-da deleted the tudor/pg-storage-backend-retry-connection-acquisition branch September 2, 2021 18:34
@tudor-da
Copy link
Contributor Author

tudor-da commented Sep 2, 2021

I would prefer to not do retries there, because:

* createDatasource functionality is lightweight, and not used in places where execution expected to take longer (direct call in factories instead of tuneling execution through Resource/ResourceOwner)

* this is not solving the issue for other db types

Please see #10759 for a quick fix, but if severity allows I would fix it later.
cc: @mziolekda @rautenrieth-da

Ok, you already opened one. Closing this one

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@kamil-da kamil-da kamil-da left review comments

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@tudor-da @nmarton-da @kamil-da