On Fri, Nov 6, 2020 at 5:02 AM Thomas Munro <thomas.munro@gmail.com> wrote:
>
> On Thu, Nov 5, 2020 at 10:47 PM Amit Kapila <amit.kapila16@gmail.com> wrote:
> > I still feel 'cached' is a better name.
>
> Amusingly, this thread is hitting all the hardest problems in computer
> science according to the well known aphorism...
>
> Here's a devil's advocate position I thought about: It's OK to leave
> stray buffers (clean or dirty) in the buffer pool if files are
> truncated underneath us by gremlins, as long as your system eventually
> crashes before completing a checkpoint. The OID can't be recycled
> until after a successful checkpoint, so the stray blocks can't be
> confused with the blocks of another relation, and weird errors are
> expected on a system that is in serious trouble. It's actually much
> worse that we can give incorrect answers to queries when files are
> truncated by gremlins (in the window of time before we presumably
> crash because of EIO), because we're violating basic ACID principles
> in user-visible ways. In this thread, discussion has focused on
> availability (ie avoiding failures when trying to write back stray
> buffers to a file that has been unlinked), but really a system that
> can't see arbitrary committed transactions *shouldn't be available*.
> This argument applies whether you think SEEK_END can only give weird
> answers in the specific scenario I demonstrated with NFS, or whether
> you think it's arbitrarily b0rked and reports random numbers: we
> fundamentally can't tolerate that, so why are we trying to?
>
It is not very clear to me how this argument applies to the patch
in-discussion where we are relying on the cached value of blocks
during recovery. I understand your point that we might skip scanning
the pages and thus might not show some recently added data but that
point is not linked with what we are trying to do with this patch.
AFAIU, the theory we discussed above is that there shouldn't be any
stray blocks in the buffers with this patch because even if the
smgrnblocks(SEEK_END) didn't gave us the right answers, we shouldn't
have any buffers for the blocks after the size returned by smgrnblocks
during recovery. I think the problem could happen if we extend the
relation by multiple blocks which will invalidate the cached value
during recovery and then probably the future calls to smgrnblocks can
lead to problems if it lies with us but as far as I know we don't do
that. Can you please be more specific how this patch can lead to a
problem?
--
With Regards,
Amit Kapila.
