Hi,
On 2019-05-13 12:24:05 -0400, Alvaro Herrera wrote:
> > My guess, just shooting from the hip, is that the smgrimmedsync call
> > can be removed here. If that's wrong, then we need a better
> > explanation for why it's needed, and we possibly need to add it to
> > every single place that does smgrcreate that doesn't have it already.
>
> AFAICS ResetUnloggedRelations copies the init fork after replaying WAL,
> so it would be sufficient to have the init fork be recovered from WAL
> for that to work. However, we also do ResetUnloggedRelations *before*
> replaying WAL in order to remove leftover not-init-fork files, and that
> process requires that the init fork is present at that time.
What scenario are you precisely wondering about? That
ResetUnloggedRelations() could overwrite the main fork, while not yet
having valid contents (due to the lack of smgrimmedsync())? Shouldn't
that only be possible while still in an inconsistent state? A checkpoint
would have serialized the correct contents, and we'd not reach HS
consistency before having replayed that WAL records resetting the table
and the init fork consistency?
> So I think
> the immedsync call is necessary (otherwise the cleanup may fail). I
> don't quite understand why the log_smgrcreate is necessary, but I think
> it is for reasons that are not adequately explained by the existing
> comments.
Well, otherwise the relation won't exist on a standby? And if replay
starts from before a database/tablespace creation we'd remove the init
fork. So if it's not in the WAL, we'd loose it.
> IMO if you can remove either the immedsync or the log_smgrcreate call
> and no test fails, then we're either missing test cases, or (one of) the
> calls is unnecessary.
We definitely don't have a high coverage of more complicated recovery
scenarios.
Greetings,
Andres Freund