On Tue, Oct 24, 2017 at 8:59 PM, Rob Emery <re-pg...@codeweavers.net> wrote:
> Awesome thanks. > > What's the release schedule like for pgagent? > I have no idea. That can be answered by Dave only. Dave? -- Thanks & Regards, Ashesh Vashi EnterpriseDB INDIA: Enterprise PostgreSQL Company <http://www.enterprisedb.com/> *http://www.linkedin.com/in/asheshvashi* <http://www.linkedin.com/in/asheshvashi> > > Thanks, > Rob > > > > On 24 October 2017 at 16:05, Ashesh Vashi <ashesh.va...@enterprisedb.com> > wrote: > >> On Tue, Oct 24, 2017 at 8:30 PM, Ashesh Vashi < >> ashesh.va...@enterprisedb.com> wrote: >> >>> On Tue, Oct 24, 2017 at 8:21 PM, Robert Emery < >>> robertem...@codeweavers.net> wrote: >>> >>>> Hello again, >>>> >>>> I can't see any other way of solving this problem without either >>>> exposing a method on JobThread to delete the Job owned by the thread which >>>> is called by https://github.com/postgres/pg >>>> agent/blob/master/pgAgent.cpp#L139 or another method that only gives >>>> up the connection which is called in the same place as the delete jt. >>>> >>>> Is there a problem with chaining the ~JobThread() and ~Job() together >>>> as I suggested previously? This does solve the leak. >>>> >>> Nothing wrong in it. >>> My intention was to release the object asap. >>> >>> As per your findings, it won't go in that execution flow in all cases. I >>> will commit your patch. >>> >> Committed your original patch. >> >> -- >> >> Thanks & Regards, >> >> Ashesh Vashi >> EnterpriseDB INDIA: Enterprise PostgreSQL Company >> <http://www.enterprisedb.com/> >> >> >> *http://www.linkedin.com/in/asheshvashi* >> <http://www.linkedin.com/in/asheshvashi> >> >>> >>> -- Thanks, Ashesh >>> >>>> >>>> Many Thanks, >>>> Rob >>>> >>>> On 23 October 2017 at 20:09, Rob Emery <re-pg...@codeweavers.net> >>>> wrote: >>>> >>>>> Hiya, >>>>> >>>>> I've just confirmed neither of these patches resolve the issue, it >>>>> appears in the error case I'm experiencing the JobThread::Entry() never >>>>> executes. >>>>> >>>>> To clarify I'm talking about the path entering the else in >>>>> pgAgent.cpp:125 which will save into pga_joblog with jlgstatus = "i" >>>>> >>>>> This path explicitly deletes jt however deleting jt doesn't clean up >>>>> the connection, hence why I cascaded the delete in the original patch. >>>>> >>>>> I hope that makes sense >>>>> >>>>> Thanks, >>>>> Rob >>>>> >>>>> On 23 October 2017 at 08:18, Neel Patel <neel.pa...@enterprisedb.com> >>>>> wrote: >>>>> >>>>>> Hi Ashesh, >>>>>> >>>>>> I just added condition before delete the job otherwise it looks good. >>>>>> Correct me if I am wrong. >>>>>> >>>>>> if (job != NULL) >>>>>> { >>>>>> delete job; >>>>>> job = NULL; >>>>>> } >>>>>> >>>>>> I have created two instances of pgagent on database cluster. As of >>>>>> now not able to see any leak and keep you updated if found anything. >>>>>> >>>>>> @Robert - Can you also test at your environment and keep us updated >>>>>> for any leak ? >>>>>> >>>>>> Thanks, >>>>>> Neel Patel >>>>>> >>>>>> On Mon, Oct 23, 2017 at 10:30 AM, Ashesh Vashi < >>>>>> ashesh.va...@enterprisedb.com> wrote: >>>>>> >>>>>>> Hi Rob, >>>>>>> >>>>>>> How about this? >>>>>>> >>>>>>> >>>>>>> -- >>>>>>> >>>>>>> Thanks & Regards, >>>>>>> >>>>>>> Ashesh Vashi >>>>>>> EnterpriseDB INDIA: Enterprise PostgreSQL Company >>>>>>> <http://www.enterprisedb.com> >>>>>>> >>>>>>> >>>>>>> *http://www.linkedin.com/in/asheshvashi* >>>>>>> <http://www.linkedin.com/in/asheshvashi> >>>>>>> >>>>>>> On Sat, Oct 21, 2017 at 8:36 PM, Rob Emery <re-pg...@codeweavers.net >>>>>>> > wrote: >>>>>>> >>>>>>>> Hi, >>>>>>>> >>>>>>>> Following on from https://www.postgresql.org/mes >>>>>>>> sage-id/CA%2BOCxoz4tONxSpd1rdU-9SPKRzucz8Bar2CXkEDnCwV6H77Zy >>>>>>>> A%40mail.gmail.com >>>>>>>> >>>>>>>> I think I've identified and fixed the issue, please see the patch >>>>>>>> attached. >>>>>>>> >>>>>>>> As I understand it when there are multiple pgagent instances and >>>>>>>> they clash executing a job (i.e rc != 1 on job.cpp:38), the loser of >>>>>>>> the >>>>>>>> conflict's thread will never be executed (i.e. job.cpp:418 >>>>>>>> JobThread::Entry), which is responsible for deleting the job owned by >>>>>>>> the >>>>>>>> thread, meaning that the connection is never returned to the pool. By >>>>>>>> moving the delete of the job into the destructor, we can assure that >>>>>>>> the >>>>>>>> connection is tidied up in both cases as the thread is deleted in the >>>>>>>> error >>>>>>>> case explicitly in pgAgent.cpp:185. >>>>>>>> >>>>>>>> The only possibly unintended difference that I can see with doing >>>>>>>> this is that the log "Completed job: %s" is now output when before it >>>>>>>> wasn't, however I think this new behaviour is actually correct as the >>>>>>>> job >>>>>>>> object is completed at that time. >>>>>>>> >>>>>>>> Thanks, >>>>>>>> Rob >>>>>>>> >>>>>>>> <https://codeweavers.net> >>>>>>>> >>>>>>>> >>>>>>>> <http://us15.campaign-archive1.com/?u=fcb361cfa194cf70551bc5169&id=f556b0bf09> >>>>>>>> Codeweavers >>>>>>>> October >>>>>>>> Newsletter >>>>>>>> <http://us15.campaign-archive1.com/?u=fcb361cfa194cf70551bc5169&id=f556b0bf09> >>>>>>>> *l *Auto Trader extends partnership with Codeweavers >>>>>>>> <https://codeweavers.net/company-blog/auto-trader-extends-partnership-with-codeweavers-to-power-finance-on-auto-trader-websites> >>>>>>>> >>>>>>>> >>>>>>>> <https://codeweavers.net/automotive-vision-conference-agenda> >>>>>>>> >>>>>>>> *What are Codeweavers doing to gear up for GDPR? >>>>>>>> <https://codeweavers.net/company-blog/what-are-codeweavers-doing-to-gear-up-for-gdpr>* >>>>>>>> >>>>>>>> >>>>>>>> >>>>>>>> *Phone:* 0800 021 0888 * Email: *contac...@codeweavers.net >>>>>>>> *Codeweavers Ltd* | Barn 4 | Dunston Business Village | Dunston | >>>>>>>> ST18 9AB >>>>>>>> Registered in England and Wales No. 04092394 | VAT registration no. >>>>>>>> 974 9705 63 >>>>>>>> >>>>>>>> -- >>>>>>>> Robert Emery >>>>>>>> Infrastructure Director >>>>>>>> >>>>>>>> E: robertem...@codeweavers.net | T: 01785 711633 | W: >>>>>>>> www.codeweavers.net >>>>>>>> >>>>>>> >>>> >>>> >>>> -- >>>> Robert Emery >>>> Infrastructure Director >>>> >>>> E: robertem...@codeweavers.net | T: 01785 711633 | W: >>>> www.codeweavers.net >>>> >>>> <https://codeweavers.net> >>>> >>>> >>>> <http://us15.campaign-archive1.com/?u=fcb361cfa194cf70551bc5169&id=f556b0bf09> >>>> Codeweavers >>>> October >>>> Newsletter >>>> <http://us15.campaign-archive1.com/?u=fcb361cfa194cf70551bc5169&id=f556b0bf09> >>>> *l *Auto Trader extends partnership with Codeweavers >>>> <https://codeweavers.net/company-blog/auto-trader-extends-partnership-with-codeweavers-to-power-finance-on-auto-trader-websites> >>>> >>>> >>>> <https://codeweavers.net/automotive-vision-conference-agenda> >>>> >>>> *What are Codeweavers doing to gear up for GDPR? >>>> <https://codeweavers.net/company-blog/what-are-codeweavers-doing-to-gear-up-for-gdpr>* >>>> >>>> >>>> >>>> *Phone:* 0800 021 0888 * Email: *contac...@codeweavers.net >>>> *Codeweavers Ltd* | Barn 4 | Dunston Business Village | Dunston | ST18 >>>> 9AB >>>> Registered in England and Wales No. 04092394 | VAT registration no. 974 >>>> 9705 63 >>>> >>>> -- >>>> Robert Emery >>>> Infrastructure Director >>>> >>>> E: robertem...@codeweavers.net | T: 01785 711633 | W: >>>> www.codeweavers.net >>>> >>> > <https://codeweavers.net> > > > <http://us15.campaign-archive1.com/?u=fcb361cfa194cf70551bc5169&id=f556b0bf09> > Codeweavers > October > Newsletter > <http://us15.campaign-archive1.com/?u=fcb361cfa194cf70551bc5169&id=f556b0bf09> > *l *Auto Trader extends partnership with Codeweavers > <https://codeweavers.net/company-blog/auto-trader-extends-partnership-with-codeweavers-to-power-finance-on-auto-trader-websites> > > > <https://codeweavers.net/automotive-vision-conference-agenda> > > *What are Codeweavers doing to gear up for GDPR? > <https://codeweavers.net/company-blog/what-are-codeweavers-doing-to-gear-up-for-gdpr>* > > > > *Phone:* 0800 021 0888 * Email: *contac...@codeweavers.net > *Codeweavers Ltd* | Barn 4 | Dunston Business Village | Dunston | ST18 9AB > Registered in England and Wales No. 04092394 | VAT registration no. 974 > 9705 63 > > <https://www.linkedin.com/company/codeweavers-limited> > <https://vimeo.com/codeweaversltd> [image: > https://plus.google.com/b/105942302039373248738/+CodeweaversNet] > <https://plus.google.com/b/105942302039373248738/+CodeweaversNet> [image: > https://twitter.com/CodeweaversTeam] <https://twitter.com/CodeweaversTeam> >
