public inbox for [email protected]help / color / mirror / Atom feed
[PATCH 2/7] Write WAL for empty nbtree index build 10+ messages / 5 participants [nested] [flat]
* [PATCH 2/7] Write WAL for empty nbtree index build @ 2018-10-11 01:03 Kyotaro Horiguchi <[email protected]> 0 siblings, 0 replies; 10+ messages in thread From: Kyotaro Horiguchi @ 2018-10-11 01:03 UTC (permalink / raw) After relation truncation indexes are also rebuild. It doesn't emit WAL in minimal mode and if truncation happened within its creation transaction, crash recovery leaves an empty index heap, which is considered broken. This patch forces to emit WAL when an index_build turns into empty nbtree index. --- src/backend/access/nbtree/nbtsort.c | 12 ++++++++++-- 1 file changed, 10 insertions(+), 2 deletions(-) diff --git a/src/backend/access/nbtree/nbtsort.c b/src/backend/access/nbtree/nbtsort.c index 9ac4c1e1c0..a31d58025f 100644 --- a/src/backend/access/nbtree/nbtsort.c +++ b/src/backend/access/nbtree/nbtsort.c @@ -654,8 +654,16 @@ _bt_blwritepage(BTWriteState *wstate, Page page, BlockNumber blkno) /* Ensure rd_smgr is open (could have been closed by relcache flush!) */ RelationOpenSmgr(wstate->index); - /* XLOG stuff */ - if (wstate->btws_use_wal) + /* XLOG stuff + * + * Even when wal_level is minimal, WAL is required here if truncation + * happened after being created in the same transaction. This is hacky but + * we cannot use BufferNeedsWAL() stuff for nbtree since it can emit + * atomic WAL records on multiple buffers. + */ + if (wstate->btws_use_wal || + (RelationNeedsWAL(wstate->index) && + (blkno == BTREE_METAPAGE && BTPageGetMeta(page)->btm_root == 0))) { /* We use the heap NEWPAGE record type for this */ log_newpage(&wstate->index->rd_node, MAIN_FORKNUM, blkno, page, true); -- 2.16.3 ----Next_Part(Fri_Apr_05_12_55_20_2019_986)-- Content-Type: Text/X-Patch; charset=us-ascii Content-Transfer-Encoding: 7bit Content-Disposition: inline; filename="v11-0003-Move-XLOG-stuff-from-heap_insert-and-heap_delete.patch" ^ permalink raw reply [nested|flat] 10+ messages in thread
* Re: Fwd: Re: A new look at old NFS readdir() problems? @ 2025-01-02 23:20 Tom Lane <[email protected]> 0 siblings, 1 reply; 10+ messages in thread From: Tom Lane @ 2025-01-02 23:20 UTC (permalink / raw) To: Thomas Munro <[email protected]>; +Cc: Robert Haas <[email protected]>; Larry Rosenman <[email protected]>; Pgsql hackers <[email protected]> Thomas Munro <[email protected]> writes: > For what little it's worth, I'm not quite convinced yet that FreeBSD's > client isn't more broken than it needs to be. I'm suspicious of that too. The wireshark trace you described is hard to read any other way than that FreeBSD went out of its way to deliver incorrect information. I'm prepared to believe that we can't work correctly on NFS servers that don't do the stable-cookie thing, but why isn't it succeeding on ones that do? regards, tom lane ^ permalink raw reply [nested|flat] 10+ messages in thread
* Re: Fwd: Re: A new look at old NFS readdir() problems? @ 2025-01-03 01:58 Tom Lane <[email protected]> parent: Tom Lane <[email protected]> 0 siblings, 1 reply; 10+ messages in thread From: Tom Lane @ 2025-01-03 01:58 UTC (permalink / raw) To: Thomas Munro <[email protected]>; +Cc: Robert Haas <[email protected]>; Larry Rosenman <[email protected]>; Pgsql hackers <[email protected]> I wrote: > Thomas Munro <[email protected]> writes: >> For what little it's worth, I'm not quite convinced yet that FreeBSD's >> client isn't more broken than it needs to be. > I'm suspicious of that too. I poked at this a little further. I made the attached stand-alone test case (you don't need any more than "cc -o rmtree rmtree.c" to build it, then point the script at some NFS-mounted directory). This fails with my NAS at least as far back as FreeBSD 11.0. I also tried it on NetBSD 9.2 which seems fine. regards, tom lane Attachments: [text/x-shellscript] rmtree-test.sh (147B, ../../[email protected]/2-rmtree-test.sh) download | inline: #! /bin/sh set -e TESTDIR="$1" mkdir "$TESTDIR" i=0 while [ $i -lt 1000 ] do touch "$TESTDIR/$i" i=`expr $i + 1` done ./rmtree "$TESTDIR" [text/x-c] rmtree.c (5.0K, ../../[email protected]/3-rmtree.c) download | inline: /*------------------------------------------------------------------------- * * rmtree.c * * Portions Copyright (c) 1996-2025, PostgreSQL Global Development Group * Portions Copyright (c) 1994, Regents of the University of California * * IDENTIFICATION * src/common/rmtree.c * *------------------------------------------------------------------------- */ #include <stdio.h> #include <stdlib.h> #include <stdbool.h> #include <string.h> #include <unistd.h> #include <errno.h> #include <sys/stat.h> #include <sys/types.h> #include <dirent.h> typedef enum PGFileType { PGFILETYPE_ERROR, PGFILETYPE_UNKNOWN, PGFILETYPE_REG, PGFILETYPE_DIR, PGFILETYPE_LNK, } PGFileType; static void * palloc(size_t size) { void *tmp; /* Avoid unportable behavior of malloc(0) */ if (size == 0) size = 1; tmp = malloc(size); if (tmp == NULL) { fprintf(stderr, "out of memory\n"); exit(1); } return tmp; } static void * repalloc(void *ptr, size_t size) { void *tmp; /* Avoid unportable behavior of realloc(NULL, 0) */ if (ptr == NULL && size == 0) size = 1; tmp = realloc(ptr, size); if (!tmp) { fprintf(stderr, "out of memory\n"); exit(1); } return tmp; } static char * pstrdup(const char *in) { char *tmp; if (!in) { fprintf(stderr, "cannot duplicate null pointer (internal error)\n"); exit(1); } tmp = strdup(in); if (!tmp) { fprintf(stderr, "out of memory\n"); exit(1); } return tmp; } /* * Return the type of a directory entry. */ static PGFileType get_dirent_type(const char *path, const struct dirent *de, bool look_through_symlinks) { PGFileType result; /* * Some systems tell us the type directly in the dirent struct, but that's * a BSD and Linux extension not required by POSIX. Even when the * interface is present, sometimes the type is unknown, depending on the * filesystem. */ #if defined(DT_REG) && defined(DT_DIR) && defined(DT_LNK) if (de->d_type == DT_REG) result = PGFILETYPE_REG; else if (de->d_type == DT_DIR) result = PGFILETYPE_DIR; else if (de->d_type == DT_LNK && !look_through_symlinks) result = PGFILETYPE_LNK; else result = PGFILETYPE_UNKNOWN; #else result = PGFILETYPE_UNKNOWN; #endif if (result == PGFILETYPE_UNKNOWN) { struct stat fst; int sret; if (look_through_symlinks) sret = stat(path, &fst); else sret = lstat(path, &fst); if (sret < 0) { result = PGFILETYPE_ERROR; fprintf(stderr, "could not stat file \"%s\": %m\n", path); } else if (S_ISREG(fst.st_mode)) result = PGFILETYPE_REG; else if (S_ISDIR(fst.st_mode)) result = PGFILETYPE_DIR; else if (S_ISLNK(fst.st_mode)) result = PGFILETYPE_LNK; } return result; } /* * rmtree * * Delete a directory tree recursively. * Assumes path points to a valid directory. * Deletes everything under path. * If rmtopdir is true deletes the directory too. * Returns true if successful, false if there was any problem. * (The details of the problem are reported already, so caller * doesn't really have to say anything more, but most do.) */ static bool rmtree(const char *path, bool rmtopdir) { char pathbuf[8192]; DIR *dir; struct dirent *de; bool result = true; size_t dirnames_size = 0; size_t dirnames_capacity = 8; char **dirnames; dir = opendir(path); if (dir == NULL) { fprintf(stderr, "could not open directory \"%s\": %m\n", path); return false; } dirnames = (char **) palloc(sizeof(char *) * dirnames_capacity); while (errno = 0, (de = readdir(dir))) { if (strcmp(de->d_name, ".") == 0 || strcmp(de->d_name, "..") == 0) continue; snprintf(pathbuf, sizeof(pathbuf), "%s/%s", path, de->d_name); switch (get_dirent_type(pathbuf, de, false)) { case PGFILETYPE_ERROR: /* already logged, press on */ break; case PGFILETYPE_DIR: /* * Defer recursion until after we've closed this directory, to * avoid using more than one file descriptor at a time. */ if (dirnames_size == dirnames_capacity) { dirnames = repalloc(dirnames, sizeof(char *) * dirnames_capacity * 2); dirnames_capacity *= 2; } dirnames[dirnames_size++] = pstrdup(pathbuf); break; default: if (unlink(pathbuf) != 0 && errno != ENOENT) { fprintf(stderr, "could not remove file \"%s\": %m\n", pathbuf); result = false; } break; } } if (errno != 0) { fprintf(stderr, "could not read directory \"%s\": %m\n", path); result = false; } closedir(dir); /* Now recurse into the subdirectories we found. */ for (size_t i = 0; i < dirnames_size; ++i) { if (!rmtree(dirnames[i], true)) result = false; free(dirnames[i]); } if (rmtopdir) { if (rmdir(path) != 0) { fprintf(stderr, "could not remove directory \"%s\": %m\n", path); result = false; } } free(dirnames); return result; } int main(int argc, char **argv) { if (argc != 2) { fprintf(stderr, "usage: %s target-directory\n", argv[0]); exit(1); } if (!rmtree(argv[1], true)) { fprintf(stderr, "rmtree failed\n"); exit(1); } return 0; } ^ permalink raw reply [nested|flat] 10+ messages in thread
* Re: Fwd: Re: A new look at old NFS readdir() problems? @ 2025-01-03 17:31 Peter Eisentraut <[email protected]> parent: Tom Lane <[email protected]> 0 siblings, 1 reply; 10+ messages in thread From: Peter Eisentraut @ 2025-01-03 17:31 UTC (permalink / raw) To: Tom Lane <[email protected]>; Thomas Munro <[email protected]>; +Cc: Robert Haas <[email protected]>; Larry Rosenman <[email protected]>; Pgsql hackers <[email protected]> On 03.01.25 02:58, Tom Lane wrote: > I wrote: >> Thomas Munro <[email protected]> writes: >>> For what little it's worth, I'm not quite convinced yet that FreeBSD's >>> client isn't more broken than it needs to be. > >> I'm suspicious of that too. > > I poked at this a little further. I made the attached stand-alone > test case (you don't need any more than "cc -o rmtree rmtree.c" > to build it, then point the script at some NFS-mounted directory). > This fails with my NAS at least as far back as FreeBSD 11.0. > I also tried it on NetBSD 9.2 which seems fine. If you use some GUI file manager, point at a directory, and select "remove this directory with everything in it", what does it do internally? Surely it runs a readdir() loop and unlinks the files as it gets to them? Or does it indeed slurp the whole directory tree into memory before starting the deletion? It seems like we're probably not the first to have this use case, so there should be some information or problem reports about this somewhere out there. Or if not, then I'd be tempted to think, someone broke it recently, even if the NFS hacker argues that it cannot possibly work and never should have worked. ^ permalink raw reply [nested|flat] 10+ messages in thread
* Re: Fwd: Re: A new look at old NFS readdir() problems? @ 2025-01-03 18:12 Tom Lane <[email protected]> parent: Peter Eisentraut <[email protected]> 0 siblings, 1 reply; 10+ messages in thread From: Tom Lane @ 2025-01-03 18:12 UTC (permalink / raw) To: Peter Eisentraut <[email protected]>; +Cc: Thomas Munro <[email protected]>; Robert Haas <[email protected]>; Larry Rosenman <[email protected]>; Pgsql hackers <[email protected]> Peter Eisentraut <[email protected]> writes: > On 03.01.25 02:58, Tom Lane wrote: >> I poked at this a little further. I made the attached stand-alone >> test case (you don't need any more than "cc -o rmtree rmtree.c" >> to build it, then point the script at some NFS-mounted directory). >> This fails with my NAS at least as far back as FreeBSD 11.0. >> I also tried it on NetBSD 9.2 which seems fine. > If you use some GUI file manager, point at a directory, and select > "remove this directory with everything in it", what does it do > internally? Surely it runs a readdir() loop and unlinks the files as it > gets to them? Or does it indeed slurp the whole directory tree into > memory before starting the deletion? One thing I noticed while testing yesterday is that "rm -rf foo" worked even in cases where "rmtree foo" didn't. I did not look into FreeBSD's rm code, but I'll bet it has the sort of retry logic that was recommended to us upthread. I agree with your point though that it's hard to believe that everyone does that in every case where it would matter. As for pre-existing bug reports, I found https://bugs.freebsd.org/bugzilla/show_bug.cgi?id=57696 but there's basically no new information there, other than the not-so-surprising fact that renaming directory entries triggers the failure as efficiently as unlinking does. regards, tom lane ^ permalink raw reply [nested|flat] 10+ messages in thread
* Re: Fwd: Re: A new look at old NFS readdir() problems? @ 2025-01-03 23:39 Thomas Munro <[email protected]> parent: Tom Lane <[email protected]> 0 siblings, 1 reply; 10+ messages in thread From: Thomas Munro @ 2025-01-03 23:39 UTC (permalink / raw) To: Tom Lane <[email protected]>; +Cc: Peter Eisentraut <[email protected]>; Robert Haas <[email protected]>; Larry Rosenman <[email protected]>; Pgsql hackers <[email protected]> On Sat, Jan 4, 2025 at 7:12 AM Tom Lane <[email protected]> wrote: > One thing I noticed while testing yesterday is that "rm -rf foo" > worked even in cases where "rmtree foo" didn't. I did not look > into FreeBSD's rm code, but I'll bet it has the sort of retry > logic that was recommended to us upthread. It punts the directory scan to the fts_read() etc functions (directory traversal helpers widely inherited from ancestral BSD but not in any standard), which buffers the scan and probably hides this issue: https://github.com/freebsd/freebsd-src/blob/main/bin/rm/rm.c https://github.com/freebsd/freebsd-src/blob/main/lib/libc/gen/fts.c I doubt that hides all potential problems though, if I have understood the vague outline of this bug correctly: perhaps if you ran large enough rm -r, and you unlinked a file concurrently with that loop, you could break it, that is, cause it to skip innocent files other than the one you unlinked? glibc also has an implementation of the BSD fts functions, and uses them in its rm: https://github.com/coreutils/coreutils/blob/master/src/rm.c https://github.com/coreutils/coreutils/blob/master/src/remove.c > As for pre-existing bug reports, I found > > https://bugs.freebsd.org/bugzilla/show_bug.cgi?id=57696 > > but there's basically no new information there, other than > the not-so-surprising fact that renaming directory entries > triggers the failure as efficiently as unlinking does. FWIW I am discussing this off-list with Rick, I *think* we can distinguish between "gripes abouts NFS that we can't do much about" and "extra thing going wrong here". The fog of superstition around NFS is thick because so many investigations end at a vendor boundary/black box, but here we can understand the cookie scheme and trace through all the layers here... ^ permalink raw reply [nested|flat] 10+ messages in thread
* Re: Fwd: Re: A new look at old NFS readdir() problems? @ 2025-01-04 01:01 Tom Lane <[email protected]> parent: Thomas Munro <[email protected]> 0 siblings, 1 reply; 10+ messages in thread From: Tom Lane @ 2025-01-04 01:01 UTC (permalink / raw) To: Thomas Munro <[email protected]>; +Cc: Peter Eisentraut <[email protected]>; Robert Haas <[email protected]>; Larry Rosenman <[email protected]>; Pgsql hackers <[email protected]> Thomas Munro <[email protected]> writes: > I doubt that hides all potential problems though, if I have understood > the vague outline of this bug correctly: perhaps if you ran large > enough rm -r, and you unlinked a file concurrently with that loop, you > could break it, that is, cause it to skip innocent files other than > the one you unlinked? Yeah. The thing that makes this untenably bad for us is that it's not only your own process's actions that can break readdir(), it's the actions of some unrelated process. So even "buffer the whole directory contents" isn't a reliable fix, because someone else could unlink a file while you're reading the directory. One way to "fix" this from our side is to institute some system-wide interlock that prevents file deletions (and renames, and maybe even creations?) while any process is executing a readdir loop. That's several steps beyond awful from a performance standpoint, and what's worse is that it's still not reliable. It only takes one bit of code that isn't on board with the locking protocol to put you right back at square one. It might not even be code that's part of Postgres proper, so long as it has access to PGDATA. As an example: it'd be unsafe to modify postgresql.conf with emacs, or any other editor that likes to make a backup file. So IMV that's no fix at all. The only other thing I can think of is to read and buffer the whole directory (no matter how large), and then do it *again*, and repeat till we get the same results twice in a row. That's likewise just horrid from a performance standpoint. Worse, I'm not entirely convinced that it's a 100% fix: it seems to rely on rather debatable assumptions about the possible consequences of concurrent directory mods. > FWIW I am discussing this off-list with Rick, I *think* we can > distinguish between "gripes abouts NFS that we can't do much about" > and "extra thing going wrong here". The fog of superstition around > NFS is thick because so many investigations end at a vendor > boundary/black box, but here we can understand the cookie scheme and > trace through all the layers here... I wouldn't have any problem with saying that we don't support NFS implementations that don't have stable cookies. But so far I haven't found any supported platform except FreeBSD that fails the rmtree test against my Synology NAS. I think the circumstantial evidence that FreeBSD is doing something wrong, or wronger than necessary, is strong. regards, tom lane ^ permalink raw reply [nested|flat] 10+ messages in thread
* Re: Fwd: Re: A new look at old NFS readdir() problems? @ 2025-01-04 08:15 Tom Lane <[email protected]> parent: Tom Lane <[email protected]> 0 siblings, 1 reply; 10+ messages in thread From: Tom Lane @ 2025-01-04 08:15 UTC (permalink / raw) To: Thomas Munro <[email protected]>; +Cc: Peter Eisentraut <[email protected]>; Robert Haas <[email protected]>; Larry Rosenman <[email protected]>; Pgsql hackers <[email protected]> I wrote: > I wouldn't have any problem with saying that we don't support NFS > implementations that don't have stable cookies. But so far I haven't > found any supported platform except FreeBSD that fails the rmtree test > against my Synology NAS. To expand on that: I've now found that Linux, macOS, NetBSD, OpenBSD, and illumos/OpenIndiana[1] pass that test. I don't believe that Windows does NFS. That means that FreeBSD is the only one of our supported platforms that fails. I think we should simply document that NFS on FreeBSD is broken, and await somebody getting annoyed enough to fix that brokenness. regards, tom lane [1] Getting OpenIndiana to work under qemu is not a task for those easily discouraged. ^ permalink raw reply [nested|flat] 10+ messages in thread
* Re: Fwd: Re: A new look at old NFS readdir() problems? @ 2025-01-06 14:20 Robert Haas <[email protected]> parent: Tom Lane <[email protected]> 0 siblings, 1 reply; 10+ messages in thread From: Robert Haas @ 2025-01-06 14:20 UTC (permalink / raw) To: Tom Lane <[email protected]>; +Cc: Thomas Munro <[email protected]>; Peter Eisentraut <[email protected]>; Larry Rosenman <[email protected]>; Pgsql hackers <[email protected]> On Sat, Jan 4, 2025 at 3:15 AM Tom Lane <[email protected]> wrote: > To expand on that: I've now found that Linux, macOS, NetBSD, OpenBSD, > and illumos/OpenIndiana[1] pass that test. I don't believe that Windows > does NFS. That means that FreeBSD is the only one of our supported > platforms that fails. I think we should simply document that NFS on > FreeBSD is broken, and await somebody getting annoyed enough to > fix that brokenness. Yeah, that seems like very strong evidence against FreeBSD, but I think Thomas Munro's point about CIFS is worth considering. That is rather widely used, and if the same workarounds would help both that and FreeBSD's NFS, we might want to adopt it even if it's not a complete fix. -- Robert Haas EDB: http://www.enterprisedb.com ^ permalink raw reply [nested|flat] 10+ messages in thread
* Re: Fwd: Re: A new look at old NFS readdir() problems? @ 2025-01-06 14:49 Tom Lane <[email protected]> parent: Robert Haas <[email protected]> 0 siblings, 0 replies; 10+ messages in thread From: Tom Lane @ 2025-01-06 14:49 UTC (permalink / raw) To: Robert Haas <[email protected]>; +Cc: Thomas Munro <[email protected]>; Peter Eisentraut <[email protected]>; Larry Rosenman <[email protected]>; Pgsql hackers <[email protected]> Robert Haas <[email protected]> writes: > Yeah, that seems like very strong evidence against FreeBSD, but I > think Thomas Munro's point about CIFS is worth considering. That is > rather widely used, and if the same workarounds would help both that > and FreeBSD's NFS, we might want to adopt it even if it's not a > complete fix. TBH, I am happy that PG is now failing in a fairly visible way on filesystems that are broken in this fashion. I think that is better than silent data corruption in obscure circumstances, which is where we were before and would be again if we band-aid rmtree() and do nothing else. Nor do I think it's worth the effort to try to become fully bulletproof on the point. I think we should document that CIFS is unsupported. The docs could say something like: Storing databases on filesystems with unreliable readdir() is not supported and can lead to data corruption. If you observe warnings like "directory is not empty" when trying to drop a database, that is strong evidence that the filesystem has unreliable readdir(). Filesystems known to have this problem include NFS on FreeBSD and CIFS on all platforms. We could s/FreeBSD/older FreeBSD/ if something comes of Thomas' efforts to fix that situation. regards, tom lane ^ permalink raw reply [nested|flat] 10+ messages in thread
end of thread, other threads:[~2025-01-06 14:49 UTC | newest] Thread overview: 10+ messages (download: mbox mbox.gz follow: Atom feed) -- links below jump to the message on this page -- 2018-10-11 01:03 [PATCH 2/7] Write WAL for empty nbtree index build Kyotaro Horiguchi <[email protected]> 2025-01-02 23:20 Re: Fwd: Re: A new look at old NFS readdir() problems? Tom Lane <[email protected]> 2025-01-03 01:58 ` Re: Fwd: Re: A new look at old NFS readdir() problems? Tom Lane <[email protected]> 2025-01-03 17:31 ` Re: Fwd: Re: A new look at old NFS readdir() problems? Peter Eisentraut <[email protected]> 2025-01-03 18:12 ` Re: Fwd: Re: A new look at old NFS readdir() problems? Tom Lane <[email protected]> 2025-01-03 23:39 ` Re: Fwd: Re: A new look at old NFS readdir() problems? Thomas Munro <[email protected]> 2025-01-04 01:01 ` Re: Fwd: Re: A new look at old NFS readdir() problems? Tom Lane <[email protected]> 2025-01-04 08:15 ` Re: Fwd: Re: A new look at old NFS readdir() problems? Tom Lane <[email protected]> 2025-01-06 14:20 ` Re: Fwd: Re: A new look at old NFS readdir() problems? Robert Haas <[email protected]> 2025-01-06 14:49 ` Re: Fwd: Re: A new look at old NFS readdir() problems? Tom Lane <[email protected]>
This inbox is served by agora; see mirroring instructions for how to clone and mirror all data and code used for this inbox