Forum OpenACS Development: PL/SQL: acs_permission__grant_permission lock problem

Request notifications

Hello everyone,

on a system of ours we require to dump database twice a day to ensure a higher degree of recover. This way is a tradeoff between requirements and ease of implementation, as more advanced techniques could be used... anyway...

One of such dumps take place around lunchtime. Business is reduced at that time, but still some users are accessing the system.

During this dump, we experience lock in some situations. After some investigating we found the problem is the execution of acs_permission__grant_permission stored procedure into db, which can happen in quite a few places in our code.

We have found out this proc actually locks permissions table in an exclusive way. This is true on our system and also on current codebase:

http://www.openacs.org/api-doc/plsql-subprogram-one?type=FUNCTION&name=acs_permission__grant_permission

If I understand correctly, acs_permission__grant_permission needs to be the only one proc accessing permissions table, so it must wait until pg_dump finishes its work to proceed and this causes the problem.

My question now is: do we really require acs_permission__grant_permission to lock permission table exclusively? Isn't enough safe to just execute this procedure in a transaction, which by default won't lock reading?

If relaxing proc behavior is not an option, could someone suggest a solution for my scenario?

Antonio

the lock operation causes grant_permission to wait until other grant_permissions are finished. just running in a transaction might have the consequence, that the transaction might raise an error in case some other concurrent operations has already inserted the same values (which is not highly likely, but possible).

the right solution is probably to perform an insert and handle the exception from unique violations (i.e. ignore the case), then the lock won't be necessary IMHO.

Note that also pg does lock tables during backup operations.

Thanks for your response Gustaf.

Some stub of your approach seems already present in the code. I will look into it and try to propose a proper solution.

This is my proposal
CREATE OR REPLACE FUNCTION acs_permission__grant_permission(
   grant_permission__object_id integer,
   grant_permission__grantee_id integer,
   grant_permission__privilege varchar
) RETURNS integer AS $$
DECLARE
    exists_p                            boolean;
BEGIN
    -- lock table acs_permissions_lock;
    select count(*) > 0 into exists_p
      from acs_permissions
     where object_id = grant_permission__object_id
       and grantee_id = grant_permission__grantee_id
       and privilege = grant_permission__privilege;

    if not exists_p then
        insert into acs_permissions
          (object_id, grantee_id, privilege)
          values
          (grant_permission__object_id, grant_permission__grantee_id, 
          grant_permission__privilege);

    end if;
    
    return 0;
-- handle exception, rather than lock table
EXCEPTION 
    when unique_violation then
      return 0;
END;
$$ LANGUAGE plpgsql;
As you can see I've just put exception handling translating from 'dup_val_on_index', Oracle flavour, to 'unique_violation', which is Postgres's. This came from:

http://www.postgresql.org/docs/9.0/static/plpgsql-porting.html

This translation seems valid from our actual Postgres 8.2 to devel version, looking at doc.

This should fix permission granting. While testing, I've found out the same problem happens when we call acs_permission__revoke_permission, as also this proc locks permissions exclusively.

In this case solution is simpler as it is sufficient to comment lock and go on with our lives.

CREATE OR REPLACE FUNCTION acs_permission__revoke_permission(
   revoke_permission__object_id integer,
   revoke_permission__grantee_id integer,
   revoke_permission__privilege varchar
) RETURNS integer AS $$
DECLARE
BEGIN
--     lock table acs_permissions_lock;
    delete from acs_permissions
    where object_id = revoke_permission__object_id
    and grantee_id = revoke_permission__grantee_id
    and privilege = revoke_permission__privilege;

    return 0; 
END;
$$ LANGUAGE plpgsql;
It is important to notice that Oracle codebase already behaves in this way, that is, in Oracle we don't lock any table and just handle exception/ignore the case

This solves our issue. If you think this solution is ok in general I can commit to cvs.

All the best
Antonio

This looks fine, although i would recommend to remove the "select count(*)..." from acs_permission__grant_permission, since these two operations smell after a race condition. In most cases, the count() should return 0, and since the EXCEPTION deals with the the duplicate case anyhow, i see no reason for the count().

-g

I have removed count from grant proc and committed changes. I've seen there is some work going on in acs-kernel, so my upgrade became from 5.9.0d5 to 5.9.0d6

Please let me know about any possible issue.

All the best