I wrote:
> Robert Haas <robertmhaas@gmail.com> writes:
>> On Mon, Jul 16, 2012 at 11:57 AM, Tom Lane <tgl@sss.pgh.pa.us> wrote:
>>> So that brings us back to the question of why this code is supporting
>>> fsync requests for local relations in the first place. Couldn't we have
>>> it ignore those, and then only ship RelFileNode to the checkpointer?
>> That's an awfully good point. I think that was just sloppy coding on
>> my part (cf commit debcec7dc31a992703911a9953e299c8d730c778). +1 for
>> changing it as you suggest.
> 2. Change the fsync forwarding code to ignore backend-local relations,
> and include only RelFileNode not RelFileNodeBackend in requests.
So I started to do this, and immediately hit a roadblock: although it's
clear that we can discard any attempt to fsync a backend-local relation,
it's not so clear that we don't need to queue UNLINK_RELATION_REQUEST
operations for local relations.
I think that this is in fact okay. The reason for delaying unlink until
after the next checkpoint is that if we did not, and the relfilenode got
re-used for an unrelated relation, and then we crashed and had to replay
WAL, we would replay any WAL referencing the old relation into the
unrelated relation's storage, with potential bad consequences if you're
unlucky. However, no WAL should ever be generated for a backend-local
relation, so there is nothing to guard against and hence no harm in
immediately unlinking backend-local rels when they are deleted. So we
can adjust mdunlink to include SmgrIsTemp() among the reasons to unlink
immediately rather than doing the truncate-and-register_unlink dance.
If anybody sees a hole in this reasoning, speak now ...
regards, tom lane
