Re: Drop type "smgr"?

On Thu, Feb 28, 2019 at 10:35:50AM -0500, Robert Haas wrote:
> On Thu, Feb 28, 2019 at 10:09 AM Tom Lane <tgl@sss.pgh.pa.us> wrote:
> > The real reason I'm concerned about this, though, is that for either
> > a database or a tablespace, you can *not* get away with having a magic
> > OID just hanging in space with no actual catalog row matching it.
> > If nothing else, you need an entry there to prevent someone from
> > reusing the OID for another purpose.  And a pg_database row that
> > doesn't correspond to a real database is going to break all kinds of
> > code, starting with pg_upgrade and the autovacuum launcher.  Special
> > rows in pg_tablespace are much less likely to cause issues, because
> > of the precedent of pg_global and pg_default.
> 
> Also, I don't see why we'd need a fake pg_database row in the first
> place.  IIUC, the OID counter wraps around to FirstNormalObjectId, so
> nobody should have a database with an OID less than that value.

We have scripts under catalog directory that can check to ensure OIDs 
aren't re-used accidentally. However, we still have to define an entry 
in a catalog somewhere and I was proposing creating a new one, 
pg_storage_managers?, to track these entries. See [1] for previous
discussion on this topic. We wouldn't need to do catalog lookups for
being able to use the smgrs as the OIDs will be hardcoded in C, but the
data will be available for posterity and OID reservation.

> It's certainly possible/likely that we're going to end up needing to
> widen buffer tags to represent the smgr explicitly, because some use
> cases are going to need a real database spec, some are going to need
> a real tablespace spec, and some might need both.  Maybe we should
> just bite that bullet.

For now, the two projects that require the new smgrs, undo and slru, can 
get away with using the database OID as the smgr differentiator. We have 
enough left over bits to get our work done (Thomas please correct me if 
I am mistaken). The only issue is pg_buffercache would present DB OIDs 
that wouldn't map correctly (along with the rest of relfilenode).  We 
can modify the output to include more columns to give caching specific 
information when these special buffers are encountered.

> Well, Andres will probably complain about that.  He thinks, IIUC, that
> the buffer tags are too wide already and that it's significantly
> hurting performance on very very common operations - like buffer
> lookups.  I haven't verified that myself, but I tend to think he knows
> what he's talking about.

I can imagine this would. I am curious about the pef impact, going to at
least create a patch and do some testing.

Another thought: my colleague Anton Shyrabokau suggested potentially
re-using forknumber to differentiate smgrs. We are using 32 bits to
map 5 entries today. This approach would be similar to how we split up
the segment numbers and use the higher bits to identify forget or
unlink requests for checkpointer.

> Anyway, given that your argument started off from the premise that we
> had to have a pg_database row, I think we'd better look a little
> harder at whether that premise is correct before getting too excited
> here.  As I said in my earlier reply, I think that we probably don't
> need to have a pg_database row given that we wrap around to
> FirstNormalObjectId; any value we hard-code would be less than that.
> If there are other reasons why we'd need that, it might be useful to
> hear about them.

See above, I think a new catalog, instead of pg_database, can resolve
the issue while providing the ability to reserve the OIDs.

> However, all we really need to decide on this thread is whether we
> need 'smgr' exposed as an SQL type.  And I can't see why we need that
> no matter how all of the rest of this turns out.  Nobody is currently
> proposing to give users a choice of smgrs, just to use them for
> internal stuff.  Even if that changed later, it doesn't necessarily
> mean we'd add back an SQL type, or that if we did it would look like
> the one we have today.

+1 smgr types can be removed.


-- 
Shawn Debnath
Amazon Web Services (AWS)