Re: init_sequence spill to hash table

On 2013-11-15 19:12:15 +1300, David Rowley wrote:
> On Fri, Nov 15, 2013 at 3:23 AM, Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> wrote:
>
> > Andres Freund <andres(at)2ndquadrant(dot)com> writes:
> > > I think it'd be a better idea to integrate the sequence caching logic
> > > into the relcache. There's a comment about it:
> > > * (We can't
> > > * rely on the relcache, since it's only, well, a cache, and may decide
> > to
> > > * discard entries.)
> > > but that's not really accurate anymore. We have the infrastructure for
> > > keeping values across resets and we don't discard entries.
> >
> > We most certainly *do* discard entries, if they're not open when a cache
> > flush event comes along.
> >
> > I suppose it'd be possible to mark a relcache entry for a sequence
> > as locked-in-core, but that doesn't attract me at all. A relcache
> > entry is a whole lot larger than the amount of state we really need
> > to keep for a sequence.
> >
> > One idea is to have a hashtable for the sequence-specific data,
> > but to add a link field to the relcache entry that points to the
> > non-flushable sequence hashtable entry. That would save the second
> > hashtable lookup as long as the relcache entry hadn't been flushed
> > since last use, while not requiring any violence to the lifespan
> > semantics of relcache entries. (Actually, if we did that, it might
> > not even be worth converting the list to a hashtable? Searches would
> > become a lot less frequent.)
> >
> >
> Unless I've misunderstood something it looks like this would mean giving
> heamam.c and relcache.c knowledge of sequences.
> Currently relation_open is called from open_share_lock in sequence.c. The
> only way I can see to do this would be to add something like
> relation_open_sequence() in heapam.c which means we'd need to invent
> RelationIdGetSequenceRelation() and use that instead
> of RelationIdGetRelation() and somewhere along the line have it pass back
> the SeqTableData struct which would be tagged onto RelIdCacheEnt.

Look like indexes already go through that path, and store data in the
relcache, without requiring heapam.c to know all that much.

> I think it can be done but I don't think it will look pretty.
> Perhaps if there was a more generic way... Would tagging some void
> *rd_extra only the RelationData be a better way? And just have sequence.c
> make use of that for storing the SeqTableData.

I'd rather have it properly typed in ->rd_sequence, maybe in a union
against the index members.

> Also I'm wondering what we'd do with all these pointers when someone does
> DISCARD SEQUENCES; would we have to invalidate the relcache or would it
> just be matter of looping over it and freeing of the sequence data setting
> the pointers to NULL?

Good question. Have a 'discard_count' member and global variable? That
starts at zero and gets incremented everytime somebody discards?

Greetings,

Andres Freund

--
Andres Freund http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Training & Services