Re: Optimize LISTEN/NOTIFY

Hi Joel,

Thanks for the patch. After reviewing it, I got a few comments.

> On Sep 25, 2025, at 04:34, Joel Jacobson <joel@compiler.org> wrote:
> 
> 
> Curious to hear thoughts on this approach.
> 
> /Joel
> <0001-LISTEN-NOTIFY-make-the-latency-throughput-trade-off-.patch>


1.
```
--- a/src/include/utils/timeout.h
+++ b/src/include/utils/timeout.h
@@ -35,6 +35,7 @@ typedef enum TimeoutId
 	IDLE_SESSION_TIMEOUT,
 	IDLE_STATS_UPDATE_TIMEOUT,
 	CLIENT_CONNECTION_CHECK_TIMEOUT,
+	NOTIFY_DEFERRED_WAKEUP_TIMEOUT,
 	STARTUP_PROGRESS_TIMEOUT,
```

Can we define the new one after STARTUP_PROGRESS_TIMEOUT to try to preserve the existing enum value?

2.
```
--- a/src/backend/utils/misc/postgresql.conf.sample
+++ b/src/backend/utils/misc/postgresql.conf.sample
@@ -766,6 +766,7 @@ autovacuum_worker_slots = 16	# autovacuum worker slots to allocate
 #lock_timeout = 0				# in milliseconds, 0 is disabled
 #idle_in_transaction_session_timeout = 0	# in milliseconds, 0 is disabled
 #idle_session_timeout = 0			# in milliseconds, 0 is disabled
+#notify_latency_target = 0	# in milliseconds, 0 is disabled
 #bytea_output = 'hex'			# hex, escape
```

I think we should add one more table to make the comment to align with last line’s comment.

3.
```
 /* GUC parameters */
 bool		Trace_notify = false;
+int			notify_latency_target;
```

I know compiler will auto initiate notify_latency_target to 0. But all other global and static variables around are explicitly initiated, so it would look better to assign 0 to it, which just keeps coding style consistent.

4.
```
+	/*
+	 * Throttling check: if we were last active too recently, defer. This
+	 * check is safe without a lock because it's based on a backend-local
+	 * timestamp.
+	 */
+	if (notify_latency_target > 0 &&
+		!TimestampDifferenceExceeds(last_wakeup_start_time,
+									GetCurrentTimestamp(),
+									notify_latency_target))
+	{
+		/*
+		 * Too soon. We leave wakeup_pending_flag untouched (it must be true,
+		 * or we wouldn't have been signaled) to tell senders we are
+		 * intentionally delaying. Arm a timer to re-awaken and process the
+		 * backlog later.
+		 */
+		enable_timeout_after(NOTIFY_DEFERRED_WAKEUP_TIMEOUT,
+							 notify_latency_target);
+		return;
+	}
+
```

Should we avid duplicate timeout to be enabled? Now, whenever a duplicate notification is avoid, a new timeout is enabled. I think we can add another variable to remember if a timeout has been enabled.

Best regards,
--
Chao Li (Evan)
HighGo Software Co., Ltd.
https://www.highgo.com/