On Mon, Jul 16, 2012 at 2:47 PM, Tom Lane <tgl@sss.pgh.pa.us> wrote:
> 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 ...
Hmm, yeah, I have a feeling this might be why I didn't do this when I
created RelFileNodeBackend. But I think your reasoning is correct.
Sticking the above text in a comment might not be out of order,
however.
--
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company