Hi,
On Tue, Jun 03, 2025 at 01:27:29PM -0400, Robert Haas wrote:
> On Mon, Jun 2, 2025 at 10:02 AM Bertrand Drouvot
> wrote:
> > So, we currently have 2 patterns:
> >
> > P1: permission checks on a referenced object is done before we call
> > recordMultipleDependencies()
> > P2: permission check
On Mon, Jun 2, 2025 at 10:02 AM Bertrand Drouvot
wrote:
> So, we currently have 2 patterns:
>
> P1: permission checks on a referenced object is done before we call
> recordMultipleDependencies()
> P2: permission checks on a referenced object is done after we call
> recordMultipleDependencies()
>
Hi,
On Thu, May 22, 2025 at 12:38:50PM -0700, Jeff Davis wrote:
> Which
> call sites are the most interesting ones that need special attention?
I think the ones that need special attention are the ones that check for
the permissions on the referenced objects *after* recordMultipleDependencies is
Hi,
On Thu, May 22, 2025 at 09:40:47AM -0400, Robert Haas wrote:
> On Thu, May 22, 2025 at 8:15 AM Bertrand Drouvot
> wrote:
> > That would probably address Robert's concern [1] "acquiring two locks on
> > the same
> > object with different lock modes where we should really only acquire one"
>
On Tue, Feb 4, 2025 at 9:24 PM Bertrand Drouvot
wrote:
>
> Hi,
>
> On Thu, Jan 02, 2025 at 08:15:13AM +, Bertrand Drouvot wrote:
> > rebased (v18 attached).
>
> Thanks to all of you that have discussed this patch during the developer
> meeting
> at FOSDEM PGDay last week [1]. I'm attaching a
On Thu, 2025-05-22 at 09:40 -0400, Robert Haas wrote:
> Pushing the locking down into recordDependencyOn amounts to hoping
> that we don't need to study each code path in detail and decide on
> the
> exactly right place to acquire the lock.
There are (by my rough count) over 250 call sites modifi
On Thu, May 22, 2025 at 8:15 AM Bertrand Drouvot
wrote:
> That would probably address Robert's concern [1] "acquiring two locks on the
> same
> object with different lock modes where we should really only acquire one" but
> probably not this one "I think it might result in acquiring the
> locks o
Hi,
On Wed, May 21, 2025 at 10:17:58AM -0700, Jeff Davis wrote:
> On Wed, 2025-05-21 at 12:55 -0400, Robert Haas wrote:
> > Yeah, that's not a terrible idea. I still like the idea I thought
> > Bertrand was pursuing, namely, to take no lock in
> > recordDependencyOn()
> > but assert that the calle
On Wed, May 21, 2025 at 1:18 PM Jeff Davis wrote:
> Of course relation_open() takes a lock, but sometimes relation_open()
> is hidden in the call stack below other functions where it's not so
> obvious.
Probably true, although some of those are probably code that could
stand to be improved.
> >
On Wed, 2025-05-21 at 12:55 -0400, Robert Haas wrote:
> > ...like generate_partition_qual() taking a lock on the parent or
> > CheckAttributeType() taking a lock on the typrelid...
>
> To me, relation_open() feels like the kind of operation that I would
> expect to take a lock. If I open something
On Tue, May 20, 2025 at 5:12 PM Jeff Davis wrote:
> But relation_open() is sort of an exception. There are lots of places
> where that takes a lock because we happen to want something out of the
> relcache, like generate_partition_qual() taking a lock on the parent or
> CheckAttributeType() taking
Hi,
On Tue, May 20, 2025 at 02:12:41PM -0700, Jeff Davis wrote:
> On Mon, 2025-05-19 at 14:07 -0400, Robert Haas wrote:
> > I agree with that, but I think that it may also be error-prone to
> > assume that it's OK to acquire heavyweight locks on other catalog
> > objects at any place in the code w
On Mon, 2025-05-19 at 14:07 -0400, Robert Haas wrote:
> I agree with that, but I think that it may also be error-prone to
> assume that it's OK to acquire heavyweight locks on other catalog
> objects at any place in the code where we record a dependency. I will
> not be surprised at all if that tur
On Mon, May 19, 2025 at 1:02 PM Jeff Davis wrote:
> I'm not sure if you intended it this way, but using "clever" here
> suggests that it's an unusual trick. But isn't that the kind of thing
> heavyweight locks are for?
>
> FWIW, a lock while recording a dependency would not be surprising to
> me.
On Wed, 2024-05-22 at 10:48 -0400, Robert Haas wrote:
> > Then, Tom proposed another approach in [2] which is that "creation
> > DDL will have
> > to take a lock on each referenced object that'd conflict with a
> > lock taken by DROP".
> > This is the one the current patch is trying to implement.
>
Hi,
On Thu, Jan 02, 2025 at 08:15:13AM +, Bertrand Drouvot wrote:
> rebased (v18 attached).
Thanks to all of you that have discussed this patch during the developer meeting
at FOSDEM PGDay last week [1]. I'm attaching a new version to address Álvaro's
concern about calling getObjectDescriptio
Hi,
On Mon, Oct 28, 2024 at 09:30:19AM +, Bertrand Drouvot wrote:
> Hi,
>
> On Mon, Aug 19, 2024 at 03:35:14PM +, Bertrand Drouvot wrote:
> > Hi,
> >
> > On Wed, Jul 10, 2024 at 07:31:06AM +, Bertrand Drouvot wrote:
> > > So, to sum up:
> > >
> > > A. Locking is now done exclusively
Hi,
On Mon, Aug 19, 2024 at 03:35:14PM +, Bertrand Drouvot wrote:
> Hi,
>
> On Wed, Jul 10, 2024 at 07:31:06AM +, Bertrand Drouvot wrote:
> > So, to sum up:
> >
> > A. Locking is now done exclusively with LockNotPinnedObject(Oid classid,
> > Oid objid)
> > so that it's now always clear
Hi,
On Wed, Jul 10, 2024 at 07:31:06AM +, Bertrand Drouvot wrote:
> So, to sum up:
>
> A. Locking is now done exclusively with LockNotPinnedObject(Oid classid, Oid
> objid)
> so that it's now always clear what object we want to acquire a lock for. It
> means
> we are not manipulating direct
Hi,
On Tue, Jul 02, 2024 at 05:56:23AM +, Bertrand Drouvot wrote:
> Hi,
>
> On Mon, Jul 01, 2024 at 09:39:17AM +, Bertrand Drouvot wrote:
> > Hi,
> >
> > On Wed, Jun 26, 2024 at 10:24:41AM +, Bertrand Drouvot wrote:
> > > Hi,
> > >
> > > On Fri, Jun 21, 2024 at 01:22:43PM +, Ber
Hi,
On Mon, Jul 01, 2024 at 09:39:17AM +, Bertrand Drouvot wrote:
> Hi,
>
> On Wed, Jun 26, 2024 at 10:24:41AM +, Bertrand Drouvot wrote:
> > Hi,
> >
> > On Fri, Jun 21, 2024 at 01:22:43PM +, Bertrand Drouvot wrote:
> > > Another thought for the RelationRelationId class case: we coul
Hi,
On Wed, Jun 26, 2024 at 10:24:41AM +, Bertrand Drouvot wrote:
> Hi,
>
> On Fri, Jun 21, 2024 at 01:22:43PM +, Bertrand Drouvot wrote:
> > Another thought for the RelationRelationId class case: we could check if
> > there
> > is a lock first and if there is no lock then acquire one. T
Hi,
On Fri, Jun 21, 2024 at 01:22:43PM +, Bertrand Drouvot wrote:
> Another thought for the RelationRelationId class case: we could check if there
> is a lock first and if there is no lock then acquire one. That way that would
> ensure the relation is always locked (so no "risk" anymore), but
Hi,
On Wed, Jun 19, 2024 at 02:11:50PM +, Bertrand Drouvot wrote:
> To sum up, I did not see any cases that did not lead to 1. or 2., so I think
> it's safe to not add an extra lock for the RelationRelationId case. If, for
> any
> reason, there is still cases that are outside 1. or 2. then th
Hi,
On Mon, Jun 17, 2024 at 05:57:05PM +, Bertrand Drouvot wrote:
> Hi,
>
> On Mon, Jun 17, 2024 at 12:24:46PM -0400, Robert Haas wrote:
> > So to try to sum up here: I'm not sure I agree with this design. But I
> > also feel like the design is not as clear and consistently implemented
> > as
Hi Robert,
On Wed, Jun 19, 2024 at 5:50 PM Robert Haas wrote:
>
> On Wed, Jun 19, 2024 at 7:49 AM Ashutosh Sharma wrote:
> > If the dependency is more, this can hit max_locks_per_transaction
> > limit very fast.
>
> Your experiment doesn't support this conclusion. Very few users would
> have 15
On Wed, Jun 19, 2024 at 7:49 AM Ashutosh Sharma wrote:
> If the dependency is more, this can hit max_locks_per_transaction
> limit very fast.
Your experiment doesn't support this conclusion. Very few users would
have 15 separate user-defined types in the same table, and even if
they did, and drop
Hi,
If the dependency is more, this can hit max_locks_per_transaction
limit very fast. Won't it? I just tried this little experiment with
and without patch.
1) created some UDTs (I have just chosen some random number, 15)
do $$
declare
i int := 1;
type_name text;
begin
while i <= 15 l
Hi,
On Mon, Jun 17, 2024 at 12:24:46PM -0400, Robert Haas wrote:
> On Fri, Jun 14, 2024 at 3:54 AM Bertrand Drouvot
> wrote:
> > > Ah, right. So, I was assuming that, with either this version of your
> > > patch or the earlier version, we'd end up locking the constraint
> > > itself. Was I wrong
On Fri, Jun 14, 2024 at 3:54 AM Bertrand Drouvot
wrote:
> > Ah, right. So, I was assuming that, with either this version of your
> > patch or the earlier version, we'd end up locking the constraint
> > itself. Was I wrong about that?
>
> The child contraint itself is not locked when going through
Hi,
On Thu, Jun 13, 2024 at 04:52:09PM +, Bertrand Drouvot wrote:
> Hi,
>
> On Thu, Jun 13, 2024 at 10:49:34AM -0400, Robert Haas wrote:
> > On Fri, Jun 7, 2024 at 4:41 AM Bertrand Drouvot
> > wrote:
> > > Do you still find the code hard to maintain with v9?
> >
> > I don't think it substan
Hi,
On Thu, Jun 13, 2024 at 02:27:45PM -0400, Robert Haas wrote:
> On Thu, Jun 13, 2024 at 12:52 PM Bertrand Drouvot
> wrote:
> > > > table_open(childRelId, ...) would lock any "ALTER TABLE
> > > > DROP CONSTRAINT"
> > > > already. Not sure I understand your concern here.
> > >
> > > I believe
Hi,
On Thu, Jun 13, 2024 at 10:49:34AM -0400, Robert Haas wrote:
> On Fri, Jun 7, 2024 at 4:41 AM Bertrand Drouvot
> wrote:
> > Do you still find the code hard to maintain with v9?
>
> I don't think it substantially changes my concerns as compared with
> the earlier version.
Thanks for the feed
On Thu, Jun 13, 2024 at 12:52 PM Bertrand Drouvot
wrote:
> > > table_open(childRelId, ...) would lock any "ALTER TABLE DROP
> > > CONSTRAINT"
> > > already. Not sure I understand your concern here.
> >
> > I believe this is not true. This would take a lock on the table, not
> > the constraint it
On Fri, Jun 7, 2024 at 4:41 AM Bertrand Drouvot
wrote:
> Do you still find the code hard to maintain with v9?
I don't think it substantially changes my concerns as compared with
the earlier version.
> > but we're not similarly careful about other operations e.g.
> > ConstraintSetParentConstraint
Hi,
On Thu, Jun 06, 2024 at 04:00:23PM -0400, Robert Haas wrote:
> On Thu, Jun 6, 2024 at 1:56 AM Bertrand Drouvot
> wrote:
> > v9 is more invasive (as it changes code in much more places) than v8 but it
> > is
> > easier to follow (as it is now clear where the new lock is acquired).
>
> Hmm, t
On Thu, Jun 6, 2024 at 1:56 AM Bertrand Drouvot
wrote:
> v9 is more invasive (as it changes code in much more places) than v8 but it is
> easier to follow (as it is now clear where the new lock is acquired).
Hmm, this definitely isn't what I had in mind. Possibly that's a sign
that what I had in
Hi,
On Thu, May 23, 2024 at 02:10:54PM -0400, Robert Haas wrote:
> On Thu, May 23, 2024 at 12:19 AM Bertrand Drouvot
> wrote:
> > The reason why we are using a dirty snapshot here is for the cases where we
> > are
> > recording a dependency on a referenced object that we are creating at the
> >
Hi,
On Thu, May 23, 2024 at 02:10:54PM -0400, Robert Haas wrote:
> On Thu, May 23, 2024 at 12:19 AM Bertrand Drouvot
> wrote:
> > The reason why we are using a dirty snapshot here is for the cases where we
> > are
> > recording a dependency on a referenced object that we are creating at the
> >
On Thu, May 23, 2024 at 12:19 AM Bertrand Drouvot
wrote:
> The reason why we are using a dirty snapshot here is for the cases where we
> are
> recording a dependency on a referenced object that we are creating at the same
> time behind the scene (for example, creating a composite type while creat
Hi,
On Wed, May 22, 2024 at 10:48:12AM -0400, Robert Haas wrote:
> On Wed, May 22, 2024 at 6:21 AM Bertrand Drouvot
> wrote:
> > I started initially with [1] but it was focusing on function-schema only.
>
> Yeah, that's what I thought we would want to do. And then just extend
> that to the other
On Wed, May 22, 2024 at 6:21 AM Bertrand Drouvot
wrote:
> I started initially with [1] but it was focusing on function-schema only.
Yeah, that's what I thought we would want to do. And then just extend
that to the other cases.
> Then I proposed [2] making use of a dirty snapshot when recording t
Hi,
On Tue, May 21, 2024 at 08:53:06AM -0400, Robert Haas wrote:
> On Wed, May 15, 2024 at 6:31 AM Bertrand Drouvot
> wrote:
> > Please find attached v6 (only diff with v5 is moving the tests as suggested
> > above).
>
> I don't immediately know what to think about this patch.
Thanks for lookin
On Wed, May 15, 2024 at 6:31 AM Bertrand Drouvot
wrote:
> Please find attached v6 (only diff with v5 is moving the tests as suggested
> above).
I don't immediately know what to think about this patch. I've known
about this issue for a long time, but I didn't think this was how we
would fix it.
I
Hi,
On Sun, May 19, 2024 at 11:00:00AM +0300, Alexander Lakhin wrote:
> Hello Bertrand,
>
> Probably, it's worth to avoid ERRCODE_INTERNAL_ERROR here in light of [1]
> and [2], as these errors are not that abnormal (not Assert-like).
>
> [1] https://www.postgresql.org/message-id/Zic_GNgos5sMxKoa
Hello Bertrand,
15.05.2024 11:31, Bertrand Drouvot wrote:
On Wed, May 15, 2024 at 10:14:09AM +0900, Michael Paquier wrote:
+if (object_description)
+ereport(ERROR, errmsg("%s does not exist",
object_description));
+else
+ereport(ERROR, e
Hi,
On Wed, May 15, 2024 at 08:31:43AM +, Bertrand Drouvot wrote:
> Hi,
>
> On Wed, May 15, 2024 at 10:14:09AM +0900, Michael Paquier wrote:
> > +++
> > b/src/test/modules/test_dependencies_locks/specs/test_dependencies_locks.spec
> >
> >
> > This introduces a module with only one single
Hi,
On Tue, May 14, 2024 at 03:00:00PM +0300, Alexander Lakhin wrote:
> Hi Bertrand,
>
> 09.05.2024 15:20, Bertrand Drouvot wrote:
> > Oh I see, your test updates an existing dependency. v4 took care about
> > brand new
> > dependencies creation (recordMultipleDependencies()) but forgot to take
Hi,
On Wed, May 15, 2024 at 10:14:09AM +0900, Michael Paquier wrote:
> On Thu, May 09, 2024 at 12:20:51PM +, Bertrand Drouvot wrote:
> > Oh I see, your test updates an existing dependency. v4 took care about
> > brand new
> > dependencies creation (recordMultipleDependencies()) but forgot to
On Thu, May 09, 2024 at 12:20:51PM +, Bertrand Drouvot wrote:
> Oh I see, your test updates an existing dependency. v4 took care about brand
> new
> dependencies creation (recordMultipleDependencies()) but forgot to take care
> about changing an existing dependency (which is done in another c
Hi Bertrand,
09.05.2024 15:20, Bertrand Drouvot wrote:
Oh I see, your test updates an existing dependency. v4 took care about brand new
dependencies creation (recordMultipleDependencies()) but forgot to take care
about changing an existing dependency (which is done in another code path:
changeDe
Hi,
On Tue, Apr 30, 2024 at 08:00:00PM +0300, Alexander Lakhin wrote:
> Hi Bertrand,
>
> But I've discovered yet another possibility to get a broken dependency.
Thanks for the testing and the report!
> Please try this script:
> res=0
> numclients=20
> for ((i=1;i<=100;i++)); do
> for ((c=1;c<=n
Hi Bertrand,
25.04.2024 10:20, Bertrand Drouvot wrote:
postgres=# CREATE FUNCTION f() RETURNS int LANGUAGE SQL RETURN f() + 1;
ERROR: cache lookup failed for function 16400
This stuff does appear before we get a chance to call the new
depLockAndCheckObject()
function.
I think this is what To
Hi,
On Thu, Apr 25, 2024 at 09:00:00AM +0300, Alexander Lakhin wrote:
> Hi,
>
> 25.04.2024 08:00, Bertrand Drouvot wrote:
> >
> > > though concurrent create/drop operations can still produce
> > > the "cache lookup failed" error, which is probably okay, except that it is
> > > an INTERNAL_ERROR,
Hi,
25.04.2024 08:00, Bertrand Drouvot wrote:
though concurrent create/drop operations can still produce
the "cache lookup failed" error, which is probably okay, except that it is
an INTERNAL_ERROR, which assumed to be not easily triggered by users.
I did not see any of those "cache lookup fa
Hi,
On Wed, Apr 24, 2024 at 03:00:00PM +0300, Alexander Lakhin wrote:
> 24.04.2024 11:38, Bertrand Drouvot wrote:
> > I gave more thought to v2 and the approach seems reasonable to me.
> > Basically what
> > it does is that in case the object is already dropped before we take the
> > new lock
>
Hi Bertrand,
24.04.2024 11:38, Bertrand Drouvot wrote:
Please find attached v2 that should not produce the issue anymore (I launched a
lot of attempts without any issues). v1 was not strong enough as it was not
always checking for the dependent object existence. v2 now always checks if the
objec
Hi,
On Tue, Apr 23, 2024 at 04:20:46PM +, Bertrand Drouvot wrote:
> Please find attached v2 that should not produce the issue anymore (I launched
> a
> lot of attempts without any issues). v1 was not strong enough as it was not
> always checking for the dependent object existence. v2 now alwa
Hi,
On Tue, Apr 23, 2024 at 04:59:09AM +, Bertrand Drouvot wrote:
> Hi,
>
> On Mon, Apr 22, 2024 at 03:00:00PM +0300, Alexander Lakhin wrote:
> > 22.04.2024 13:52, Bertrand Drouvot wrote:
> > >
> > > That's weird, I just launched it several times with the patch applied and
> > > I'm not
> >
Hi,
On Mon, Apr 22, 2024 at 03:00:00PM +0300, Alexander Lakhin wrote:
> 22.04.2024 13:52, Bertrand Drouvot wrote:
> >
> > That's weird, I just launched it several times with the patch applied and
> > I'm not
> > able to see the seg fault (while I can see it constently failing on the
> > master
22.04.2024 13:52, Bertrand Drouvot wrote:
That's weird, I just launched it several times with the patch applied and I'm
not
able to see the seg fault (while I can see it constently failing on the master
branch).
Are you 100% sure you tested it against a binary with the patch applied?
Yes, a
Hi,
On Mon, Apr 22, 2024 at 01:00:00PM +0300, Alexander Lakhin wrote:
> Hi Bertrand,
>
> 22.04.2024 11:45, Bertrand Drouvot wrote:
> > Hi,
> >
> > This new thread is a follow-up of [1] and [2].
> >
> > Problem description:
> >
> > We have occasionally observed objects having an orphaned depend
Hi Bertrand,
22.04.2024 11:45, Bertrand Drouvot wrote:
Hi,
This new thread is a follow-up of [1] and [2].
Problem description:
We have occasionally observed objects having an orphaned dependency, the
most common case we have seen is functions not linked to any namespaces.
...
Looking forwar
Hi,
This new thread is a follow-up of [1] and [2].
Problem description:
We have occasionally observed objects having an orphaned dependency, the
most common case we have seen is functions not linked to any namespaces.
Examples to produce such orphaned dependencies:
Scenario 1:
session 1: beg
64 matches
Mail list logo
