Re: [HACKERS] Interrupting long external library calls

2012-06-07 Thread Sandro Santilli
On Sat, May 26, 2012 at 01:24:26PM +0200, Florian Pflug wrote:
 On May26, 2012, at 12:40 , Simon Riggs wrote:
  On 25 May 2012 17:34, Tom Lane t...@sss.pgh.pa.us wrote:
  I assume that the geos::util::Interrupt::request() call sets a flag
  somewhere that's going to be periodically checked in long-running
  loops.  Would it be possible for the periodic checks to include a
  provision for a callback into Postgres-specific glue code, wherein
  you could test the same flags CHECK_FOR_INTERRUPTS does?  A similar
  approach might then be usable in other contexts, and it seems safer
  to me than messing with a host environment's signal handling.
  
  Can we do that as a macro, e.g.
  
  POSTGRES_LIBRARY_INTERRUPT_CHECK()
  
  Otherwise the fix to this problem may be worse - faulty interrupt
  handlers are worse than none at all.
 
 As it stands, ProcessInterrupts() sometimes returns instead of
 ereport()ing even if InterruptPending is set, interrupts aren't
 held off, and the code isn't in a critical section. That happens if
 QueryCancelPending (or however that's called) gets reset after a
 SIGINT arrived but before CHECK_FOR_INTERRUPTS() is called. Or at
 least that is how I interpret the comment at the bottom of that
 function...
 
 We could thus easily provide POSTGRES_LIBRARY_INTERRUPT_CHECK() if
 we're content with the (slim, probably) chance of false positives.
 
 Or we'd need to refactor things in a way that allows the hypothetical
 POSTGRES_LIBRARY_INTERRUPT_CHECK() to re-use the tests in
 ProcessInterrupts(), but without modifying any of the flags.

So back to this, shortly after discovering (sorry for ignorance, but I
just don't care about programming in a black box environment) that windows
doesn't support posix signals.

If I understood correctly the CHECK_FOR_INTERRUPTS postgresql function
does also take care of events dispatching within windows, so that if
nothing calls that macro there's no way that a pqsignal handler would
be invoked. Is that right ?

In that case I can understand Tom's advice about providing a callback,
and then I would only need to perform the events flushing part of
from within the callback, and only for windows.

Does it sound right ?

--strk; 

-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers


Re: [HACKERS] Interrupting long external library calls

2012-06-07 Thread Florian Pflug
On Jun7, 2012, at 10:20 , Sandro Santilli wrote:
 On Sat, May 26, 2012 at 01:24:26PM +0200, Florian Pflug wrote:
 On May26, 2012, at 12:40 , Simon Riggs wrote:
 On 25 May 2012 17:34, Tom Lane t...@sss.pgh.pa.us wrote:
 I assume that the geos::util::Interrupt::request() call sets a flag
 somewhere that's going to be periodically checked in long-running
 loops.  Would it be possible for the periodic checks to include a
 provision for a callback into Postgres-specific glue code, wherein
 you could test the same flags CHECK_FOR_INTERRUPTS does?  A similar
 approach might then be usable in other contexts, and it seems safer
 to me than messing with a host environment's signal handling.
 
 Can we do that as a macro, e.g.
 
 POSTGRES_LIBRARY_INTERRUPT_CHECK()
 
 Otherwise the fix to this problem may be worse - faulty interrupt
 handlers are worse than none at all.
 
 As it stands, ProcessInterrupts() sometimes returns instead of
 ereport()ing even if InterruptPending is set, interrupts aren't
 held off, and the code isn't in a critical section. That happens if
 QueryCancelPending (or however that's called) gets reset after a
 SIGINT arrived but before CHECK_FOR_INTERRUPTS() is called. Or at
 least that is how I interpret the comment at the bottom of that
 function...
 
 We could thus easily provide POSTGRES_LIBRARY_INTERRUPT_CHECK() if
 we're content with the (slim, probably) chance of false positives.
 
 Or we'd need to refactor things in a way that allows the hypothetical
 POSTGRES_LIBRARY_INTERRUPT_CHECK() to re-use the tests in
 ProcessInterrupts(), but without modifying any of the flags.
 
 So back to this, shortly after discovering (sorry for ignorance, but I
 just don't care about programming in a black box environment) that windows
 doesn't support posix signals.
 
 If I understood correctly the CHECK_FOR_INTERRUPTS postgresql function
 does also take care of events dispatching within windows, so that if
 nothing calls that macro there's no way that a pqsignal handler would
 be invoked. Is that right ?

Yes. 

 In that case I can understand Tom's advice about providing a callback,
 and then I would only need to perform the events flushing part of
 from within the callback, and only for windows.

Why would you need a signal handler in the library at all, then? Just
test the same flags that CHECK_FOR_INTERRUPTS does in the callback, and
call your interrupt request method if they indicate abort. (Or, slightly
cleaner maybe, allow the callback to abort processing by returning false)

best regards,
Florian Pflug


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers


Re: [HACKERS] Interrupting long external library calls

2012-06-07 Thread Sandro Santilli
On Thu, Jun 07, 2012 at 12:00:09PM +0200, Florian Pflug wrote:
 On Jun7, 2012, at 10:20 , Sandro Santilli wrote:

  In that case I can understand Tom's advice about providing a callback,
  and then I would only need to perform the events flushing part of
  from within the callback, and only for windows.
 
 Why would you need a signal handler in the library at all, then? Just
 test the same flags that CHECK_FOR_INTERRUPTS does in the callback, and
 call your interrupt request method if they indicate abort. (Or, slightly
 cleaner maybe, allow the callback to abort processing by returning false)

I'm just afraid that invoking a callback (from a library to user code)
could be significantly slower than simply lookup a variable, especially
if the interruption checking is performed very frequently. But maybe I'm
being overparanoid.

--strk;

  ,--o-. 
  |   __/  |Delivering high quality PostGIS 2.0 !
  |  / 2.0 |http://strk.keybit.net - http://vizzuality.com
  `-o--'


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers


Re: [HACKERS] Interrupting long external library calls

2012-06-07 Thread Florian Pflug
On Jun7, 2012, at 12:08 , Sandro Santilli wrote:

 On Thu, Jun 07, 2012 at 12:00:09PM +0200, Florian Pflug wrote:
 On Jun7, 2012, at 10:20 , Sandro Santilli wrote:
 
 In that case I can understand Tom's advice about providing a callback,
 and then I would only need to perform the events flushing part of
 from within the callback, and only for windows.
 
 Why would you need a signal handler in the library at all, then? Just
 test the same flags that CHECK_FOR_INTERRUPTS does in the callback, and
 call your interrupt request method if they indicate abort. (Or, slightly
 cleaner maybe, allow the callback to abort processing by returning false)
 
 I'm just afraid that invoking a callback (from a library to user code)
 could be significantly slower than simply lookup a variable, especially
 if the interruption checking is performed very frequently. But maybe I'm
 being overparanoid.

It's definitely slower, probably at least an order of magnitude. But you
don't have to call that callback very often - once every few hundred
milliseconds is already more then plenty. Isn't there a place in the library
where you could put it where it'd be called with roughly that frequency?

IMHO the advantage of not messing with signal handling in the library
is more than worth the slight overhead of a callback, but YMMV.

best regards,
Florian Pflug


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers


Re: [HACKERS] Interrupting long external library calls

2012-06-06 Thread Sandro Santilli
FYI, I finally committed the code installing a signal handler in PostGIS,
using the pqsignal function:

  https://trac.osgeo.org/postgis/changeset/9850

It is currently only used to interrupt GEOS calls, but the idea is that
it could eventually also be used to interrupt other library calls having
a provision for such interruption request. GEOS itself only supports it
in the 3.4 branch.

In order to test it you'll need to define POSTGIS_ENABLE_GEOS_INTERRUPTIBILITY
at the top of postgis/postgis_module.c - the macro is off by default due
to concerns about possible consequences we may be unaware of.

Your comments will be very useful to reduce (or confirm) the concerns.
If we can get this confidently straight there may be the possibility to 
toggle the default to on before 2.0.1 ...

Thanks in advance.

PS: Tom, I still don't know what you mean by the SIGINFO case.

--strk;

On Mon, May 28, 2012 at 08:48:21AM +0200, Sandro Santilli wrote:
 On Fri, May 25, 2012 at 12:34:54PM -0400, Tom Lane wrote:
  Sandro Santilli s...@keybit.net writes:
   I ended up providing an explicit mechanism to request interruption of
   whatever the library is doing, and experimented (successfully so far)
   requesting the interruption from a SIGINT handler.
  
   Do you see any major drawback in doing so ?
  
  This seems a bit fragile.  It might work all right in Postgres, where
  we tend to set up signal handlers just once at process start, but ISTM
  other systems might assume they can change their signal handlers at
  any time.  The handler itself looks less than portable anyway ---
  what about the SIGINFO case?
 
 Indeed setting the handler from within the library itself was a temporary
 implementation to see how effective it would have been. The idea is to
 move the registration of the hanlder outside the library and into the
 user (PostGIS in this case). The library itself would only expose 
 GEOS_requestInterrupt/GEOS_cancelInterrupt API calls.
 
 I'm guessing PostGIS should use the more portable pqsignal functions ?
 
 What should I know about SIGINFO ?
 
  I assume that the geos::util::Interrupt::request() call sets a flag
  somewhere that's going to be periodically checked in long-running
  loops. 
 
 Yes, this is what will happen.
 
  Would it be possible for the periodic checks to include a
  provision for a callback into Postgres-specific glue code, wherein
  you could test the same flags CHECK_FOR_INTERRUPTS does?  A similar
  approach might then be usable in other contexts, and it seems safer
  to me than messing with a host environment's signal handling.
 
 Would it be enough for the signal handler (installed by PostGIS) 
 to check those flags and call the GEOS_requestInterrupt function
 when appropriate ?
 
 --strk; 

-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers


Re: [HACKERS] Interrupting long external library calls

2012-05-28 Thread Sandro Santilli
On Fri, May 25, 2012 at 12:34:54PM -0400, Tom Lane wrote:
 Sandro Santilli s...@keybit.net writes:
  I ended up providing an explicit mechanism to request interruption of
  whatever the library is doing, and experimented (successfully so far)
  requesting the interruption from a SIGINT handler.
 
  Do you see any major drawback in doing so ?
 
 This seems a bit fragile.  It might work all right in Postgres, where
 we tend to set up signal handlers just once at process start, but ISTM
 other systems might assume they can change their signal handlers at
 any time.  The handler itself looks less than portable anyway ---
 what about the SIGINFO case?

Indeed setting the handler from within the library itself was a temporary
implementation to see how effective it would have been. The idea is to
move the registration of the hanlder outside the library and into the
user (PostGIS in this case). The library itself would only expose 
GEOS_requestInterrupt/GEOS_cancelInterrupt API calls.

I'm guessing PostGIS should use the more portable pqsignal functions ?

What should I know about SIGINFO ?

 I assume that the geos::util::Interrupt::request() call sets a flag
 somewhere that's going to be periodically checked in long-running
 loops. 

Yes, this is what will happen.

 Would it be possible for the periodic checks to include a
 provision for a callback into Postgres-specific glue code, wherein
 you could test the same flags CHECK_FOR_INTERRUPTS does?  A similar
 approach might then be usable in other contexts, and it seems safer
 to me than messing with a host environment's signal handling.

Would it be enough for the signal handler (installed by PostGIS) 
to check those flags and call the GEOS_requestInterrupt function
when appropriate ?

--strk; 

  ,--o-. 
  |   __/  |Delivering high quality PostGIS 2.0 !
  |  / 2.0 |http://strk.keybit.net - http://vizzuality.com
  `-o--'


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers


Re: [HACKERS] Interrupting long external library calls

2012-05-26 Thread Simon Riggs
On 25 May 2012 17:34, Tom Lane t...@sss.pgh.pa.us wrote:
 Sandro Santilli s...@keybit.net writes:
 I ended up providing an explicit mechanism to request interruption of
 whatever the library is doing, and experimented (successfully so far)
 requesting the interruption from a SIGINT handler.

 Do you see any major drawback in doing so ?

 This seems a bit fragile.  It might work all right in Postgres, where
 we tend to set up signal handlers just once at process start, but ISTM
 other systems might assume they can change their signal handlers at
 any time.  The handler itself looks less than portable anyway ---
 what about the SIGINFO case?

 I assume that the geos::util::Interrupt::request() call sets a flag
 somewhere that's going to be periodically checked in long-running
 loops.  Would it be possible for the periodic checks to include a
 provision for a callback into Postgres-specific glue code, wherein
 you could test the same flags CHECK_FOR_INTERRUPTS does?  A similar
 approach might then be usable in other contexts, and it seems safer
 to me than messing with a host environment's signal handling.

Can we do that as a macro, e.g.

POSTGRES_LIBRARY_INTERRUPT_CHECK()

Otherwise the fix to this problem may be worse - faulty interrupt
handlers are worse than none at all.

-- 
 Simon Riggs   http://www.2ndQuadrant.com/
 PostgreSQL Development, 24x7 Support, Training  Services

-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers


Re: [HACKERS] Interrupting long external library calls

2012-05-26 Thread Florian Pflug
On May26, 2012, at 12:40 , Simon Riggs wrote:
 On 25 May 2012 17:34, Tom Lane t...@sss.pgh.pa.us wrote:
 I assume that the geos::util::Interrupt::request() call sets a flag
 somewhere that's going to be periodically checked in long-running
 loops.  Would it be possible for the periodic checks to include a
 provision for a callback into Postgres-specific glue code, wherein
 you could test the same flags CHECK_FOR_INTERRUPTS does?  A similar
 approach might then be usable in other contexts, and it seems safer
 to me than messing with a host environment's signal handling.
 
 Can we do that as a macro, e.g.
 
 POSTGRES_LIBRARY_INTERRUPT_CHECK()
 
 Otherwise the fix to this problem may be worse - faulty interrupt
 handlers are worse than none at all.

As it stands, ProcessInterrupts() sometimes returns instead of
ereport()ing even if InterruptPending is set, interrupts aren't
held off, and the code isn't in a critical section. That happens if
QueryCancelPending (or however that's called) gets reset after a
SIGINT arrived but before CHECK_FOR_INTERRUPTS() is called. Or at
least that is how I interpret the comment at the bottom of that
function...

We could thus easily provide POSTGRES_LIBRARY_INTERRUPT_CHECK() if
we're content with the (slim, probably) chance of false positives.

Or we'd need to refactor things in a way that allows the hypothetical
POSTGRES_LIBRARY_INTERRUPT_CHECK() to re-use the tests in
ProcessInterrupts(), but without modifying any of the flags.

best regards,
Florian Pflug


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers


Re: [HACKERS] Interrupting long external library calls

2012-05-25 Thread Sandro Santilli
On Thu, May 24, 2012 at 04:37:04PM +0200, Florian Pflug wrote:
 On May24, 2012, at 15:04 , Sandro Santilli wrote:
  On Wed, May 16, 2012 at 07:30:03PM +0300, Heikki Linnakangas wrote:
  On 16.05.2012 15:42, Sandro Santilli wrote:
  But CHECK_FOR_INTERRUPTS doesn't return, right ?
  Is there another macro for just checking w/out yet acting upon it ?
  
  Hmm, no. CHECK_FOR_INTERRUPTS() checks the InterruptPending
  variable, but on Windows it also checks for
  UNBLOCKED_SIGNAL_QUEUE(). And even if InterruptPending is set, it's
  not totally certain that CHECK_FOR_INTERRUPTS() won't return. I
  think InterruptPending can be set spuriously (even if that's not
  possible today, I wouldn't rely on it), and if you're in a
  HOLD/RESUME_INTERRUPTS block, CHECK_FOR_INTERRUPTS() will do nothing
  even if InterruptPending is true.
  
  The only sane way to make 3rd party code interruptible is to add
  CHECK_FOR_INTERRUPTS() to it, in safe places.
  
  No place is safe if CHECK_FOR_INTERRUPTS doesn't return.
  How could caller code cleanup on interruption ?
 
 The postgres way is to use PG_TRY/PG_CATCH to make sure stuff gets cleaned
 up if an error or an interrupts occurs. You could use those to make the
 third-party library exception safe, but it'll probably be a quite
 invasive change :-(.
 
 Alternatively, you could replicate the check CHECK_FOR_INTERRUPTS() does,

I ended up providing an explicit mechanism to request interruption of
whatever the library is doing, and experimented (successfully so far)
requesting the interruption from a SIGINT handler.

Do you see any major drawback in doing so ?

So far I installed the SIGINT handler within the library itself, but
I guess it could be moved out instead to have ore fine-grained control
over when to request interruption.

Here's the code installing the signal handler within the library:
https://github.com/strk/libgeos/commit/e820ecd0469b777953c132661877c2967b10cee2

--strk; 

  ,--o-. 
  |   __/  |Delivering high quality PostGIS 2.0 !
  |  / 2.0 |http://strk.keybit.net - http://vizzuality.com
  `-o--'


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers


Re: [HACKERS] Interrupting long external library calls

2012-05-25 Thread Tom Lane
Sandro Santilli s...@keybit.net writes:
 I ended up providing an explicit mechanism to request interruption of
 whatever the library is doing, and experimented (successfully so far)
 requesting the interruption from a SIGINT handler.

 Do you see any major drawback in doing so ?

This seems a bit fragile.  It might work all right in Postgres, where
we tend to set up signal handlers just once at process start, but ISTM
other systems might assume they can change their signal handlers at
any time.  The handler itself looks less than portable anyway ---
what about the SIGINFO case?

I assume that the geos::util::Interrupt::request() call sets a flag
somewhere that's going to be periodically checked in long-running
loops.  Would it be possible for the periodic checks to include a
provision for a callback into Postgres-specific glue code, wherein
you could test the same flags CHECK_FOR_INTERRUPTS does?  A similar
approach might then be usable in other contexts, and it seems safer
to me than messing with a host environment's signal handling.

regards, tom lane

-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers


Re: [HACKERS] Interrupting long external library calls

2012-05-24 Thread Sandro Santilli
On Wed, May 16, 2012 at 07:30:03PM +0300, Heikki Linnakangas wrote:
 On 16.05.2012 15:42, Sandro Santilli wrote:
 But CHECK_FOR_INTERRUPTS doesn't return, right ?
 Is there another macro for just checking w/out yet acting upon it ?
 
 Hmm, no. CHECK_FOR_INTERRUPTS() checks the InterruptPending
 variable, but on Windows it also checks for
 UNBLOCKED_SIGNAL_QUEUE(). And even if InterruptPending is set, it's
 not totally certain that CHECK_FOR_INTERRUPTS() won't return. I
 think InterruptPending can be set spuriously (even if that's not
 possible today, I wouldn't rely on it), and if you're in a
 HOLD/RESUME_INTERRUPTS block, CHECK_FOR_INTERRUPTS() will do nothing
 even if InterruptPending is true.
 
 The only sane way to make 3rd party code interruptible is to add
 CHECK_FOR_INTERRUPTS() to it, in safe places.

No place is safe if CHECK_FOR_INTERRUPTS doesn't return.
How could caller code cleanup on interruption ?

--strk; 

-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers


Re: [HACKERS] Interrupting long external library calls

2012-05-24 Thread Heikki Linnakangas

On 24.05.2012 16:04, Sandro Santilli wrote:

On Wed, May 16, 2012 at 07:30:03PM +0300, Heikki Linnakangas wrote:

On 16.05.2012 15:42, Sandro Santilli wrote:

But CHECK_FOR_INTERRUPTS doesn't return, right ?
Is there another macro for just checking w/out yet acting upon it ?


Hmm, no. CHECK_FOR_INTERRUPTS() checks the InterruptPending
variable, but on Windows it also checks for
UNBLOCKED_SIGNAL_QUEUE(). And even if InterruptPending is set, it's
not totally certain that CHECK_FOR_INTERRUPTS() won't return. I
think InterruptPending can be set spuriously (even if that's not
possible today, I wouldn't rely on it), and if you're in a
HOLD/RESUME_INTERRUPTS block, CHECK_FOR_INTERRUPTS() will do nothing
even if InterruptPending is true.

The only sane way to make 3rd party code interruptible is to add
CHECK_FOR_INTERRUPTS() to it, in safe places.


No place is safe if CHECK_FOR_INTERRUPTS doesn't return.
How could caller code cleanup on interruption ?


It would only be safe to call it in places where no cleanup is 
necessary. I don't know if there are any such places in the geos library.


--
  Heikki Linnakangas
  EnterpriseDB   http://www.enterprisedb.com

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers


Re: [HACKERS] Interrupting long external library calls

2012-05-24 Thread Sandro Santilli
On Thu, May 24, 2012 at 04:55:34PM +0300, Heikki Linnakangas wrote:
 On 24.05.2012 16:04, Sandro Santilli wrote:
 On Wed, May 16, 2012 at 07:30:03PM +0300, Heikki Linnakangas wrote:
 On 16.05.2012 15:42, Sandro Santilli wrote:
 But CHECK_FOR_INTERRUPTS doesn't return, right ?
 Is there another macro for just checking w/out yet acting upon it ?
 
 Hmm, no. CHECK_FOR_INTERRUPTS() checks the InterruptPending
 variable, but on Windows it also checks for
 UNBLOCKED_SIGNAL_QUEUE(). And even if InterruptPending is set, it's
 not totally certain that CHECK_FOR_INTERRUPTS() won't return. I
 think InterruptPending can be set spuriously (even if that's not
 possible today, I wouldn't rely on it), and if you're in a
 HOLD/RESUME_INTERRUPTS block, CHECK_FOR_INTERRUPTS() will do nothing
 even if InterruptPending is true.
 
 The only sane way to make 3rd party code interruptible is to add
 CHECK_FOR_INTERRUPTS() to it, in safe places.
 
 No place is safe if CHECK_FOR_INTERRUPTS doesn't return.
 How could caller code cleanup on interruption ?
 
 It would only be safe to call it in places where no cleanup is
 necessary. I don't know if there are any such places in the geos
 library.

Sure, right before starting and after finishing.
That is nowhere useful for a premature interruption...

The whole point of the work I'm doing these days is letting the
users interrupt GEOS calls which often take a long time and a lot
of memory resources.

The current plan is to provide custom allocators to have automatic
garbage collection on interruption. Turned out not to be an easy task
to bend GEOS into using palloc/pfree, but there's progress in that
direction.

--strk;

-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers


Re: [HACKERS] Interrupting long external library calls

2012-05-24 Thread Florian Pflug
On May24, 2012, at 15:04 , Sandro Santilli wrote:
 On Wed, May 16, 2012 at 07:30:03PM +0300, Heikki Linnakangas wrote:
 On 16.05.2012 15:42, Sandro Santilli wrote:
 But CHECK_FOR_INTERRUPTS doesn't return, right ?
 Is there another macro for just checking w/out yet acting upon it ?
 
 Hmm, no. CHECK_FOR_INTERRUPTS() checks the InterruptPending
 variable, but on Windows it also checks for
 UNBLOCKED_SIGNAL_QUEUE(). And even if InterruptPending is set, it's
 not totally certain that CHECK_FOR_INTERRUPTS() won't return. I
 think InterruptPending can be set spuriously (even if that's not
 possible today, I wouldn't rely on it), and if you're in a
 HOLD/RESUME_INTERRUPTS block, CHECK_FOR_INTERRUPTS() will do nothing
 even if InterruptPending is true.
 
 The only sane way to make 3rd party code interruptible is to add
 CHECK_FOR_INTERRUPTS() to it, in safe places.
 
 No place is safe if CHECK_FOR_INTERRUPTS doesn't return.
 How could caller code cleanup on interruption ?

The postgres way is to use PG_TRY/PG_CATCH to make sure stuff gets cleaned
up if an error or an interrupts occurs. You could use those to make the
third-party library exception safe, but it'll probably be a quite
invasive change :-(.

Alternatively, you could replicate the check CHECK_FOR_INTERRUPTS() does,
but then don't actually call ProcessInterrupts() but instead just make
the third-party code abort signalling an error, and call
CHECK_FOR_INTERRUPTS() after the third-party code has returned. On unix-like
systems that'd be a simple as aborting if InterruptPending is set, while
on windows you'd have to do the if(UNBLOCKED_SIGNAL_QUEUE())… stuff first,
since otherwise InterruptPending will never get set. The following function
should do the trick

  bool have_pending_interrupts() {
  #ifdef WIN32
if (UNBLOCKED_SIGNAL_QUEUE())
  pgwin32_dispatch_queued_signals();
  #endif

return InterruptPending  !InterruptHoldoffCount  !CritSectionCount;
  }

The third-party could would then do

  if (have_pending_interrupts())
return some_error;

and you'd invoke in with

  state = third_party_code();
  CHECK_FOR_INTERRUPTS();
  if (state != success)
ereport(…);

There might be slim chance for false positives with that approach, since
ProcessInterrupts() might not always ereport(), even if InterruptHoldoffCount
and CritSectionCount are zero. But they'll simply get turned into spurious
ereport() by the if(state != success) check after CHECK_FOR_INTERRUPTS, and
should be very rare, and happen only shortly after a query cancel request
was received.

best regards,
Florian Pflug


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers


Re: [HACKERS] Interrupting long external library calls

2012-05-16 Thread Heikki Linnakangas

On 16.05.2012 13:25, Mark Cave-Ayland wrote:

One of the issues we've been looking at with PostGIS is how to interrupt
long-running processing tasks in external libraries, particularly GEOS.
After a few tests here, it seems that even the existing SIGALRM handler
doesn't get called if statement_timeout is reached when in an external
library, e.g. with PostgreSQL 9.0/PostGIS 2.0 trunk/GEOS:


If you interrupt an external library call, it might leave memory in an 
inconsistent state, or some other such thing. It's not generally safe to 
interrupt arbitrary 3rd party code.


However, if you're absolutely positively sure that the library function 
can tolerate that, you can set ImmediateInterruptOK = true before 
calling it. See e.g PGSemaphoreLock() on how that's done before starting 
to sleep on a semapgore.


--
  Heikki Linnakangas
  EnterpriseDB   http://www.enterprisedb.com

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers


Re: [HACKERS] Interrupting long external library calls

2012-05-16 Thread Mark Cave-Ayland

On 16/05/12 11:39, Heikki Linnakangas wrote:


One of the issues we've been looking at with PostGIS is how to interrupt
long-running processing tasks in external libraries, particularly GEOS.
After a few tests here, it seems that even the existing SIGALRM handler
doesn't get called if statement_timeout is reached when in an external
library, e.g. with PostgreSQL 9.0/PostGIS 2.0 trunk/GEOS:


If you interrupt an external library call, it might leave memory in an
inconsistent state, or some other such thing. It's not generally safe to
interrupt arbitrary 3rd party code.

However, if you're absolutely positively sure that the library function
can tolerate that, you can set ImmediateInterruptOK = true before
calling it. See e.g PGSemaphoreLock() on how that's done before starting
to sleep on a semapgore.


Hi Heikki,

Yes that appears to work fine on the surface - just testing now to 
determine what state everything is left in when queries are aborted 
prematurely.



Many thanks,

Mark.

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers


Re: [HACKERS] Interrupting long external library calls

2012-05-16 Thread Heikki Linnakangas

On 16.05.2012 14:30, Mark Cave-Ayland wrote:

On 16/05/12 11:39, Heikki Linnakangas wrote:


However, if you're absolutely positively sure that the library function
can tolerate that, you can set ImmediateInterruptOK = true before
calling it. See e.g PGSemaphoreLock() on how that's done before starting
to sleep on a semapgore.


Hi Heikki,

Yes that appears to work fine on the surface - just testing now to
determine what state everything is left in when queries are aborted
prematurely.


You're unlikely to catch all problems just by testing. I wouldn't trust 
that it's safe unless the library authors specifically mentions that it 
is safe to longjump out of the function at any time. Note for example 
that if the library function calls back to palloc/pfree, then it's not 
safe, because interrupting those functions is not safe.


--
  Heikki Linnakangas
  EnterpriseDB   http://www.enterprisedb.com

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers


Re: [HACKERS] Interrupting long external library calls

2012-05-16 Thread Sandro Santilli
On Wed, May 16, 2012 at 02:46:17PM +0300, Heikki Linnakangas wrote:
 On 16.05.2012 14:30, Mark Cave-Ayland wrote:
 On 16/05/12 11:39, Heikki Linnakangas wrote:
 
 However, if you're absolutely positively sure that the library function
 can tolerate that, you can set ImmediateInterruptOK = true before
 calling it. See e.g PGSemaphoreLock() on how that's done before starting
 to sleep on a semapgore.
 
 Hi Heikki,
 
 Yes that appears to work fine on the surface - just testing now to
 determine what state everything is left in when queries are aborted
 prematurely.
 
 You're unlikely to catch all problems just by testing. I wouldn't
 trust that it's safe unless the library authors specifically
 mentions that it is safe to longjump out of the function at any
 time. Note for example that if the library function calls back to
 palloc/pfree, then it's not safe, because interrupting those
 functions is not safe.

I'm right now getting the external library into using memory allocator
wrappers so to provide an syntetized OOM condition in an aim to have a 
more predictable answer to that.

But CHECK_FOR_INTERRUPTS doesn't return, right ?
Is there another macro for just checking w/out yet acting upon it ?

--strk;

  ,--o-. 
  |   __/  |Delivering high quality PostGIS 2.0 !
  |  / 2.0 |http://strk.keybit.net - http://vizzuality.com
  `-o--'


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers


Re: [HACKERS] Interrupting long external library calls

2012-05-16 Thread Heikki Linnakangas

On 16.05.2012 15:42, Sandro Santilli wrote:

But CHECK_FOR_INTERRUPTS doesn't return, right ?
Is there another macro for just checking w/out yet acting upon it ?


Hmm, no. CHECK_FOR_INTERRUPTS() checks the InterruptPending variable, 
but on Windows it also checks for UNBLOCKED_SIGNAL_QUEUE(). And even if 
InterruptPending is set, it's not totally certain that 
CHECK_FOR_INTERRUPTS() won't return. I think InterruptPending can be set 
spuriously (even if that's not possible today, I wouldn't rely on it), 
and if you're in a HOLD/RESUME_INTERRUPTS block, CHECK_FOR_INTERRUPTS() 
will do nothing even if InterruptPending is true.


The only sane way to make 3rd party code interruptible is to add 
CHECK_FOR_INTERRUPTS() to it, in safe places.


--
  Heikki Linnakangas
  EnterpriseDB   http://www.enterprisedb.com

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers