Re: Proposal to add a QNX 6.5 port to PostgreSQL

Hello Andres,

Thanks for your response.

About SA_RESTART:
------------------------
I would like to offer you a different perspective which may alter your current opinion.
I believe the port.h QNX macro replacement for SA_RESTART is still a reasonable solution on QNX for these reasons:

First, I think it is better to adapt PostgreSQL to suit the platform than to adapt the platform to suit PostgreSQL.
Changing default behavior of libc on QNX to suit PostgreSQL may break other applications which rely on the current behavior of libc.

Yes, I could forget to add a port.h macro for a given interruptible primitive, but I could likewise forget to update the wrapper for that call in a custom libc.
I requested that QNX support provide me a list of interruptible primitives, but I was able to identify many by searching through the QNX help.
Definition of a new interruptible primitive is a rare event, so once a solid list of macros is in place for QNX, it should need very little maintenance.
If you have any specific calls you believe are missing from my list of macros, I would be happy to add them.

port.h is included in c.h, which is in postgres.h, so the QNX macros should be effective for all QNX PostgreSQL compiles.
If it were not, no one could reply on any port.h features on any platform.

Testing so far has demonstrated that the macro fixes are effective on QNX. Repeated runs of the regression tests run cleanly.
More testing will be required to boost the confidence and expose any gaps, but the foundation appears to be solid.

The first release on any platform has risk of defects, which can be corrected once identified.
I would expect that a first release on any platform would include a warning or disclaimer stating that it is new port.

Lastly, the QNX-specific section added to port.h appears to solve the SA_RESTART issue for QNX, while having no impact on compiles of existing platforms.

About configure:
--------------------
"./configure" barked at 2 things on QNX, and it advised using both "--without-readline --disable-thread-safety".
I can investigate further, but I have been focusing on the bigger issues first.

I hope the explanations above address your main concerns.
Again, thanks for your response!

Keith Baker

> -----Original Message-----
> From: Andres Freund [mailto:andres(at)2ndquadrant(dot)com]
> Sent: Wednesday, August 20, 2014 7:25 PM
> To: Baker, Keith [OCDUS Non-J&J]
> Cc: Alvaro Herrera; Robert Haas; Tom Lane; pgsql-hackers(at)postgresql(dot)org
> Subject: Re: [HACKERS] Proposal to add a QNX 6.5 port to PostgreSQL
>
> Hi,
>
> On 2014-08-20 21:21:41 +0000, Baker, Keith [OCDUS Non-J&J] wrote:
> > To work around lack of SA_RESTART, I added QNX-specific retry macros
> > to port.h With these macros in place "make check" runs cleanly (fails in
> many place without them).
> >
> > +#if defined(__QNX__)
> > +/* QNX does not support sigaction SA_RESTART. We must retry
> interrupted calls (EINTR) */
> > +/* Helper macros, used to build our retry macros */
> > +#define PG_RETRY_EINTR3(exp,val,type) ({ type _tmp_rc; do _tmp_rc =
> (exp); while (_tmp_rc == (val) && errno == EINTR); _tmp_rc; })
> > +#define PG_RETRY_EINTR(exp) PG_RETRY_EINTR3(exp,-1L,long int)
> > +#define PG_RETRY_EINTR_FILE(exp) PG_RETRY_EINTR3(exp,NULL,FILE
> *)
> > +/* override calls known to return EINTR when interrupted */
> > +#define close(a) PG_RETRY_EINTR(close(a))
> > +#define fclose(a) PG_RETRY_EINTR(fclose(a))
> > +#define fdopen(a,b) PG_RETRY_EINTR_FILE(fdopen(a,b))
> > +#define fopen(a,b) PG_RETRY_EINTR_FILE(fopen(a,b))
> > +#define freopen(a,b,c) PG_RETRY_EINTR_FILE(freopen(a,b,c))
> > +#define fseek(a,b,c) PG_RETRY_EINTR(fseek(a,b,c))
> > +#define fseeko(a,b,c) PG_RETRY_EINTR(fseeko(a,b,c))
> > +#define ftruncate(a,b) PG_RETRY_EINTR(ftruncate(a,b))
> > +#define lseek(a,b,c) PG_RETRY_EINTR(lseek(a,b,c))
> > +#define open(a,b,...) ({ int _tmp_rc; do _tmp_rc =
> open(a,b,##__VA_ARGS__); while (_tmp_rc == (-1) && errno == EINTR);
> _tmp_rc; })
> > +#define shm_open(a,b,c) PG_RETRY_EINTR(shm_open(a,b,c))
> > +#define stat(a,b) PG_RETRY_EINTR(stat(a,b))
> > +#define unlink(a) PG_RETRY_EINTR(unlink(a))
> > ... (Macros for read and write are similar but slightly longer, so I omit
> them here)...
> > +#endif /* __QNX__ */
>
> I think this is a horrible way to go and unlikely to succeed. You're surely going
> to miss calls and it's going to need to be maintained continuously. We'll miss
> adding things which will then only break under load. Which most poeple
> won't be able to generate under qnx.
>
> The only reasonably way to fake kernel SA_RESTART support is doing so is in
> $platform's libc. In the syscall wrapper.
>
> > Here is what I used for configure, I am open to suggestions:
> > ./configure --without-readline --disable-thread-safety
>
> Why is the --disable-thread-safety needed?
>
> Greetings,
>
> Andres Freund
>
> --
> Andres Freund http://www.2ndQuadrant.com/
> PostgreSQL Development, 24x7 Support, Training & Services