Re: [PATCH 07/16] Log enough data into the wal to reconstruct logical changes from it if wal_level=logical

On 13.06.2012 14:28, Andres Freund wrote:
> @@ -2584,6 +2610,73 @@ l1:
>           rdata[1].buffer_std = true;
>           rdata[1].next = NULL;
>
> +        /*
> +         * XXX: We could decide not to log changes when the origin is not the
> +         * local node, that should reduce redundant logging.
> +         */
> +        if(need_tuple){
> +            xl_heap_header xlhdr;> ...
> +            relationFindPrimaryKey(relation,&indexoid,&pknratts, pkattnum, pktypoid, pkopclass);
> +
> +            if(!indexoid){
> +                elog(WARNING, "Could not find primary key for table with oid %u",
> +                     relation->rd_id);
> +                goto no_index_found;
> +            }
> +
> +            index_rel = index_open(indexoid, AccessShareLock);
> +
> +            indexdesc = RelationGetDescr(index_rel);
> +
> +            for(natt = 0; natt<  indexdesc->natts; natt++){
> +                idxvals[natt] =
> +                    fastgetattr(&tp, pkattnum[natt], desc,&idxisnull[natt]);
> +                Assert(!idxisnull[natt]);
> +            }
> +
> +            idxtuple = heap_form_tuple(indexdesc, idxvals, idxisnull);
> +
> +            xlhdr.t_infomask2 = idxtuple->t_data->t_infomask2;
> +            xlhdr.t_infomask = idxtuple->t_data->t_infomask;
> +            xlhdr.t_hoff = idxtuple->t_data->t_hoff;
> +
> +            rdata[1].next =&(rdata[2]);
> +            rdata[2].data = (char*)&xlhdr;
> +            rdata[2].len = SizeOfHeapHeader;
> +            rdata[2].buffer = InvalidBuffer;
> +            rdata[2].next = NULL;
> +
> +            rdata[2].next =&(rdata[3]);
> +            rdata[3].data = (char *) idxtuple->t_data + offsetof(HeapTupleHeaderData, t_bits);
> +            rdata[3].len = idxtuple->t_len - offsetof(HeapTupleHeaderData, t_bits);
> +            rdata[3].buffer = InvalidBuffer;
> +            rdata[3].next = NULL;
> +
> +            heap_close(index_rel, NoLock);
> +        no_index_found:
> +            ;
> +        }
> +
>           recptr = XLogInsert(RM_HEAP_ID, XLOG_HEAP_DELETE, rdata);
>
>           PageSetLSN(page, recptr);

It's not cool to do all that primary key lookup stuff within the 
critical section, while holding a lock on the page.

--   Heikki Linnakangas  EnterpriseDB   http://www.enterprisedb.com