Postgres Pro
Downloads
Home   >   mailing lists

Re: replication slot restart_lsn initialization - Mailing list pgsql-hackers

From Gurjeet Singh
Subject Re: replication slot restart_lsn initialization
Date
Msg-id CABwTF4Wh_dBCzTU=49pFXR6coR4NW1ynb+vBqT+Po=7fuq5iCw@mail.gmail.com
Whole thread Raw
In response to Re: replication slot restart_lsn initialization  (Andres Freund <andres@anarazel.de>)
Responses Re: replication slot restart_lsn initialization  (Andres Freund <andres@anarazel.de>)
Re: replication slot restart_lsn initialization  (Andres Freund <andres@anarazel.de>)
List pgsql-hackers
Tree view
On Tue, Jul 7, 2015 at 6:49 AM, Andres Freund <andres@anarazel.de> wrote:
On 2015-07-07 06:41:55 -0700, Gurjeet Singh wrote:
> There seems to be a misplaced not operator  ! in that if statement, as
> well. That sucks :( The MacOS gcc binary is actually clang, and its output
> is too noisy [1], which is probably why I might have missed warning if any.

Which version of clang is it? With newer ones you can individually
disable nearly all of the warnings. I regularly use clang, and most of
the time it compiles master without warnings.
 
$ gcc --version
Configured with: --prefix=/Applications/Xcode.app/Contents/Developer/usr --with-gxx-include-dir=/usr/include/c++/4.2.1
Apple LLVM version 6.1.0 (clang-602.0.53) (based on LLVM 3.6.0svn)
Target: x86_64-apple-darwin14.4.0
Thread model: posix

I see that -Wno-parentheses-equality might help, but I am not looking forward to maintaining OS specific flags for clang-that-pretends-to-be-gcc in my shell scripts [2]


Attached is a patch that introduces SlotIsPhyscial/SlotIsLogical macros. This patch, if accepted, goes on top of the v4 patch.
 
> > >  pg_create_physical_replication_slot(PG_FUNCTION_ARGS)
> > >  {
> > >       Name            name = PG_GETARG_NAME(0);
> > > +     bool            activate = PG_GETARG_BOOL(1);
> >
> > Don't like 'activate' much as a name. How about 'immediately_reserve'?
> >
>
> I still like 'activate, but okay. How about 'reserve_immediately' instead?

Activate is just utterly wrong. A slot isn't inactive before. And
'active' already is used for slots that are currently in use
(i.e. connected to).

> Also, do you want this name change just in the C code, or in the pg_proc
> and docs as well?

All.

Patch v4 attached.

On a side note, I see that the pg_create_*_replication_slot() functions do not behave transactionally; that is, rolling back a transaction does not undo the slot creation. Should we prevent these, and corresponding drop functions, from being called inside an explicit transaction? PreventTransactionChain() is geared towards serving just the utility commands. Do we protect against calling such functions in a transaction block, or from user functions? How?

Best regards,
--
Gurjeet Singh http://gurjeet.singh.im/
Attachment

pgsql-hackers by date:

Previous
From: Heikki Linnakangas
Date:
Subject: Re: Improving test coverage of extensions with pg_dump
Next
From: Bruce Momjian
Date:
Subject: Re: Missing latex-longtable value