agora inbox for pgsql-hackers@postgresql.orghelp / color / mirror / Atom feed
[PATCH v2 1/1] reduce archiving overhead 2+ messages / 2 participants [nested] [flat]
* [PATCH v2 1/1] reduce archiving overhead @ 2022-02-22 19:39 Nathan Bossart <nathandbossart@gmail.com> 0 siblings, 0 replies; 2+ messages in thread From: Nathan Bossart @ 2022-02-22 19:39 UTC (permalink / raw) --- src/backend/access/transam/xlog.c | 10 ++++++---- src/backend/postmaster/pgarch.c | 14 +++++++++++++- 2 files changed, 19 insertions(+), 5 deletions(-) diff --git a/src/backend/access/transam/xlog.c b/src/backend/access/transam/xlog.c index 0d2bd7a357..2ad047052f 100644 --- a/src/backend/access/transam/xlog.c +++ b/src/backend/access/transam/xlog.c @@ -3334,13 +3334,15 @@ InstallXLogFileSegment(XLogSegNo *segno, char *tmppath, } /* - * Perform the rename using link if available, paranoidly trying to avoid - * overwriting an existing file (there shouldn't be one). + * Perform the rename. Ideally, we'd use link() and unlink() to avoid + * overwriting an existing file (there shouldn't be one). However, that + * approach opens up the possibility that pg_wal will contain multiple hard + * links to the same WAL file after a crash. */ - if (durable_rename_excl(tmppath, path, LOG) != 0) + if (durable_rename(tmppath, path, LOG) != 0) { LWLockRelease(ControlFileLock); - /* durable_rename_excl already emitted log message */ + /* durable_rename already emitted log message */ return false; } diff --git a/src/backend/postmaster/pgarch.c b/src/backend/postmaster/pgarch.c index d916ed39a8..641297e9f5 100644 --- a/src/backend/postmaster/pgarch.c +++ b/src/backend/postmaster/pgarch.c @@ -746,7 +746,19 @@ pgarch_archiveDone(char *xlog) StatusFilePath(rlogready, xlog, ".ready"); StatusFilePath(rlogdone, xlog, ".done"); - (void) durable_rename(rlogready, rlogdone, WARNING); + + /* + * To avoid extra overhead, we don't durably rename the .ready file to + * .done. Archive commands and libraries must already gracefully handle + * attempts to re-archive files (e.g., if the server crashes just before + * this function is called), so it should be okay if the .ready file + * reappears after a crash. + */ + if (rename(rlogready, rlogdone) < 0) + ereport(WARNING, + (errcode_for_file_access(), + errmsg("could not rename file \"%s\" to \"%s\": %m", + rlogready, rlogdone))); } -- 2.25.1 --2oS5YaxWCcQjTEyO-- ^ permalink raw reply [nested|flat] 2+ messages in thread
* [PATCH v2 1/2] Do not lock tables in get_tables_to_repack(). @ 2026-06-16 06:49 ChangAo Chen <cca5507@qq.com> 0 siblings, 0 replies; 2+ messages in thread From: ChangAo Chen @ 2026-06-16 06:49 UTC (permalink / raw) When doing a whole database repack, we build a list of repackable tables and take a lock on them to prevent concurrent drops. But concurrent drops can always happen after we build the list because we process each table in a separate transaction. The ConditionalLockRelationOid() also makes the default behavior like SKIP_LOCKED, which is unexpected. To remove the locks, we need to make repack_is_permitted_for_relation() handles concurrent drops correctly: it should not report an error when failing to search the syscache in pg_class_aclcheck(). Use pg_class_aclcheck_ext() instead to detect a concurrent drop. Also check the return value of get_rel_name(). While at it, replace relation_close() with table_close() to match the table_open(). --- src/backend/commands/repack.c | 67 ++++++++++------------------------- 1 file changed, 19 insertions(+), 48 deletions(-) diff --git a/src/backend/commands/repack.c b/src/backend/commands/repack.c index faa07d1a118..2879c8af574 100644 --- a/src/backend/commands/repack.c +++ b/src/backend/commands/repack.c @@ -2169,22 +2169,9 @@ get_tables_to_repack(RepackCommand cmd, bool usingindex, MemoryContext permcxt) index = (Form_pg_index) GETSTRUCT(tuple); - /* - * Try to obtain a light lock on the index's table, to ensure it - * doesn't go away while we collect the list. If we cannot, just - * disregard it. Be sure to release this if we ultimately decide - * not to process the table! - */ - if (!ConditionalLockRelationOid(index->indrelid, AccessShareLock)) - continue; - - /* Verify that the table still exists; skip if not */ classtup = SearchSysCache1(RELOID, ObjectIdGetDatum(index->indrelid)); if (!HeapTupleIsValid(classtup)) - { - UnlockRelationOid(index->indrelid, AccessShareLock); continue; - } classForm = (Form_pg_class) GETSTRUCT(classtup); /* Skip temp relations belonging to other sessions */ @@ -2192,7 +2179,6 @@ get_tables_to_repack(RepackCommand cmd, bool usingindex, MemoryContext permcxt) !isTempOrTempToastNamespace(classForm->relnamespace)) { ReleaseSysCache(classtup); - UnlockRelationOid(index->indrelid, AccessShareLock); continue; } @@ -2201,10 +2187,7 @@ get_tables_to_repack(RepackCommand cmd, bool usingindex, MemoryContext permcxt) /* noisily skip rels which the user can't process */ if (!repack_is_permitted_for_relation(cmd, index->indrelid, GetUserId())) - { - UnlockRelationOid(index->indrelid, AccessShareLock); continue; - } /* Use a permanent memory context for the result list */ oldcxt = MemoryContextSwitchTo(permcxt); @@ -2228,45 +2211,20 @@ get_tables_to_repack(RepackCommand cmd, bool usingindex, MemoryContext permcxt) class = (Form_pg_class) GETSTRUCT(tuple); - /* - * Try to obtain a light lock on the table, to ensure it doesn't - * go away while we collect the list. If we cannot, just - * disregard the table. Be sure to release this if we ultimately - * decide not to process the table! - */ - if (!ConditionalLockRelationOid(class->oid, AccessShareLock)) - continue; - - /* Verify that the table still exists */ - if (!SearchSysCacheExists1(RELOID, ObjectIdGetDatum(class->oid))) - { - UnlockRelationOid(class->oid, AccessShareLock); - continue; - } - /* Can only process plain tables and matviews */ if (class->relkind != RELKIND_RELATION && class->relkind != RELKIND_MATVIEW) - { - UnlockRelationOid(class->oid, AccessShareLock); continue; - } /* Skip temp relations belonging to other sessions */ if (class->relpersistence == RELPERSISTENCE_TEMP && !isTempOrTempToastNamespace(class->relnamespace)) - { - UnlockRelationOid(class->oid, AccessShareLock); continue; - } /* noisily skip rels which the user can't process */ if (!repack_is_permitted_for_relation(cmd, class->oid, GetUserId())) - { - UnlockRelationOid(class->oid, AccessShareLock); continue; - } /* Use a permanent memory context for the result list */ oldcxt = MemoryContextSwitchTo(permcxt); @@ -2279,7 +2237,7 @@ get_tables_to_repack(RepackCommand cmd, bool usingindex, MemoryContext permcxt) } table_endscan(scan); - relation_close(catalog, AccessShareLock); + table_close(catalog, AccessShareLock); return rtcs; } @@ -2357,15 +2315,28 @@ get_tables_to_repack_partitioned(RepackCommand cmd, Oid relid, static bool repack_is_permitted_for_relation(RepackCommand cmd, Oid relid, Oid userid) { + bool is_missing = false; + Assert(cmd == REPACK_COMMAND_CLUSTER || cmd == REPACK_COMMAND_REPACK); - if (pg_class_aclcheck(relid, userid, ACL_MAINTAIN) == ACLCHECK_OK) + if (pg_class_aclcheck_ext(relid, userid, ACL_MAINTAIN, &is_missing) == ACLCHECK_OK) return true; - ereport(WARNING, - errmsg("permission denied to execute %s on \"%s\", skipping it", - RepackCommandAsString(cmd), - get_rel_name(relid))); + /* Report a warning if the relation still exists. */ + if (!is_missing) + { + char *relname; + + relname = get_rel_name(relid); + if (relname != NULL) + { + ereport(WARNING, + errmsg("permission denied to execute %s on \"%s\", skipping it", + RepackCommandAsString(cmd), relname)); + + pfree(relname); + } + } return false; } -- 2.47.3 --op6mnexl7cn72cto Content-Type: text/x-diff; charset=utf-8 Content-Disposition: attachment; filename=v2-0002-fixups.patch ^ permalink raw reply [nested|flat] 2+ messages in thread
end of thread, other threads:[~2026-06-16 06:49 UTC | newest] Thread overview: 2+ messages (download: mbox mbox.gz follow: Atom feed) -- links below jump to the message on this page -- 2022-02-22 19:39 [PATCH v2 1/1] reduce archiving overhead Nathan Bossart <nathandbossart@gmail.com> 2026-06-16 06:49 [PATCH v2 1/2] Do not lock tables in get_tables_to_repack(). ChangAo Chen <cca5507@qq.com>
This inbox is served by agora; see mirroring instructions for how to clone and mirror all data and code used for this inbox