From: Ashutosh B. <ash...@en...> - 2013-08-14 09:37:16
|
Yes, that's correct. My patch was not intended to fix this. This was added while fixing a bug for parameterised quals on RemoteQuery I think. Check commits by Amit in this area. On Wed, Aug 14, 2013 at 3:03 PM, Nikhil Sontakke <ni...@st...>wrote: > Yeah, but AFAICS, even 1.1 (and head) *still* has a leak in it. > > Here's the snippet from RemoteQueryNext: > > if (tuplestorestate && !TupIsNull(scanslot)) > tuplestore_puttupleslot(tuplestorestate, scanslot); > > I printed the current memory context inside this function, it is > ""ExecutorState". This means that the tuple will stay around till the query > is executing in its entirety! For large COPY queries this is bound to cause > issues as is also reported by Hitoshi san on another thread. > > I propose that in RemoteQueryNext, before calling the > tuplestore_puttupleslot we switch into > scan_node->ps.ps_ExprContext's ecxt_per_tuple_memory context. It will get > reset, when the next tuple has to be returned to the caller and the leak > will be curtailed. Thoughts? > > Regards, > Nikhils > > > > > On Wed, Aug 14, 2013 at 11:33 AM, Ashutosh Bapat < > ash...@en...> wrote: > >> There has been an overhaul in the planner (and corresponding parts of >> executor) in 1.1, so it would be better if they move to 1.1 after GA. >> >> >> On Wed, Aug 14, 2013 at 10:54 AM, Nikhil Sontakke <ni...@st...>wrote: >> >>> Ah, I see. >>> >>> I was looking at REL_1_0 sources. >>> >>> There are people out there using REL_1_0 as well. >>> >>> Regards, >>> Nikhils >>> >>> >>> On Tue, Aug 13, 2013 at 9:52 AM, Ashutosh Bapat < >>> ash...@en...> wrote: >>> >>>> It should be part of 1.1 as well. It was done to support projection out >>>> of RemoteQuery node. >>>> >>>> >>>> On Tue, Aug 13, 2013 at 7:17 AM, Nikhil Sontakke <ni...@st...>wrote: >>>> >>>>> Hi Ashutosh, >>>>> >>>>> I guess you have changed it in pgxc head? I was looking at 103 and 11 >>>>> branches and saw this. In that even ExecRemoteQuery seems to have an issue >>>>> wherein it's not using the appropriate context. >>>>> >>>>> Regards, >>>>> Nikhils >>>>> >>>>> Sent from my iPhone >>>>> >>>>> On Aug 12, 2013, at 9:54 AM, Ashutosh Bapat < >>>>> ash...@en...> wrote: >>>>> >>>>> >>>>> Welcome to the mess ;) and enjoy junk food. >>>>> >>>>> Sometime back, I have changed ExecRemoteQuery to be called in the same >>>>> fashion as other Scan nodes. So, you will see ExecRemoteQuery calling >>>>> ExecScan with RemoteQueryNext as the iterator. So, I assume your comment >>>>> pertains to RemoteQueryNext and its minions and not ExecRemoteQuery per say! >>>>> >>>>> This code needs a lot of rework, removing duplications, using proper >>>>> way of materialisation, central response handler and error handler etc. If >>>>> we clean up this code, some improvements in planner (like using MergeAppend >>>>> plan) for Sort, will be possible. Regarding materialisation, the code uses >>>>> a linked list for materialising the rows from datanodes (in case the same >>>>> connection needs to be given to other remote query node), which must be >>>>> eating a lot of performance. Instead we should be using some kind of >>>>> tuplestore there. We actually use tuplestore (as well) in the RemoteQuery >>>>> node; the same method can be used. >>>>> >>>>> >>>>> On Sat, Aug 10, 2013 at 10:48 PM, Nikhil Sontakke <ni...@st... >>>>> > wrote: >>>>> >>>>>> Hi, >>>>>> >>>>>> Have a Query about ExecRemoteQuery. >>>>>> >>>>>> The logic seems to have been modeled after ExecMaterial. ISTM, that >>>>>> it should have been modeled after ExecScan because we fetch tuples, and >>>>>> those which match the qual should be sent up. ExecMaterial is for >>>>>> materializing and collecting and storing tuples. >>>>>> >>>>>> Can anyone explain? The reason for asking this is I am suspecting a >>>>>> big memory leak in this code path. We are not using any expression context >>>>>> nor we are freeing up tuples as we scan for the one which qualifies. >>>>>> >>>>>> Regards, >>>>>> Nikhils >>>>>> -- >>>>>> StormDB - http://www.stormdb.com >>>>>> The Database Cloud >>>>>> >>>>>> ------------------------------------------------------------------------------ >>>>>> Get 100% visibility into Java/.NET code with AppDynamics Lite! >>>>>> It's a free troubleshooting tool designed for production. >>>>>> Get down to code-level detail for bottlenecks, with <2% overhead. >>>>>> Download for free and get started troubleshooting in minutes. >>>>>> >>>>>> http://pubads.g.doubleclick.net/gampad/clk?id=48897031&iu=/4140/ostg.clktrk >>>>>> _______________________________________________ >>>>>> Postgres-xc-developers mailing list >>>>>> Pos...@li... >>>>>> https://lists.sourceforge.net/lists/listinfo/postgres-xc-developers >>>>>> >>>>>> >>>>> >>>>> >>>>> -- >>>>> Best Wishes, >>>>> Ashutosh Bapat >>>>> EntepriseDB Corporation >>>>> The Postgres Database Company >>>>> >>>>> >>>> >>>> >>>> -- >>>> Best Wishes, >>>> Ashutosh Bapat >>>> EntepriseDB Corporation >>>> The Postgres Database Company >>>> >>> >>> >>> >>> -- >>> StormDB - http://www.stormdb.com >>> The Database Cloud >>> >> >> >> >> -- >> Best Wishes, >> Ashutosh Bapat >> EntepriseDB Corporation >> The Postgres Database Company >> > > > > -- > StormDB - http://www.stormdb.com > The Database Cloud > -- Best Wishes, Ashutosh Bapat EntepriseDB Corporation The Postgres Database Company |