Re: [HACKERS] Need some help on code - Mailing list pgsql-hackers

From dg@illustra.com (David Gould)
Subject Re: [HACKERS] Need some help on code
Date
Msg-id 9806082212.AA11383@hawk.illustra.com
Whole thread Raw
In response to Need some help on code  (Maarten Boekhold <maartenb@dutepp0.et.tudelft.nl>)
Responses Re: [HACKERS] Need some help on code  (Maarten Boekhold <maartenb@dutepp0.et.tudelft.nl>)
List pgsql-hackers
Maarten wrote:

> I was trying to change to cluster command to do the its writes clustered
> by a 100 tuples, thus hoping to improve performance. However, the code
> I've written crashes. This has certainly to do with some internal states
> of pgsql that aren't preserved in a HeapTuple.
>
> Could somebody with knowledge have a brief glimpse on my code and perhaps
> tell me how to do it properly?
...
> static void
> rebuildheap(Oid OIDNewHeap, Oid OIDOldHeap, Oid OIDOldIndex)
> {
>     Relation    LocalNewHeap,
>                 LocalOldHeap,
>                 LocalOldIndex;
>     IndexScanDesc ScanDesc;
>     RetrieveIndexResult ScanResult;
>     ItemPointer HeapTid;
>     HeapTuple    LocalHeapTuple;
>     Buffer        LocalBuffer[100];
>     Oid            OIDNewHeapInsert;
>     Dllist      *ScanResList;
>     Dlelem      *ListEl;
>     int         count, loop;
>
>     /*
>      * Open the relations I need. Scan through the OldHeap on the OldIndex
>      * and insert each tuple into the NewHeap.
>      */
>     LocalNewHeap = (Relation) heap_open(OIDNewHeap);
>     LocalOldHeap = (Relation) heap_open(OIDOldHeap);
>     LocalOldIndex = (Relation) index_open(OIDOldIndex);
>     ScanResList = DLNewList();
>
>     ScanDesc = index_beginscan(LocalOldIndex, false, 0, (ScanKey) NULL);
>
>     loop = 1;
>     while (loop) {
>         count = 0;
>         while ((count < 100) &&
>                ((ScanResult =
>                 index_getnext(ScanDesc,
>                     ForwardScanDirection)) != NULL))
>         {
>
>             HeapTid = &ScanResult->heap_iptr;
>             pfree(ScanResult);
                        ^^^^^^^^^^^^^^^^^^
Hmmm, at this point, HeapTid is a pointer to what?

>             LocalHeapTuple = heap_fetch(LocalOldHeap, false,
>                     HeapTid, &LocalBuffer[count]);

Given more than one tuple on a page, then there may exist some
 LocalBuffer[i] == LocalBuffer[j] where i and j are distinct values of count.

>             ListEl = DLNewElem(LocalHeapTuple);
>             DLAddTail(ScanResList, ListEl);
>             count++;
>         }
>
>         if (count < 100) loop = 0;
>
>         count = 0;
>         while ((ListEl = DLRemHead(ScanResList)) != NULL) {
>             LocalHeapTuple = (HeapTuple)ListEl->dle_val;
>             DLFreeElem(ListEl);
>             OIDNewHeapInsert =
>                 heap_insert(LocalNewHeap, LocalHeapTuple);
>             ReleaseBuffer(LocalBuffer[count]);

So here we ReleaseBuffer(LocalBuffer[count]) which if there are more than
one LocalBuffer[] that are in fact the same buffer will release the buffer
multiple times.

>             count++;
>         }
>     }
>
>     index_endscan(ScanDesc);
>
>     index_close(LocalOldIndex);
>     heap_close(LocalOldHeap);
>     heap_close(LocalNewHeap);
>     DLFreeList(ScanResList);
> }


Hope this helps.
-dg

David Gould            dg@illustra.com           510.628.3783 or 510.305.9468
Informix Software  (No, really)         300 Lakeside Drive  Oakland, CA 94612
"Don't worry about people stealing your ideas.  If your ideas are any
 good, you'll have to ram them down people's throats." -- Howard Aiken

pgsql-hackers by date:

Previous
From: Bruce Momjian
Date:
Subject: Re: [HACKERS] Re: Cancel key now ready
Next
From: Maarten Boekhold
Date:
Subject: Re: [HACKERS] Need some help on code