HOOKS for Synchronous Replication

Hi,

The implementation of a set of hooks for efficient synchronous replication
without extensive patching of Postgresql source is now available at:

http://gorda.di.uminho.pt/community/

Note that this is far from a full replication product. It is made available
for early feedback by the Postgresql hacker community and because it is
probably already useful for other projects (e.g. materialized views).

This is the first self-contained component extracted from a working prototype
of replication based on group communication under development by the GORDA
project.

Comments and suggestions welcome,

Regards Alfranio Junior.

On Mon, Jun 20, 2005 at 11:03:45AM +0100, Alfranio Correia Junior wrote:

Alfranio,

> The implementation of a set of hooks for efficient synchronous replication
> without extensive patching of Postgresql source is now available at:
>
> http://gorda.di.uminho.pt/community/

Some random coments, from general eyeballing the patch:

First of all your patch does not conform with the project style. Please
have a look at how other files are indented, in particular regarding
brace position and ereport() arguments (any function arguments really,
but in ereport your problems are more visible). Also, always use
ereport() for user messages, elog() for conditions that involve
can't-happen situations (server bugs, like not finding a tuple in a
catalog that should be there, etc).

Also there are some changes that you certainly don't want committed.
For example why are you removing the TransState from xact.c?

Also, consider using the XactCallback mechanism instead of inventing
your own.

On a different front, have you considered talking to the people behind
Slony-II to see if they'd have some use for your hooks?

--
Alvaro Herrera (<alvherre[a]surnet.cl>)
Thou shalt check the array bounds of all strings (indeed, all arrays), for
surely where thou typest "foo" someone someday shall type
"supercalifragilisticexpialidocious" (5th Commandment for C programmers)

On Mon, 20 Jun 2005, Alvaro Herrera wrote:

> On Mon, Jun 20, 2005 at 11:03:45AM +0100, Alfranio Correia Junior wrote:
>
> Alfranio,
>
>> The implementation of a set of hooks for efficient synchronous replication
>> without extensive patching of Postgresql source is now available at:
>>
>> http://gorda.di.uminho.pt/community/
>
> ...
>
> Also there are some changes that you certainly don't want committed.
> For example why are you removing the TransState from xact.c?

That puzzled me too, until I noticed that it's not removed, just moved to
xact.h. The current transaction state is exposed in a new
getCurrentTransactionState function, that's why TransState has to be in a
header file...

I just took a quick glance, I can't say why it's that way.

- Heikki

Thank you for the comments.

>First of all your patch does not conform with the project style. Please
>have a look at how other files are indented, in particular regarding
>brace position and ereport() arguments (any function arguments really,
>but in ereport your problems are more visible). Also, always use
>ereport() for user messages, elog() for conditions that involve
>can't-happen situations (server bugs, like not finding a tuple in a
>catalog that should be there, etc).
>
>
OK !!! I am going to correct these problems.

>Also there are some changes that you certainly don't want committed.
>For example why are you removing the TransState from xact.c?
>
>
I simply moved it to the xact.h...
Basically, it was done to be able to access the current transaction
state from other files.
Thus, I can avoid to rebuild the structure that stores the global
triggers in the middle of a transaction.
However, the lack of this functionality it is not critical since the
superuser is the only
person that can change such triggers.

>Also, consider using the XactCallback mechanism instead of inventing
>your own.
>
>
>
Unfortunately, the XactCallback is called after "commit" which
means that a transaction cannot be rolled back.
For synchronous replication, it is necessary to have a before commit event.
Moreover, the addition of a callback function implies modifications to
the source code, am I wrong ?
These global triggers could be easily adapted to call functions "before"
and "after commit" without
changing the code.

>On a different front, have you considered talking to the people behind
>Slony-II to see if they'd have some use for your hooks?
>
>
>
I would like to share the hooks with them and other things that I have
been done.

Best regards,

Alfranio Junior.

>First of all your patch does not conform with the project style. Please
>have a look at how other files are indented, in particular regarding
>brace position and ereport() arguments (any function arguments really,
>but in ereport your problems are more visible). Also, always use
>ereport() for user messages, elog() for conditions that involve
>can't-happen situations (server bugs, like not finding a tuple in a
>catalog that should be there, etc).
>
>
I think it is ok now.
However, I corrected the indentation manually.
I could not run some of the tools, namely the "entab".

/usr/lib/gcc-lib/i386-redhat-linux/3.3.3/include/varargs.h:4:2: #error
"GCC no longer implements <varargs.h>."
/usr/lib/gcc-lib/i386-redhat-linux/3.3.3/include/varargs.h:5:2: #error
"Revise your code to use <stdarg.h>."
halt.c:23: error: syntax error before "va_dcl"
halt.c:24: error: syntax error before '{' token
----
Linux alfranio.lsd.di.uminho.pt 2.6.8-1.521 #1 Mon Aug 16 09:01:18 EDT
2004 i686 i686 i386 GNU/Linux
(GCC) 3.3.3 20040412 (Red Hat Linux 3.3.3-7)

Do you have any idea ?

Best regards,

Alfranio Junior.

Alfranio Correia Junior wrote:
> I think it is ok now.
> However, I corrected the indentation manually.
> I could not run some of the tools, namely the "entab".
>
> /usr/lib/gcc-lib/i386-redhat-linux/3.3.3/include/varargs.h:4:2: #error
> "GCC no longer implements <varargs.h>."
> /usr/lib/gcc-lib/i386-redhat-linux/3.3.3/include/varargs.h:5:2: #error
> "Revise your code to use <stdarg.h>."
> halt.c:23: error: syntax error before "va_dcl"
> halt.c:24: error: syntax error before '{' token

I believe this should be fixed in CVS HEAD.

-Neil