Re: CompactCheckpointerRequestQueue versus pad bytes

From: Robert Haas <robertmhaas(at)gmail(dot)com>
To: Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>
Cc: pgsql-hackers(at)postgresql(dot)org
Subject: Re: CompactCheckpointerRequestQueue versus pad bytes
Date: 2012-07-16 19:04:47
Message-ID: CA+Tgmob4oxXE7oY7K-wgGtFzeVzaXW_aZy9QfJ_EGWfu=k=dug@mail.gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

On Mon, Jul 16, 2012 at 2:47 PM, Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> wrote:
> I wrote:
>> Robert Haas <robertmhaas(at)gmail(dot)com> writes:
>>> On Mon, Jul 16, 2012 at 11:57 AM, Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)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

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Tom Lane 2012-07-16 19:18:53 Re: [PERFORM] DELETE vs TRUNCATE explanation
Previous Message Tom Lane 2012-07-16 19:03:03 Re: [PERFORM] DELETE vs TRUNCATE explanation