Re: [Proposal] Fully WAL logged CREATE DATABASE - No Checkpoints

On Wed, Mar 9, 2022 at 6:07 AM Dilip Kumar <dilipbalaut@gmail.com> wrote:
> Yeah you are correct about the local map, but I am not sure whether we
> can rely on not updating the shared map in the startup process.
> Because how can we guarantee that now or in future the startup process
> can never look into the map?  I agree that it is not connected to the
> database so it doesn't make sense to look into the local map but how
> we are going to ensure the shared map.  Said that I think there are
> only 3 function which must be looking at these maps directly
> RelationMapOidToFilenode(), RelationMapFilenodeToOid() and
> RelationMapUpdateMap() and these functions are called from a very few
> places and I don't think these should be called during recovery.  So
> probably we can put a elog saying they should never be called during
> recovery?

Yeah, that seems reasonable.

> Right, infact now also if you see the logic, the
> write_relmap_file_internal() is taking care of the actual update and
> the write_relmap_file() is doing update + setting the global
> variables.  So yeah we can rename as you suggested in 0001 and here
> also we can change the logic as you suggested that the recovery and
> createdb will only call the first function which is just doing the
> update.

But I think we want the path construction to be managed by the
function rather than the caller, too.

> I'd expect to
> > find it in storage.c, I think. And I think I'd be surprised to find
> > out that it doesn't actually know anything about copying; it's
> > basically just a loop over the forks to which you can supply your own
> > copy-function.
>
> Yeah but it eventually expects a function pointer to copy storage so
> we can not completely deny that it knows nothing about the copy?

Sure, I guess. It's just not obvious why the argument would actually
need to be a function that copies storage, or why there's more than
one way to copy storage. I'd rather keep all the code paths unified,
if we can, and set behavior via flags or something, maybe. I'm not
sure whether that's realistic, though.

> And the fact that it's got an argument of type
> > copy_relation_storage and the argument name is copy_storage and the
> > value is sometimes RelationCopyStorageis a terminological muddle, too.
> > So I feel like perhaps this needs more thought.
>
> One option is that we can duplicate this loop in dbcommand.c as well
> like we are having it already in tablecmds.c and heapam_handler.c?

Yeah, I think this is also worth considering.

-- 
Robert Haas
EDB: http://www.enterprisedb.com