Re: [PATCH 10/16] Introduce the concept that wal has a 'origin' node

Hi Simon,

On Monday, June 18, 2012 05:35:40 PM Simon Riggs wrote:
> On 13 June 2012 19:28, Andres Freund <andres@2ndquadrant.com> wrote:
> > This adds a new configuration parameter multimaster_node_id which
> > determines the id used for wal originating in one cluster.
> 
> Looks good and it seems this aspect at least is commitable in this CF.
I think we need to agree on the parameter name. It currently is 
'multimaster_node_id'. In the discussion with Steve we got to 
"replication_node_id". I don't particularly like either.

Other suggestions?

> Design decisions I think we need to review are
> 
> * Naming of field. I think origin is the right term, borrowing from Slony.
I think it fits as well.

> * Can we add the origin_id dynamically to each WAL record? Probably no
> need, but lets consider why and document that.
Not sure what you mean? Its already set in XLogInsert to 
current_replication_origin_id which defaults to the value of the guc?

> * Size of field. 16 bits is enough for 32,000 master nodes, which is
> quite a lot. Do we need that many? I think we may have need for a few
> flag bits, so I'd like to reserve at least 4 bits for flag bits, maybe
> 8 bits. Even if we don't need them in this release, I'd like to have
> them. If they remain unused after a few releases, we may choose to
> redeploy some of them as additional nodeids in future. I don't foresee
> complaints that 256 master nodes is too few anytime soon, so we can
> defer that decision.
I wished we had some flag bits available before as well. I find 256 nodes a 
pretty low value to start with though, 4096 sounds better though, so I would 
be happy with 4 flag bits. I think for cascading setups and such you want to 
add node ids for every node, not only masters...

Any opinions from others on this?

> * Do we want origin_id as a parameter or as a setting in pgcontrol?
> IIRC we go to a lot of trouble elsewhere to avoid problems with
> changing on/off parameter values. I think we need some discussion to
> validate where that should live.
Hm. I don't really forsee any need to have it in pg_control. What do you want 
to protect against with that?
It would need to be changeable anyway, because otherwise it would need to 
become a parameter for initdb which would suck for anybody migrating to use 
replication at some point.

Do you want to protect against problems in replication setups after changing 
the value?

> * Is there any overhead from CRC of WAL record because of this? I'd
> guess not, but just want to double check thinking.
I cannot imagine that there is. The actual size of the record didn't change 
because of alignment padding (both on 32 and 64 bit systems). 

> Presumably there is no issue wrt Heikki's WAL changes? I assume not,
> but ask since I know you're reviewing that also.
It might clash minimally because of offset changes or such, but otherwise 
there shouldn't be much.

Thanks,

Andres
-- Andres Freund                       http://www.2ndQuadrant.com/PostgreSQL Development, 24x7 Support, Training &
Services