Re: [Patch] Optimize dropping of relation buffers using dlist

Thomas Munro <thomas.munro@gmail.com>

From: Thomas Munro <thomas.munro@gmail.com>
To: Tom Lane <tgl@sss.pgh.pa.us>
Cc: "k.jamison@fujitsu.com" <k.jamison@fujitsu.com>, "tsunakawa.takay@fujitsu.com" <tsunakawa.takay@fujitsu.com>, Kyotaro Horiguchi <horikyota.ntt@gmail.com>, "amit.kapila16@gmail.com" <amit.kapila16@gmail.com>, "andres@anarazel.de" <andres@anarazel.de>, "robertmhaas@gmail.com" <robertmhaas@gmail.com>, "tomas.vondra@2ndquadrant.com" <tomas.vondra@2ndquadrant.com>, "pgsql-hackers@postgresql.org" <pgsql-hackers@postgresql.org>
Date: 2020-10-22T05:54:43Z
Lists: pgsql-hackers
On Thu, Oct 22, 2020 at 5:52 PM Tom Lane <tgl@sss.pgh.pa.us> wrote:
> Per the referenced bug-reporting thread, it was ReiserFS and/or NFS on
> SLES 9.3; so, dubious storage choices on an ancient-even-then Linux
> kernel.

Ohhhh.  I can reproduce that on a modern Linux box by forcing
writeback to a full NFS filesystem[1], approximately as the kernel
does asynchronously when it feels like it, causing the size reported
by SEEK_END to go down.

$ cat magic_shrinking_file.c
#include <fcntl.h>
#include <stdio.h>
#include <stdlib.h>
#include <unistd.h>

int main()
{
  int fd;
  char buffer[8192] = {0};

  fd = open("/mnt/test_loopback_remote/dir/file", O_RDWR | O_APPEND);
  if (fd < 0) {
    perror("open");
    return EXIT_FAILURE;
  }
  printf("lseek(..., SEEK_END) = %jd\n", lseek(fd, 0, SEEK_END));
  printf("write(...)  = %zd\n", write(fd, buffer, sizeof(buffer)));
  printf("lseek(..., SEEK_END) = %jd\n", lseek(fd, 0, SEEK_END));
  printf("fsync(...) = %d\n", fsync(fd));
  printf("lseek(..., SEEK_END) = %jd\n", lseek(fd, 0, SEEK_END));

  return EXIT_SUCCESS;
}
$ cc magic_shrinking_file.c
$ ./a.out
lseek(..., SEEK_END) = 9670656
write(...)  = 8192
lseek(..., SEEK_END) = 9678848
fsync(...) = -1
lseek(..., SEEK_END) = 9670656

> I think the takeaway point is not so much that that particular bug
> might recur as that storage infrastructure does sometimes have bugs.
> If you're wanting to introduce new assumptions about what the filesystem
> will do, it's prudent to think about how badly will we break if the
> assumptions fail.

Yeah.  My point was just that the caching trick doesn't seem to
improve matters on this particular front, it can just cache a bogus
value.

[1] https://www.postgresql.org/message-id/CAEepm=1FGo=ACPKRmAxvb53mBwyVC=TDwTE0DMzkWjdbAYw7sw@mail.gmail.com



Commits

  1. Fix size overflow in calculation introduced by commits d6ad34f3 and bea449c6.

  2. Optimize DropRelFileNodesAllBuffers() for recovery.

  3. Optimize DropRelFileNodeBuffers() for recovery.

  4. Cache smgrnblocks() results in recovery.

  5. Add a check to prevent overwriting valid data if smgrnblocks() gives a