On Thu, Mar 28, 2024 at 12:01 PM Tom Lane wrote:
> > Well, there's the abort case, too, which I think is almost equally
> > important.
>
> True, but in the abort case there probably *are* resources to be
> cleaned up, so I'm not seeing that the fast-path idea helps.
> Although maybe the idea of b
Robert Haas writes:
> On Thu, Mar 28, 2024 at 11:50 AM Tom Lane wrote:
>> Yeah, I was thinking about that too. The normal case is that you
>> don't hold any releasable resources except locks when arriving at
>> CommitSubTransaction --- if you do, it's a bug and we're going to
>> print leak warni
On Thu, Mar 28, 2024 at 11:50 AM Tom Lane wrote:
> Yeah, I was thinking about that too. The normal case is that you
> don't hold any releasable resources except locks when arriving at
> CommitSubTransaction --- if you do, it's a bug and we're going to
> print leak warnings. Seems like maybe it'd
Robert Haas writes:
> Hmm, I wonder if that's actually where the cycles are going. There's
> an awful lot of separate function calls inside CommitSubTransaction(),
> and in the common case, each one of them has to individually decide
> that it doesn't need to do anything. Sure, they're all fast, b
On Thu, Mar 28, 2024 at 10:59 AM Tom Lane wrote:
> Yeah. The whole ResourceOwner mechanism is not exactly lightweight,
> but it's hard to argue that we don't need it. I wonder whether we
> could get anywhere by deeming that a "small enough" subtransaction
> doesn't need to have its resources cle
Robert Haas writes:
> I sort of assumed you were going to commit the patch as you had it.
OK, I will move ahead on that.
> I actually
> really wish we could find some way of making subtransactions
> significantly lighter-wait, because I think the cost of spinning up
> and tearing down a trivial
On Wed, Mar 27, 2024 at 5:28 PM Tom Lane wrote:
> After mulling it over for awhile, I still think the extra checking
> is appropriate, especially since this patch is enlarging the set of
> things that can happen in parallel mode. How do you want to proceed?
I sort of assumed you were going to co
Robert Haas writes:
> On Mon, Mar 25, 2024 at 11:36 AM Tom Lane wrote:
>> ... I don't see why this case is different,
>> especially when the added cost compared to HEAD is not much more than
>> one C function call.
> Well, I explained why *I* thought it was different, but obviously you
> don't
On Mon, Mar 25, 2024 at 11:36 AM Tom Lane wrote:
> By that logic, we should rip out every Assert in the system, as well
> as all of the (extensive) resource leak checking that already happens
> during CommitTransaction. We've always felt that those leak checks
> were worth the cost to help us fin
Robert Haas writes:
> On Sat, Mar 23, 2024 at 12:31 PM Tom Lane wrote:
>> However, the calling logic seems a bit shy of a load, in that it
>> trusts IsInParallelMode() completely to decide whether to check for
>> leaked parallel contexts. So we'd miss the case where somebody did
>> ExitParallelM
On Sat, Mar 23, 2024 at 12:31 PM Tom Lane wrote:
> However, the calling logic seems a bit shy of a load, in that it
> trusts IsInParallelMode() completely to decide whether to check for
> leaked parallel contexts. So we'd miss the case where somebody did
> ExitParallelMode without having cleaned
Robert Haas writes:
> On Fri, Mar 22, 2024 at 4:37 PM Tom Lane wrote:
>> I think these things are already dealt with. However, one thing
>> worth questioning is that CommitSubTransaction() will just silently
>> kill any workers started during the current subxact, and likewise
>> CommitTransactio
On Fri, Mar 22, 2024 at 4:37 PM Tom Lane wrote:
> Fair enough. In the attached v2, I wrote "change the transaction
> state (other than by using a subtransaction for error recovery)";
> what do you think of that?
I think that's pretty good. I wonder if there are some bizarre cases
where the patch
Robert Haas writes:
> - I don't think the documentation changes are entirely accurate. The
> whole point of the patch is to allow parallel workers to make changes
> to the transaction state, but the documentation says you can't. Maybe
> we should just delete "change the transaction state" entirely
On Fri, Mar 22, 2024 at 1:52 PM Tom Lane wrote:
> Robert Haas writes:
> > I agree with the general direction. A few comments:
>
> > - Isn't it redundant to test if IsInParallelMode() ||
> > IsParallelWorker()? We can't be in a parallel worker without also
> > being in parallel mode, except during
Robert Haas writes:
> I agree with the general direction. A few comments:
> - Isn't it redundant to test if IsInParallelMode() ||
> IsParallelWorker()? We can't be in a parallel worker without also
> being in parallel mode, except during the worker startup sequence.
Hmm. The existing code in As
Robert Haas writes:
> On Fri, Dec 29, 2023 at 12:56 PM Tom Lane wrote:
>> Here's a draft patch along this line. Basically the idea is that
>> subtransactions used for error control are now legal in parallel
>> mode (including in parallel workers) so long as they don't try to
>> acquire their own
On Fri, Dec 29, 2023 at 12:56 PM Tom Lane wrote:
> Here's a draft patch along this line. Basically the idea is that
> subtransactions used for error control are now legal in parallel
> mode (including in parallel workers) so long as they don't try to
> acquire their own XIDs. I had to clean up s
I wrote:
> Hao Zhang writes:
>> IMHO, there are other error reports in the function
>> BeginInternalSubTransaction(), like
> Sure, but all the other ones are extremely hard to hit, which is why
> we didn't bother to worry about them to begin with. If we want to
> make this more formally bulletpr
Hao Zhang writes:
>> The only readily-reachable error case in BeginInternalSubTransaction
>> is this specific one about IsInParallelMode, which was added later
>> than the original design and evidently with not a lot of thought or
>> testing. The comment for it speculates about whether we could g
Thanks for your reply. These patches look good to me!
> The only readily-reachable error case in BeginInternalSubTransaction
> is this specific one about IsInParallelMode, which was added later
> than the original design and evidently with not a lot of thought or
> testing. The comment for it spe
I wrote:
> The only readily-reachable error case in BeginInternalSubTransaction
> is this specific one about IsInParallelMode, which was added later
> than the original design and evidently with not a lot of thought or
> testing. The comment for it speculates about whether we could get
> rid of it
Andres Freund writes:
> On 2023-12-01 20:04:15 -0500, Tom Lane wrote:
>> Thanks for the report! I see the problem is that we're not expecting
>> BeginInternalSubTransaction to fail. However, I'm not sure I like
>> this solution, mainly because it's only covering a fraction of the
>> problem. Th
Hi,
On 2023-12-01 20:04:15 -0500, Tom Lane wrote:
> Hao Zhang writes:
> > I found a problem when executing the plpython function:
> > After the plpython function returns an error, in the same session, if we
> > continue to execute
> > plpython function, the server panic will be caused.
>
> Thank
Hao Zhang writes:
> I found a problem when executing the plpython function:
> After the plpython function returns an error, in the same session, if we
> continue to execute
> plpython function, the server panic will be caused.
Thanks for the report! I see the problem is that we're not expecting
Hi hackers,
I found a problem when executing the plpython function:
After the plpython function returns an error, in the same session, if we
continue to execute
plpython function, the server panic will be caused.
*Reproduce*
preparation
SET max_parallel_workers_per_gather=4;
SET parallel_setup_co
26 matches
Mail list logo
