Remove unnecessary smgrimmedsync() when creating unlogged table.

hlinnaka · hlinnaka · commit 18724af9e83b · 2023-09-15T17:29:37.000+03:00
This became safe after commit 4b4798e. The smgrcreate() call will now register the segment for syncing at the next checkpoint, so we don't need to sync it here. If a checkpoint happens before the creation is WAL-logged, the records will be replayed when starting recovery from the checkpoint. If a checkpoint happens after the WAL logging, the checkpoint will fsync() it. In the passing, clarify a comment in smgrDoPendingSyncs(). Discussion: https://www.postgresql.org/message-id/6e5bbc08-cdfc-b2b3-9e23-1a914b9850a9%40iki.fi Reviewed-by: Robert Haas
diff --git a/src/backend/access/heap/heapam_handler.c b/src/backend/access/heap/heapam_handler.c
@@ -606,12 +606,9 @@ heapam_relation_set_new_filelocator(Relation rel,
 
 	/*
 	 * If required, set up an init fork for an unlogged table so that it can
-	 * be correctly reinitialized on restart.  An immediate sync is required
-	 * even if the page has been logged, because the write did not go through
-	 * shared_buffers and therefore a concurrent checkpoint may have moved the
-	 * redo pointer past our xlog record.  Recovery may as well remove it
-	 * while replaying, for example, XLOG_DBASE_CREATE* or XLOG_TBLSPC_CREATE
-	 * record. Therefore, logging is necessary even if wal_level=minimal.
+	 * be correctly reinitialized on restart.  Recovery may remove it while
+	 * replaying, for example, an XLOG_DBASE_CREATE* or XLOG_TBLSPC_CREATE
+	 * record.  Therefore, logging is necessary even if wal_level=minimal.
 	 */
 	if (persistence == RELPERSISTENCE_UNLOGGED)
 	{
@@ -620,7 +617,6 @@ heapam_relation_set_new_filelocator(Relation rel,
 			   rel->rd_rel->relkind == RELKIND_TOASTVALUE);
 		smgrcreate(srel, INIT_FORKNUM, false);
 		log_smgrcreate(newrlocator, INIT_FORKNUM);
-		smgrimmedsync(srel, INIT_FORKNUM);
 	}
 
 	smgrclose(srel);
diff --git a/src/backend/catalog/storage.c b/src/backend/catalog/storage.c
@@ -764,7 +764,7 @@ smgrDoPendingSyncs(bool isCommit, bool isParallelWorker)
 		/*
 		 * We emit newpage WAL records for smaller relations.
 		 *
-		 * Small WAL records have a chance to be emitted along with other
+		 * Small WAL records have a chance to be flushed along with other
 		 * backends' WAL records.  We emit WAL records instead of syncing for
 		 * files that are smaller than a certain threshold, expecting faster
 		 * commit.  The threshold is defined by the GUC wal_skip_threshold.

Original file line number	Diff line number	Diff line change
`@@ -764,7 +764,7 @@ smgrDoPendingSyncs(bool isCommit, bool isParallelWorker)`
`764`	`764`	`/*`
`765`	`765`	`* We emit newpage WAL records for smaller relations.`
`766`	`766`	`*`
`767`		`- * Small WAL records have a chance to be emitted along with other`
	`767`	`+ * Small WAL records have a chance to be flushed along with other`
`768`	`768`	`* backends' WAL records. We emit WAL records instead of syncing for`
`769`	`769`	`* files that are smaller than a certain threshold, expecting faster`
`770`	`770`	`* commit. The threshold is defined by the GUC wal_skip_threshold.`