Create/alter policy and exclusive table lock

Previous Topic Next Topic
 
classic Classic list List threaded Threaded
4 messages Options
Reply | Threaded
Open this post in threaded view
|

Create/alter policy and exclusive table lock

konstantin knizhnik
Hi hackers,

Right now changing policies (create/alter policy statements) requires
exclusive lock of target table:

     /* Get id of table.  Also handles permissions checks. */
     table_id = RangeVarGetRelidExtended(stmt->table, AccessExclusiveLock,
                                         0,
                                         RangeVarCallbackForPolicy,
                                         (void *) stmt);

Unfortunately there are use cases where policies are changed quite
frequently and this exclusive lock becomes a bottleneck.
I wonder why do we really need exclusive lock here?
Policies are stored in pg_policy table and we get  RowExclusiveLock on it.

May be I missed something, but why we can not rely on standard MVCC
visibility rules for pg_policy table?
Until transaction executing CREATE/ALTER POLICY is committed, other
transactions will not see its changes in pg_policy table and perform
RLS checks according to old policies. Once transaction is committed,
everybody will switch to new policies.

I wonder if we it is possible to replace AccessExclusiveLock with
AccessSharedLock in RangeVarGetRelidExtended in CreatePolicy and
AlterPolicy?

--
Konstantin Knizhnik
Postgres Professional: http://www.postgrespro.com
The Russian Postgres Company



Reply | Threaded
Open this post in threaded view
|

Re: Create/alter policy and exclusive table lock

Tom Lane-2
Konstantin Knizhnik <[hidden email]> writes:
> Right now changing policies (create/alter policy statements) requires
> exclusive lock of target table:

Yup.

> I wonder why do we really need exclusive lock here?

Because it affects the behavior of a SELECT.

> May be I missed something, but why we can not rely on standard MVCC
> visibility rules for pg_policy table?

We cannot have a situation where the schema details of a table might
change midway through planning/execution of a statement.  The results
are unlikely to be as clean as "you get either the old behavior or the
new one", because that sequence might examine the details more than
once.  Also, even if you cleanly get the old behavior, that's hardly
satisfactory.  Consider

Session 1                        Session 2

begin;
alter policy ... on t1 ...;
insert new data into t1;

                                 begin planning SELECT on t1;

commit;

                                 begin executing SELECT on t1;

With your proposal, session 2 would see the new data in t1
(because the executor takes a fresh snapshot) but it would not
be affected by the new policy.  That's a security failure,
and it's one that does not happen today.

                        regards, tom lane


Reply | Threaded
Open this post in threaded view
|

Re: Create/alter policy and exclusive table lock

konstantin knizhnik


On 14.01.2020 17:40, Tom Lane wrote:

> Konstantin Knizhnik <[hidden email]> writes:
>> Right now changing policies (create/alter policy statements) requires
>> exclusive lock of target table:
> Yup.
>
>> I wonder why do we really need exclusive lock here?
> Because it affects the behavior of a SELECT.
>
>> May be I missed something, but why we can not rely on standard MVCC
>> visibility rules for pg_policy table?
> We cannot have a situation where the schema details of a table might
> change midway through planning/execution of a statement.  The results
> are unlikely to be as clean as "you get either the old behavior or the
> new one", because that sequence might examine the details more than
> once.  Also, even if you cleanly get the old behavior, that's hardly
> satisfactory.  Consider
>
> Session 1                        Session 2
>
> begin;
> alter policy ... on t1 ...;
> insert new data into t1;
>
>                                   begin planning SELECT on t1;
>
> commit;
>
>                                   begin executing SELECT on t1;
>
> With your proposal, session 2 would see the new data in t1
> (because the executor takes a fresh snapshot) but it would not
> be affected by the new policy.  That's a security failure,
> and it's one that does not happen today.

Thank you for explanation.
But let me ask you one more question: why do we obtaining snapshot twice
in exec_simple_query:
first for analyze (pg_analyze_and_rewrite) and one for execution
(PortalStart)?
GetSnapshotData is quite expensive operation and the fact that we are
calling it twice for each query execution (with read committed isolation
level)
seems to be strange. Also the problem in scenario you have described
above is caused by using different snapshots by planner and executor. If
them will share the same snapshot,
then there should be no security violation, right?

--
Konstantin Knizhnik
Postgres Professional: http://www.postgrespro.com
The Russian Postgres Company



Reply | Threaded
Open this post in threaded view
|

Re: Create/alter policy and exclusive table lock

Tom Lane-2
Konstantin Knizhnik <[hidden email]> writes:
> But let me ask you one more question: why do we obtaining snapshot twice
> in exec_simple_query:
> first for analyze (pg_analyze_and_rewrite) and one for execution
> (PortalStart)?

That would happen anyway if the plan is cached.  If we were to throw away
all plan caching and swear a mighty oath that we'll never put it back,
maybe we could build in a design assumption that planning and execution
use identical snapshots.  I doubt that would lead to a net win though.

Also note that our whole approach to cache invalidation is based on the
assumption that if session A needs to see the effects of session B,
they will be taking conflicting locks.  Otherwise sinval signaling
is not guaranteed to be detected at the necessary times.

                        regards, tom lane