Andreas Pflug wrote:

> Christopher Browne wrote:
>
>> Listed below is a context diff for a patch which I intend to apply to
>> the 1.1 branch next week in order to generate a 1.1.1 release.
>
I'm not certain that I can agree with either.

> 4. sl_confirm.con_timestamp default should be fixed
> http://gborg.postgresql.org/pipermail/slony1-general/2005-August/002793.html
>

The proposed patch wouldn't do anything to fix the perceived problem
because the timestamp is assigned as CURRENT_TIME in function
query_append_event() in src/slon/remote_worker.c

                     "insert into %s.sl_confirm "
                     "    (con_origin, con_received, con_seqno,
con_timestamp) "
                     "   values (%d, %d, '%s', CURRENT_TIMESTAMP); ",

That is where the change would have to be applied.

That is actually a pretty safe place to put now(), so I'll do that...

> 5. rebuildListenEntries for enabled nodes only
> http://gborg.postgresql.org/pipermail/slony1-general/2005-September/002909.html
>

I have a concern about this in that having extra nodes around could lead
to the cleanup thread deciding it can NEVER purge out old data for
enabled nodes if any activity ever hits one of the unenabled nodes.

I am VERY reluctant to do this as a 1.1 change because I would prefer to
see it heavily tested before deployment, and the upcoming 1.1 changes
won't be seeing the full redo-all-platforms-from-scratch testing that
1.2 will.

In any case, the use of "virtual nodes" strikes me as being a
questionable way to do things.

You're using the "virtual nodes" to store the "admin conninfo"
information, right?

If so, then it seems to me that a better way to handle this is NOT to
create "virtual nodes," but rather to create a new table, sl_adminconn,
which stores this information and makes it unnecessary to have "fake nodes."

I'd much rather add a new table (ideally with foreign key constraints
and such) which are invisible to slon than have to add logic to ignore
the "virtual nodes."