Re: use less space in xl_xact_commit patch - Mailing list pgsql-hackers

On Wed, May 25, 2011 at 3:41 PM, Simon Riggs <simon@2ndquadrant.com> wrote:
> On Wed, May 25, 2011 at 2:43 PM, Leonardo Francalanci <m_lists@yahoo.it> wrote:
>>> Da: Simon Riggs <simon@2ndQuadrant.com>
>>> I can't find a clear  discussion of what you are trying to do, and how,
>>> just a URL back to a  complex discussion on another topic.
>>
>>
>> While trying to write a patch to allow changing an unlogged table into
>> a logged one, I had to add another int field to xl_xact_commit.
>> Robert Haas said:
>>
>>  "I have to admit I don't like this approach very much.  I can't see
>> adding 4 bytes to every commit record for this feature."
>>
>>
>> which is a correct remark.
>>
>> xl_xact_commit can contain some arrays (relation to drops,
>> committed sub-trans, shared invalidation msgs). The length of
>> these arrays is specified using 3 ints in the struct.
>>
>> So, to avoid adding more ints to the struct, I've been suggested to
>> remove all the ints, and use   xl_xact_commit.xinfo to flag which
>> arrays are, in fact, present.
>>
>> So the whole idea is:
>>
>> - remove nrels, nsubxacts and nmsgs from xl_xact_commit
>> - use bits in xinfo to signal which arrays are present at the end
>> of   xl_xact_commit
>> - for each present array, add the length of the array (as int) at
>> the end of    xl_xact_commit
>> - add each present array after all the lengths
>
>
> OK, thats clear. Thanks.
>
> That formatting sounds quite complex.
>
> I would propose we split this into 2 WAL records: xl_xact_commit and
> xl_xact_commit_with_info
>
> xl_xact_commit doesn't have any flags, counts or arrays.
>
> xl_xact_commit_with_info always has all 3 counts, even if zero.
> Arrays follow the main record
>
> I think it might also be possible to removed dbId and tsId from
> xl_act_commit if we use that definition.

Yes, that's correct. We can remove them from the normal commit record
when nmsgs == 0.

I think we can get away with these 2 definitions, but pls check. Using
static definitions works better for me because we can see what they
contain, rather than having the info flags imply that the record can
contain any permutation of settings when that's not really possible.
 {     TimestampTz xact_time;        /* time of commit */    int            nsubxacts;        /* number of
subtransactionXIDs */    /* ARRAY OF COMMITTED SUBTRANSACTION XIDs FOLLOWS */ } xl_xact_commit; 
 {     TimestampTz xact_time;        /* time of commit */     uint32        xinfo;            /* info flags */    int
        nrels;            /* number of RelFileNodes */    int            nsubxacts;        /* number of subtransaction
XIDs*/    int            nmsgs;        /* number of shared inval msgs */     Oid            dbId;            /*
MyDatabaseId*/     Oid            tsId;            /* MyDatabaseTableSpace */    /* Array of RelFileNode(s) to drop at
commit*/    RelFileNode xnodes[1];        /* VARIABLE LENGTH ARRAY */    /* ARRAY OF COMMITTED SUBTRANSACTION XIDs
FOLLOWS*/    /* ARRAY OF SHARED INVALIDATION MESSAGES FOLLOWS */ } xl_xact_commit_with_info; 

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