Re: GIN improvements part 1: additional information

On Thu, Nov 14, 2013 at 3:00 PM, Alexander Korotkov <aekorotkov(at)gmail(dot)com>wrote:

> On Thu, Nov 14, 2013 at 2:17 PM, Alexander Korotkov <aekorotkov(at)gmail(dot)com>wrote:
>
>> On Tue, Nov 5, 2013 at 9:49 PM, Heikki Linnakangas <
>> hlinnakangas(at)vmware(dot)com> wrote:
>>
>>> On 04.11.2013 23:44, Alexander Korotkov wrote:
>>>
>>>> On Mon, Oct 21, 2013 at 11:12 PM, Alexander Korotkov
>>>> <aekorotkov(at)gmail(dot)com>wrote:
>>>>
>>>> Attached version of patch is debugged. I would like to note that
>>>>> number of
>>>>> bugs was low and it wasn't very hard to debug. I've rerun tests on it.
>>>>> You
>>>>> can see that numbers are improved as the result of your refactoring.
>>>>>
>>>>> event | period
>>>>> -----------------------+-----------------
>>>>> index_build | 00:01:45.416822
>>>>> index_build_recovery | 00:00:06
>>>>> index_update | 00:05:17.263606
>>>>> index_update_recovery | 00:01:22
>>>>> search_new | 00:24:07.706482
>>>>> search_updated | 00:26:25.364494
>>>>> (6 rows)
>>>>>
>>>>> label | blocks_mark
>>>>> ----------------+-------------
>>>>> search_new | 847587636
>>>>> search_updated | 881210486
>>>>> (2 rows)
>>>>>
>>>>> label | size
>>>>> ---------------+-----------
>>>>> new | 419299328
>>>>> after_updates | 715915264
>>>>> (2 rows)
>>>>>
>>>>> Beside simple bugs, there was a subtle bug in incremental item indexes
>>>>> update. I've added some more comments including ASCII picture about how
>>>>> item indexes are shifted.
>>>>>
>>>>> Now, I'm trying to implement support of old page format. Then we can
>>>>> decide which approach to use.
>>>>>
>>>>>
>>>> Attached version of patch has support of old page format. Patch still
>>>> needs
>>>> more documentation and probably refactoring, but I believe idea is clear
>>>> and it can be reviewed. In the patch I have to revert change of null
>>>> category placement for compatibility with old posting list format.
>>>>
>>>
>>> Thanks, just glanced at this quickly.
>>>
>>> If I'm reading the patch correctly, old-style non-compressed posting
>>> tree leaf pages are not vacuumed at all; that's clearly not right.
>>>
>>
>> Fixed. Now separate function handles uncompressed posting lists and
>> compress them if as least one TID is deleted.
>>
>>
>>> I argued earlier that we don't need to handle the case that compressing
>>> a page makes it larger, so that the existing items don't fit on the page
>>> anymore. I'm having some second thoughts on that; I didn't take into
>>> account the fact that the "mini-index" in the new page format takes up some
>>> space. I think it's still highly unlikely that there isn't enough space on
>>> a 8k page, but it's not totally crazy with a non-standard small page size.
>>> So at least that situation needs to be checked for with an ereport(),
>>> rather than Assert.
>>>
>>
>> Now this situation is ereported before any change in page.
>>
>> To handle splitting a non-compressed page, it seems that you're relying
>>> on the fact that before splitting, we try to insert, which compresses the
>>> page. The problem with that is that it's not correctly WAL-logged. The
>>> split record that follows includes a full copy of both page halves, but if
>>> the split fails for some reason, e.g you run out of disk space, there is no
>>> WAL record at all of the the compression. I'd suggest doing the compression
>>> in the insert phase on a temporary copy of the page, leaving the original
>>> page untouched if there isn't enough space for the insertion to complete.
>>> (You could argue that this case can't happen because the compression must
>>> always create enough space to insert one more item. maybe so, but at least
>>> there should be an explicit check for that.)
>>
>>
>> Good point. Yes, if we don't handle specially inserting item indexes, I
>> see no point to do special handling for single TID which is much smaller.
>> In the attached patch dataCompressLeafPage just reserves space for single
>> TID.
>>
>> Also, many changes in comments and README.
>>
>> Unfortunally, I didn't understand what is FIXME about in
>> ginVacuumEntryPage. So, it's not fixed :)
>>
>
> Sorry, I posted buggy version of patch. Attached version is correct.
>

Another bug fix by report from Rod Taylor.

------
With best regards,
Alexander Korotkov.
>
>