review comments

Author: yananxin

src/backend/access/transam/xlog.c

@@ -838,6 +838,10 @@ XLogInsertRecord(XLogRecData *rdata,
 	 * but not force a recomputation.  (If doPageWrites was just turned off,
 	 * we could recompute the record without full pages, but we choose not to
 	 * bother.)
+	 *
+	 * However, if suppress_fpi is true, we skip the recomputation check
+	 * since we're deliberately suppressing full page images for this
+	 * record via the FPI control hook.
 	 */
 	if (RedoRecPtr != Insert->RedoRecPtr)
 	{
@@ -848,8 +852,8 @@ XLogInsertRecord(XLogRecData *rdata,
 
 	if (doPageWrites &&
 		(!prevDoPageWrites ||
-		(!force_disable_full_page_write &&
-			fpw_lsn != InvalidXLogRecPtr && fpw_lsn <= RedoRecPtr)))
+		 (!suppress_fpi &&
+		  fpw_lsn != InvalidXLogRecPtr && fpw_lsn <= RedoRecPtr)))
 	{
 		/*
 		 * Oops, some buffer now needs to be backed up that the caller didn't

src/backend/access/transam/xloginsert.c

@@ -94,11 +94,11 @@ int			max_replication_apply_lag;
 int			max_replication_flush_lag;
 int			max_replication_write_lag;
 
-/* NEON: Hook to control Full Page Image (FPI) writes */
-xlog_fpi_control_hook_type xlog_fpi_control_hook = NULL;
+/* NEON: Hook to determine if FPI should be suppressed for a WAL record */
+xlog_should_suppress_fpi_hook_type xlog_should_suppress_fpi_hook = NULL;
 
-/* NEON: Global flag to force disable FPI for current WAL record */
-bool force_disable_full_page_write = false;
+/* NEON: Global flag to suppress FPI for current WAL record */
+bool suppress_fpi = false;
 
 static registered_buffer *registered_buffers;
 static int	max_registered_buffers; /* allocated size */
@@ -523,14 +523,14 @@ XLogInsert(RmgrId rmid, uint8 info)
 		GetFullPageWriteInfo(&RedoRecPtr, &doPageWrites);
 
 		/*
-		 * NEON: Check if we should force disable FPI for this WAL record.
+		 * NEON: Check if we should suppress FPI for this WAL record.
 		 */
-		 force_disable_full_page_write = false;
-		 if (xlog_fpi_control_hook != NULL) {
-			 force_disable_full_page_write = xlog_fpi_control_hook(rmid);
-			 elog(DEBUG1, "FPI control hook called: rmid=%u, force_disable=%d, doPageWrites=%d",
-				 rmid, force_disable_full_page_write, doPageWrites);
-		 }
+		suppress_fpi = false;
+		if (xlog_should_suppress_fpi_hook != NULL) {
+			suppress_fpi = xlog_should_suppress_fpi_hook();
+			elog(DEBUG1, "FPI suppress hook called: suppress_fpi=%d, doPageWrites=%d",
+				suppress_fpi, doPageWrites);
+		}
 
 		rdt = XLogRecordAssemble(rmid, info, RedoRecPtr, doPageWrites,
 								 &fpw_lsn, &num_fpi, &topxid_included);
@@ -632,7 +632,7 @@ XLogRecordAssemble(RmgrId rmid, uint8 info,
 			 */
 			XLogRecPtr	page_lsn = PageGetLSN(regbuf->page);
 
-			if (force_disable_full_page_write)
+			if (suppress_fpi)
 				needs_backup = false;
 			else
 				needs_backup = (page_lsn <= RedoRecPtr);

src/include/access/xloginsert.h

@@ -42,16 +42,15 @@ extern int max_replication_apply_lag;
 extern int max_replication_flush_lag;
 extern int max_replication_write_lag;
 
-/* NEON: Hook to control Full Page Image (FPI) writes 
- * Returns true to DISABLE FPI, false to keep FPI enabled 
- * Parameters:
- *   rmid - Resource manager ID (e.g., RM_HEAP_ID, RM_BTREE_ID)
+/* NEON: Hook to determine if FPI should be suppressed for a WAL record
+ * Returns true to suppress FPI, false to allow FPI
+ * Applies to all resource managers for checkpoint-based FPI triggers.
  */
- typedef bool (*xlog_fpi_control_hook_type)(RmgrId rmid);
- extern PGDLLIMPORT xlog_fpi_control_hook_type xlog_fpi_control_hook;
- 
- /* NEON: Flag set per-record to force disable FPI (set by neon_should_disable_fpi hook) */
- extern bool force_disable_full_page_write;
+typedef bool (*xlog_should_suppress_fpi_hook_type)(void);
+extern PGDLLIMPORT xlog_should_suppress_fpi_hook_type xlog_should_suppress_fpi_hook;
+
+/* NEON: Flag set per-record to suppress FPI (set by xlog_should_suppress_fpi_hook) */
+extern bool suppress_fpi;
 
 /* prototypes for public functions in xloginsert.c: */
 extern void XLogBeginInsert(void);