On Tue, Dec 20, 2016 at 8:04 PM, Robert Haas <[email protected]> wrote: > On Sat, Dec 17, 2016 at 5:46 AM, Amit Kapila <[email protected]> wrote: >> Yeah, but we are planning to add a generic flag in WaitEvent structure >> which can be used for similar purpose. However, as per your >> suggestion, I have changed it only for Win32 port. Also, I kept the >> change in ModifyWaitEvent API as that seems to be okay considering >> 'reset' is a generic flag in WaitEvent structure. > > Well, we don't really need the change in ModifyWaitEvent if we have > the change in WaitEventSetWaitBlock, right? >
Yes.
> I'd be inclined to ditch
> the former and keep the latter.
>
Okay, I think you want to keep the change just for Win32 which seems
fair as we haven't noticed such problem on any other platform.
> Also, this doesn't look right:
>
> + for (cur_event = set->events;
> + cur_event < (set->events + set->nevents)
> + && returned_events < nevents;
> + cur_event++)
> + {
> + if (cur_event->reset)
> + {
> + WaitEventAdjustWin32(set, cur_event);
> + cur_event->reset = false;
> + }
> + }
>
> There's no need to include returned_events < nevents in the loop
> condition here because returned_events isn't changing.
>
Fixed.
> I think I'd also guard the reset flag with #ifdef WIN32. If it's not
> properly supported elsewhere it's just a foot-gun, and there seems to
> be no reason to write the code to properly support it elsewhere,
> whatever that would mean.
>
Okay.
Ashutosh Sharma has helped to test that pldebugger issue is fixed with
attached version.
--
With Regards,
Amit Kapila.
EnterpriseDB: http://www.enterprisedb.com
reset_wait_events_v4.patch
Description: Binary data
-- Sent via pgsql-hackers mailing list ([email protected]) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
