Fix getting stuck on refreshing postgresql grant #351
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
pagalba-com
approved these changes
Sep 18, 2023
pagalba-com
approved these changes
Sep 19, 2023
|
@cyrilgdn please approve this MR! Its impossible to work on aws prod without this. |
giner
force-pushed
the
fix_getting_stuck_on_refreshing_postgresql_grant
branch
from
72d17d4 to
45faa28
Compare
|
@kylejohnson would you take a look at this fix please? |
giner
force-pushed
the
fix_getting_stuck_on_refreshing_postgresql_grant
branch
from
45faa28 to
a9554d8
Compare
cyrilgdn
reviewed
Feb 24, 2024
| @@ -730,6 +730,8 @@ func checkRoleDBSchemaExists(client *Client, d *schema.ResourceData) (bool, erro | |||
| return false, nil | |||
| } | |||
|
|
|||
| deferredRollback(txn) | |||
There was a problem hiding this comment.
I think we should better avoid creating a new transaction and use the existing one if it's the same database.
So line 738:
var dbTxn *sql.Tx
if database == client.databaseName {
dbTxn = txn
} else {
var err error;
dbTxn, err := startTransaction(client, database)
if err != nil {
return false, err
}
defer deferredRollback(dbTxn)
}(and actually if it's for another database a transaction is not really needed as it executes only 1 request but I don't think we already have an helper to get a simple DB connection for alternative DB)
What do you think?
There was a problem hiding this comment.
Makes sense to me. I'll unify this function to be more in line with the rest of the code.
This change fixes terraform refresh getting stuck forever on postgresql_grant schema when provider connection database is the same as the one we are trying to read schema information from. Until now the issue could be reproduced only on RDS (PostgreSQL 14.7). Fixed by explicitly finishing one transaction before starting a new one. Fixes cyrilgdn#335
- Pass DBConnection instead of Client (the same as in other functions) - Check database existence with a simple query outside of a transaction - Use a single transaction for queries within the function
giner
force-pushed
the
fix_getting_stuck_on_refreshing_postgresql_grant
branch
from
a159d0b to
19a04cd
Compare
|
@cyrilgdn, Does anything need to be done for v1.22.0 to hit the registry? It's not there yet. |
lemeurherve
referenced
this pull request
in jenkins-infra/azure
Mar 11, 2024
<Actions>
<action
id="703b1c5c9886f66325fdfa2d8c42994644728056afe84d8427884bee985d0729">
<h3>Bump Terraform `postgresql` provider version</h3>
<details
id="1d9343c012f5434ac9fe8a98135bae3667b399259be16d9b14302ea3bd424a24">
<summary>Update Terraform lock file</summary>
<p>changes detected:
	"cyrilgdn/postgresql" updated from
"1.21.0" to "1.22.0" in file
".terraform.lock.hcl"</p>
<details>
<summary>1.22.0</summary>
<pre>Changelog retrieved
from:
	https://github.com/cyrilgdn/terraform-provider-postgresql/releases/tag/v1.22.0
##
What's Changed

#### Bug fixes
* Fix
getting stuck on refreshing postgresql grant by @giner in
https://github.com/cyrilgdn/terraform-provider-postgresql/pull/351
*
fix: quote table name in postgresql_publication by @jerome-quere in
https://github.com/cyrilgdn/terraform-provider-postgresql/pull/402

####
Doc
* Add `sslinline` argument description to
index.html.markdown by @Hakkenouw in
https://github.com/cyrilgdn/terraform-provider-postgresql/pull/409
*
Fix indentation in index.html.markdown by @hfm in
https://github.com/cyrilgdn/terraform-provider-postgresql/pull/380
*
fix(doc): Typo in `postgresql_grant_role` doc by @cyrilgdn in
https://github.com/cyrilgdn/terraform-provider-postgresql/pull/412

####
Libs
* Bump golang.org/x/crypto from 0.11.0 to 0.17.0 by
@dependabot in
https://github.com/cyrilgdn/terraform-provider-postgresql/pull/386

##
New Contributors
* @Hakkenouw made their first contribution in
https://github.com/cyrilgdn/terraform-provider-postgresql/pull/409
*
@jerome-quere made their first contribution in
https://github.com/cyrilgdn/terraform-provider-postgresql/pull/402
*
@hfm made their first contribution in
https://github.com/cyrilgdn/terraform-provider-postgresql/pull/380
*
@dependabot made their first contribution in
https://github.com/cyrilgdn/terraform-provider-postgresql/pull/386
*
@giner made their first contribution in
https://github.com/cyrilgdn/terraform-provider-postgresql/pull/351

**Full
Changelog**:
https://github.com/cyrilgdn/terraform-provider-postgresql/compare/v1.21.0...v1.22.0</pre>
</details>
</details>
<a
href="https://infra.ci.jenkins.io/job/updatecli/job/azure/job/main/33/">Jenkins
pipeline link</a>
</action>
</Actions>
---
<table>
<tr>
<td width="77">
<img src="https://www.updatecli.io/images/updatecli.png" alt="Updatecli
logo" width="50" height="50">
</td>
<td>
<p>
Created automatically by <a
href="https://www.updatecli.io/">Updatecli</a>
</p>
<details><summary>Options:</summary>
<br />
<p>Most of Updatecli configuration is done via <a
href="https://www.updatecli.io/docs/prologue/quick-start/">its
manifest(s)</a>.</p>
<ul>
<li>If you close this pull request, Updatecli will automatically reopen
it, the next time it runs.</li>
<li>If you close this pull request and delete the base branch, Updatecli
will automatically recreate it, erasing all previous commits made.</li>
</ul>
<p>
Feel free to report any issues at <a
href="https://github.com/updatecli/updatecli/issues">github.com/updatecli/updatecli</a>.<br
/>
If you find this tool useful, do not hesitate to star <a
href="https://github.com/updatecli/updatecli/stargazers">our GitHub
repository</a> as a sign of appreciation, and/or to tell us directly on
our <a
href="https://matrix.to/#/#Updatecli_community:gitter.im">chat</a>!
</p>
</details>
</td>
</tr>
</table>
Co-authored-by: Jenkins Infra Bot (updatecli) <60776566+jenkins-infra-bot@users.noreply.github.com>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This change fixes terraform refresh getting stuck forever on postgresql_grant schema when provider connection database is the same as the one we are trying to read schema information from. Until now the issue could be reproduced only on RDS (PostgreSQL 14.7). Fixed by explicitly finishing one transaction before starting a new one.
Fixes #335