On Fri, Jan 17, 2014 at 10:38 PM, Heikki Linnakangas < hlinnakan...@vmware.com> wrote:
> On 01/17/2014 01:05 PM, Alexander Korotkov wrote: > >> Seems to be fixed in the attached version of patch. >> Another improvement in this version: only left-most segments and further >> are re-encoded. Left part of page are left untouched. >> > > I'm looking into this now. A few quick notes: > > * Even when you don't re-encode the whole page, you still WAL-log the > whole page. While correct, it'd be a pretty obvious optimization to only > WAL-log the modified part. > Oh, I overlooked it. I wrote code in ginRedoInsertData which finds appropriate place fro data but didn't notice that I still wrote whole page to xlog record. > * When new items are appended to the end of the page so that only the last > existing compressed segment is re-encoded, and the page has to be split, > the items aren't divided 50/50 on the pages. The loop that moves segments > destined for the left page to the right won't move any existing, untouched, > segments. > I think this loop will move one segment. However, it's too small. > ! /* >> ! * Didn't fit uncompressed. We'll have to encode them. Check if >> both >> ! * new items and uncompressed items can be placed starting from >> last >> ! * segment of page. Then re-encode only last segment of page. >> ! */ >> ! minNewItem = newItems[0]; >> ! if (nolduncompressed == 0 && >> ! ginCompareItemPointers(&olduncompressed[0], >> &minNewItem) < 0) >> ! minNewItem = olduncompressed[0]; >> > > That looks wrong. If I'm understanding it right, it's trying to do > minNewItem = Min(newItems[0], olduncompressed[0]). The test should be > "nolduncompressed > 0 && ..." > Yes, definitely a bug. > No need to send a new patch, I'll just fix those while reviewing... Ok, thanks! ------ With best regards, Alexander Korotkov.
