On Fri, Mar 1, 2019 at 11:31 AM Shawn Debnath <sdn@amazon.com> wrote:
> On Thu, Feb 28, 2019 at 02:08:49PM -0800, Andres Freund wrote:
> > On 2019-03-01 09:48:33 +1300, Thomas Munro wrote:
> > > On Fri, Mar 1, 2019 at 7:24 AM Andres Freund <andres@anarazel.de> wrote:
> > > > On 2019-02-28 13:16:02 -0500, Tom Lane wrote:
> > > > > Shawn Debnath <sdn@amazon.com> writes:
> > > > > > Another thought: my colleague Anton Shyrabokau suggested potentially
> > > > > > re-using forknumber to differentiate smgrs. We are using 32 bits to
> > > > > > map 5 entries today.
> > > > >
> > > > > Yeah, that seems like it might be a workable answer.
> > > >
> > > > Yea, if we just split that into two 16bit entries, there'd not be much
> > > > lost. Some mild mild performance regression due to more granular
> > > > memory->register reads/writes, but I can't even remotely see that
> > > > matter.
> > >
> > > Ok, that's a interesting way to include it in BufferTag without making
> > > it wider. But then how about the SMGR interface? I think that value
> > > would need to be added to the smgropen() interface, and all existing
> > > callers would pass in (say) SGMR_RELATION (meaning "use md.c"), and
> > > new code would use SMGR_UNDO, SMGR_SLRU etc. That seems OK to me.
> >
> > Right, seems like we should do that independent of whether we end up
> > reusing the dboid or not.
>
> Food for thought: if we are going to muck with the smgr APIs, would it
> make sense to move away from SMgrRelation to something a bit more
> generic like, say, SMgrHandle to better organize the internal contents
> of this structure? Internally, we could move things into an union and
> based on type of handle: relation, undo, slru/generic, translate the
> contents correctly. As you can guess, SMgrRelationData today is very
> specific to relations and holding md specific information whose memory
> would be better served re-used for the other storage managers.
>
> Thoughts?
Right, it does contain some md-specific stuff without even
apologising. Also smgropen() was rendered non-virtual at some point
(I mean that implementations don't even get a chance to initialise
anything, which works out only because md-specific code has leaked
into smgr.c). In one of my undo patches (which I'll post an updated
version of on the appropriate thread soon) I added a void * called
private_data so that undo_file.c can keep track of some stuff, but
yeah I agree that more tidying up could be done.
--
Thomas Munro
https://enterprisedb.com
