Thread: Re: pgstat: remove delayed destroy / pipe: socketerror fix
That's probably not a bad idea. AFAIK we haven't had reports of it elsewhere, but it oculd happen. Want to code up a new patch, and run some tests? //Magnus > -----Original Message----- > > Also, do we want to move the retry loop to pgwin32_recv? > That seems like a good idea. I'm not sure users of recv > should ever have to deal with WSAEWOULDBLOCK as it's not > really an error. > > Pete > > >>> "Magnus Hagander" <mha@sollentuna.net> 04/06/06 9:58 pm >>> > > > Attached are two patches which in combination make > pg_stat_activity > > > > work reliably for us on Windows. > > > ... > > > pgstat.patch removes the delayed destroy code for backends, > > databases, > > > and tables. Database and table entries are cleaned up immediately > > > > upon receipt of the appropriate message. > > > > I'll go ahead and apply the delayed-destroy-removal part > > (just to HEAD for the time being --- seems a bit risky to > > apply it to the stable branches). The Windows-specific > > change sounds like it may need more review. > > Actually, I think that's mostly me being confused and taking others > with > me ;-) > > It's definitly a problem, and we have a solution there. The one thing > I'm still a bit concerned about is: Do we need a CHECK_FOR_INTERRUPTS, > and do we need an upper limit on the spinning. In theory we can spin > with 100% CPU usage, which is not good. So we should either spin a > limited amount of times, or we should perhaps introduce a tiny delay? > > //Magnus > > ---------------------------(end of > broadcast)--------------------------- > TIP 1: if posting/reading through Usenet, please send an appropriate > subscribe-nomail command to majordomo@postgresql.org so that > your > message can get through to the mailing list cleanly >
Sounds good. I'll check how much we're actually looping too. Pete >>> "Magnus Hagander" <mha@sollentuna.net> 04/06/06 10:27 pm >>> That's probably not a bad idea. AFAIK we haven't had reports of it elsewhere, but it oculd happen. Want to code up a new patch, and run some tests? //Magnus > -----Original Message----- > > Also, do we want to move the retry loop to pgwin32_recv? > That seems like a good idea. I'm not sure users of recv > should ever have to deal with WSAEWOULDBLOCK as it's not > really an error. > > Pete > > >>> "Magnus Hagander" <mha@sollentuna.net> 04/06/06 9:58 pm >>> > > > Attached are two patches which in combination make > pg_stat_activity > > > > work reliably for us on Windows. > > > ... > > > pgstat.patch removes the delayed destroy code for backends, > > databases, > > > and tables. Database and table entries are cleaned up immediately > > > > upon receipt of the appropriate message. > > > > I'll go ahead and apply the delayed-destroy-removal part > > (just to HEAD for the time being --- seems a bit risky to > > apply it to the stable branches). The Windows-specific > > change sounds like it may need more review. > > Actually, I think that's mostly me being confused and taking others > with > me ;-) > > It's definitly a problem, and we have a solution there. The one thing > I'm still a bit concerned about is: Do we need a CHECK_FOR_INTERRUPTS, > and do we need an upper limit on the spinning. In theory we can spin > with 100% CPU usage, which is not good. So we should either spin a > limited amount of times, or we should perhaps introduce a tiny delay? > > //Magnus > > ---------------------------(end of > broadcast)--------------------------- > TIP 1: if posting/reading through Usenet, please send an appropriate > subscribe-nomail command to majordomo@postgresql.org so that > your > message can get through to the mailing list cleanly >
Would someone generate a patch that includes all the new ideas and post it here? Thanks. --------------------------------------------------------------------------- Peter Brant wrote: > Sounds good. I'll check how much we're actually looping too. > > Pete > > >>> "Magnus Hagander" <mha@sollentuna.net> 04/06/06 10:27 pm >>> > That's probably not a bad idea. AFAIK we haven't had reports of it > elsewhere, but it oculd happen. Want to code up a new patch, and run > some tests? > > //Magnus > > > -----Original Message----- > > > > Also, do we want to move the retry loop to pgwin32_recv? > > That seems like a good idea. I'm not sure users of recv > > should ever have to deal with WSAEWOULDBLOCK as it's not > > really an error. > > > > Pete > > > > >>> "Magnus Hagander" <mha@sollentuna.net> 04/06/06 9:58 pm >>> > > > > Attached are two patches which in combination make > > pg_stat_activity > > > > > > work reliably for us on Windows. > > > > ... > > > > pgstat.patch removes the delayed destroy code for backends, > > > databases, > > > > and tables. Database and table entries are cleaned up > immediately > > > > > > upon receipt of the appropriate message. > > > > > > I'll go ahead and apply the delayed-destroy-removal part > > > (just to HEAD for the time being --- seems a bit risky to > > > apply it to the stable branches). The Windows-specific > > > change sounds like it may need more review. > > > > Actually, I think that's mostly me being confused and taking others > > with > > me ;-) > > > > It's definitly a problem, and we have a solution there. The one > thing > > I'm still a bit concerned about is: Do we need a > CHECK_FOR_INTERRUPTS, > > and do we need an upper limit on the spinning. In theory we can spin > > with 100% CPU usage, which is not good. So we should either spin a > > limited amount of times, or we should perhaps introduce a tiny > delay? > > > > //Magnus > > > > ---------------------------(end of > > broadcast)--------------------------- > > TIP 1: if posting/reading through Usenet, please send an appropriate > > subscribe-nomail command to majordomo@postgresql.org so that > > your > > message can get through to the mailing list cleanly > > > > ---------------------------(end of broadcast)--------------------------- > TIP 3: Have you checked our extensive FAQ? > > http://www.postgresql.org/docs/faq > -- Bruce Momjian http://candle.pha.pa.us EnterpriseDB http://www.enterprisedb.com + If your life is a hard drive, Christ can be your backup. +
I'm still planning on doing this. I did add a loop around the second WSARecv in pgwin32_recv() as that was our best guess about where the error was coming from. As it turns out, that's apparently not the case and I haven't had a chance to come back to it yet. It might be that my original patch (putting the loop in piperead()) is the best approach. All indications are that it fixes the problem and it's the lowest impact change. I'm still quite curious what API call is returning the WSAEWOULDBLOCK though... Pete >>> Bruce Momjian <pgman@candle.pha.pa.us> 04/19/06 5:07 am >>> Would someone generate a patch that includes all the new ideas and post it here? Thanks.
It turns out the problem is that port/pipe.c is compiled with -DFRONTEND and include/port/win32.h wraps the recv to pgwin32_recv macro in a #ifndef FRONTEND. We've actually been using the WinSock recv function directly (verified with gcc -E). If somebody else could take over actually fixing this, that would be great. As I mentioned before, we're heading away from Windows for the time being. Pete >>> Peter Brant 04/19/06 5:16 pm >>> I'm still planning on doing this. I did add a loop around the second WSARecv in pgwin32_recv() as that was our best guess about where the error was coming from. As it turns out, that's apparently not the case and I haven't had a chance to come back to it yet. It might be that my original patch (putting the loop in piperead()) is the best approach. All indications are that it fixes the problem and it's the lowest impact change. I'm still quite curious what API call is returning the WSAEWOULDBLOCK though... Pete >>> Bruce Momjian <pgman@candle.pha.pa.us> 04/19/06 5:07 am >>> Would someone generate a patch that includes all the new ideas and post it here? Thanks.
Right, and the FRONTEND fix should correct this. Sorry you had to do so much legwork to find my mistake. --------------------------------------------------------------------------- Peter Brant wrote: > I'm still planning on doing this. > > I did add a loop around the second WSARecv in pgwin32_recv() as that > was our best guess about where the error was coming from. As it turns > out, that's apparently not the case and I haven't had a chance to come > back to it yet. > > It might be that my original patch (putting the loop in piperead()) is > the best approach. All indications are that it fixes the problem and > it's the lowest impact change. I'm still quite curious what API call is > returning the WSAEWOULDBLOCK though... > > Pete > > >>> Bruce Momjian <pgman@candle.pha.pa.us> 04/19/06 5:07 am >>> > Would someone generate a patch that includes all the new ideas and > post > it here? Thanks. > > > ---------------------------(end of broadcast)--------------------------- > TIP 9: In versions below 8.0, the planner will ignore your desire to > choose an index scan if your joining column's datatypes do not > match > -- Bruce Momjian http://candle.pha.pa.us EnterpriseDB http://www.enterprisedb.com + If your life is a hard drive, Christ can be your backup. +