On Fri, Mar 1, 2019 at 11:31 AM Shawn Debnath <s...@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 <and...@anarazel.de> wrote: > > > > On 2019-02-28 13:16:02 -0500, Tom Lane wrote: > > > > > Shawn Debnath <s...@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