Fix concurrency issue when granting privileges on databases

[timothegenzmer]

This fixes #223

[philip-harvey]

Any news on getting this merged?

[brokenjacobs]

@cyrilgdn any chance of this getting merged?

[BartoszGiza]

Hey, any news ? It would be nice to merge it.

[BartoszGiza]

BTW: i think i have workaround for this problem. I have added max_connections = 1 to provider block. Seems to work. Probably it will slow down plans :(

[seanamos]

BTW: i think i have workaround for this problem. I have added max_connections = 1 to provider block. Seems to work. Probably it will slow down plans :(

@BartoszGiza That does work in some cases. Unfortunately that also has another side effect that it can cause the provider to hang indefinitely.

[BartoszGiza]

BTW: i think i have workaround for this problem. I have added max_connections = 1 to provider block. Seems to work. Probably it will slow down plans :(

@BartoszGiza That does work in some cases. Unfortunately that also has another side effect that it can cause the provider to hang indefinitely.

Hi, I think i just hit that problem. I have problems with hanging during refreshing states. Also for some reason i can't delete database which provider created.

[vr]

Hey @cyrilgdn any updates on this ?

[cyrilgdn]

Hi,

Thanks for your work on that, and sorry for the delay.
I had no time to work on opensource projects these past months...

I haven't test it but code looks good so we can ship it 👍

[BartoszGiza]

@cyrilgdn thank You

[philip-harvey]

Will there be a release with this PR included? Thanks.

[timothegenzmer]

Thanks for merging.
We have tested this change in production for a while and it resolved the issue for us without any side effects

[BartoszGiza]

@cyrilgdn when do You plan to release new version ?

[cyrilgdn]

This has been released in v1.18.0

[philip-harvey]

I tested 1.18 and am still getting concurrency issues on grants so unfortunately #223 is not fixed

[timothegenzmer]

@philip-harvey Are you sure that you are facing the same issue?
This PR only fixes an concurrency issue if you simultaneously grant two different roles privileges on a database.
See the example code in the linked issue

[philip-harvey]

I am getting errors such as the below when updating grants if I don't have max_connections = 1

│ Error: could not get advisory lock for members of role pdr_ro_rl: pq: deadlock detected
│
│ with module.int_pdr.postgresql_grant.objects["b8e4ec3df0d0c7e7d493c4e904bc820e"],
│ on ../../modules/pdr-config/main.tf line 235, in resource "postgresql_grant" "objects":
│ 235: resource "postgresql_grant" "objects" {
│
╵
╷
│ Error: could not execute revoke query: pq: tuple concurrently updated
│
│ with module.int_pdr.postgresql_grant.objects["027c01cac83a9d668d9fe39e210a6278"],
│ on ../../modules/pdr-config/main.tf line 235, in resource "postgresql_grant" "objects":
│ 235: resource "postgresql_grant" "objects" {
│
╵
╷
│ Error: could not execute revoke query: pq: tuple concurrently updated
│
│ with module.int_pdr.postgresql_grant.objects["117188f8b95fd3ede43ec4bc981c8abb"],
│ on ../../modules/pdr-config/main.tf line 235, in resource "postgresql_grant" "objects":
│ 235: resource "postgresql_grant" "objects" {
│
╵
╷
│ Error: could not execute revoke query: pq: tuple concurrently updated
│
│ with module.int_pdr.postgresql_grant.objects["de1c9bcc954536336bb466b2a011beef"],
│ on ../../modules/pdr-config/main.tf line 235, in resource "postgresql_grant" "objects":
│ 235: resource "postgresql_grant" "objects" {

[timothegenzmer]

Can you provide the relevant terraform code?
Just from the error it is hard to say that you are affected by the same issue, as all are just postgresql_grant.

Also the first error: could not get advisory lock for members of role pdr_ro_rl: pq: deadlock detected looks worrisome.
Is this a new error since you updated to 1.18?
Could you verify that the same error happened on 1.17.1?

[philip-harvey]

This is the specific part of the code that is still failing due to concurrency issues.

# Grant each role rights to the objects in the schema
resource "postgresql_grant" "objects" {
  for_each    = local.grants_map
  role        = each.value.rolename
  database    = local.database_name
  schema      = each.value.schema
  object_type = each.value.object_type
  objects     = each.value.objects
  privileges  = each.value.privileges
  depends_on = [
    postgresql_role.roles,
    postgresql_schema.schema
  ]
}

AFAIK this issue has existed on all previous and current versions of the provider and the only known way to work around the issue currently is to set max_connections = 1

In this particular use case objects_type = table for most of the grants. Is the merged fix only valid if object_type = database? The exact same issue occurs for tables and schemas as well as databases, and any combination of object_types. e.g. #178

[timothegenzmer]

This is the most generic code that you could have provided :D

Yes this PR only fixes object_type = database

If you want to know which query is actually responsible for your issues you can have a look at the postgres logs.

[seanamos]

I've had to downgrade as this change is causing all sorts of havoc. The biggest one is that its "leaking" advisory locks on databases. I'm often left with multiple locks that I have to manually cleanup.

[timothegenzmer]

Hey @seanamos,
Sorry for the late response.

I have tested my code again, but I was not able to leak any locks
I have tested this via while :; do terraform apply --auto-approve && terraform destroy --auto-approve; done
and I have checked this with select relation::regclass, * from pg_locks;

from what I get pg_advisory_xact_lock should not leak any locks when the transaction is not leaked and the transaction should not be leaked as it is released via defer

Do you have an example code to reproduce the issue or could you share the leaked locks here so that I can investigate further?

[philip-harvey]

Any chance the change in #224 can be rolled back? In the past we could set max_connections = 1 and it would work, now it's just totally broken for versions > 1.1.7

[timothegenzmer]

Are you able to provide a terraform configuration that has this issue?
I have tested this again and again with the config I shared in #223 that previously would throw and error and this PR fixed this configuration.

The fix that I provided might have had some side effects that I am not aware of.
I am happy to provide a fix for them if I am able to reproduce the issue

[pagalba-com]

I have tested 1.21.1-beta.1 and concurrency issues are not fixed.

The fix provided by giner works. This one not.
Ref: https://github.com/giner/terraform-provider-postgresql/commits/fix_getting_stuck_on_refreshing_postgresql_grant