pg.ddx.io  pgsql-hackers@postgresql.org mailing list archive  
help / color / mirror / Atom feed
From: Heikki Linnakangas <hlinnakangas@vmware.com>
To: Alexander Korotkov <aekorotkov@gmail.com>
Cc: Tomas Vondra <tv@fuzzy.cz>
Cc: pgsql-hackers@postgresql.org
Subject: Re: GIN improvements part 1: additional information
Date: Thu, 10 Oct 2013 14:57:41 +0300
Message-ID: <52569635.5090808@vmware.com> (raw)
In-Reply-To: <52548F79.6030407@fuzzy.cz>
References: <CAPpHfduxv-iL7aedwPW0W5fXrWGAKfxijWM63_hZujaCRxnmFQ@mail.gmail.com>
	<CAPpHfduQrtC4B+MC9_z3QB8oJsK1Ws-H8ivEsy_cmDnfoJqkeQ@mail.gmail.com>
	<CAPpHfdszEJhSynTPX6=tB3aKq1EA1JXpwx4xiFYM097K7PhQLA@mail.gmail.com>
	<CAPpHfdu8=5spaS7c9mxwH7bNMDR6n378fZCv6c2Y3YM7iE5KiQ@mail.gmail.com>
	<51CEA13C.8040103@vmware.com>
	<CAPpHfdutJEC9+6vWX579afZKzdHKCT2wzDkAEom1XmiXEwF3pw@mail.gmail.com>
	<524E7C3B.7060402@vmware.com>
	<5250A7A1.2010701@fuzzy.cz>
	<CAPpHfdu5ODt1UB5SSpMHHD6LCB+k_t6XCsLBozZP_dyVm1ieLQ@mail.gmail.com>
	<52546434.1000303@vmware.com>
	<52548F79.6030407@fuzzy.cz>
List-Unsubscribe: <mailto:majordomo@postgresql.org?body=unsub%20pgsql-hackers>

On 09.10.2013 02:04, Tomas Vondra wrote:
> On 8.10.2013 21:59, Heikki Linnakangas wrote:
>> On 08.10.2013 17:47, Alexander Korotkov wrote:
>>> Hi, Tomas!
>>>
>>> On Sun, Oct 6, 2013 at 3:58 AM, Tomas Vondra<tv@fuzzy.cz>   wrote:
>>>
>>>> I've attempted to rerun the benchmarks tests I did a few weeks ago, but
>>>>    I got repeated crashes when loading the data (into a table with
>>>> tsvector+gin index).
>>>>
>>>> Right before a crash, theres this message in the log:
>>>>
>>>>      PANIC:  not enough space in leaf page!
>>>>
>>>
>>> Thanks for testing. Heikki's version of patch don't works for me too on
>>> even much more simplier examples. I can try to get it working if he
>>> answer
>>> my question about GinDataLeafPageGetPostingList* macros.
>>
>> The new macros in that patch version were quite botched. Here's a new
>> attempt.
>
> Nope, still the same errors :-(
>
> PANIC:  not enough space in leaf page!
> LOG:  server process (PID 29722) was terminated by signal 6: Aborted
> DETAIL:  Failed process was running: autovacuum: ANALYZE public.messages

I've continued hacking away at the patch, here's yet another version. 
I've done a lot of cleanup and refactoring to make the code more 
readable (I hope). I'm not sure what caused the panic you saw, but it's 
probably fixed now.  Let me know if not.

Some notable changes since my last patch version:

* I changed the ginbtree interface so that isEnoughSpace method is no 
more. Instead, placeToPage returns false without doing anything if the 
item doesn't fit. It was slightly simpler this way when working with the 
compressed posting lists, as placeToPage had to calculate tuple sizes 
anyway to decide how many items fit on the page.

* I rewrote incrUpdateItemIndexes(). It now operates in two stages: 1. 
adjust the pageOffsets and 2. split the bin. I found it more readable 
this way.

* I changed the WAL format of insert records so that there is a separate 
struct for data-leaf, data-internal, and entry pages. The information 
recorded for each of those was so different that it was confusing to 
cram them all into the same struct.


I'm going to step back a bit from the details of the patch now. I think 
there's enough work for you, Alexander, until the next commitfest:

* Try to make the code also work with the old page format, so that you 
don't need to REINDEX after pg_upgrade.

* Documentation. The new compressed posting list format needs to be 
explained somewhere. At the top of ginpostinglist.c would be a good 
place. The README should be improved too.

* Fix whatever I broke (sorry)

Are we committed to go ahead with this patch in 9.4 timeframe, in one 
form or another? If we are, then I'd like to start committing 
refactoring patches that just move code around in preparation for the 
Patch, to make the review of the core of the patch easier. For example, 
merging the isEnoughSpace and placeToPage methods in the ginbtree 
interface. Without the patch, it's unnecessary code churn - it won't do 
any harm but it won't do any good either. I'm pretty confident that this 
patch will land in the 9.4 timeframe, so barring objections I'll start 
committing such refactorings.

- Heikki

-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Attachments:

  [application/x-gzip] gin-packed-postinglists-10-heikki.patch.gz (25.5K, ../52569635.5090808@vmware.com/2-gin-packed-postinglists-10-heikki.patch.gz)
  download

view thread (119+ messages)  latest in thread

Message-ID: <52569635.5090808@vmware.com>
Permalink:  ../52569635.5090808@vmware.com/
Also on:    postgresql.org/message-id/52569635.5090808@vmware.com

 · 

reply

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Reply to all the recipients using the --to and --cc options:
  reply via email

  To: pgsql-hackers@postgresql.org
  Cc: hlinnakangas@vmware.com, aekorotkov@gmail.com, tv@fuzzy.cz
  Subject: Re: GIN improvements part 1: additional information
  In-Reply-To: <52569635.5090808@vmware.com>

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

This inbox is served by DDX for PostgreSQL; see mirroring instructions
for how to clone and mirror all data and code used for this inbox