Christopher Browne wrote:

> 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...

AFAIR I got an SQL exception from a pl function that triggered the 
default when enabling a node, so your fix doesn't replace mine, but 
fixes another occurrence.


> 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.

Ignoring non-enabled nodes seems the correct handling for all activities 
in general. Non-enabled nodes won't create events or consume events, so 
there's no need for listens to them (independent of my "misuse" of 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?

Correct, as path. Actually, the admin node *is* a node, simply not a 
slon node but a controlling participant in the slony replication setup.

> 
> 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."

It needs replication as well. With 1.0 everything was fine, only 1.1 
auto-listen creation broke it.

> 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."

We want to support 1.0 and up. Ignoring disabled nodes would accomplish 
this. A new table would mean we can support 1.0 and 1.2, but not 1.1.

Regards,
Andreas