From 7ec7db9d77fe732fd6867b9bf2843eda69ce9f59 Mon Sep 17 00:00:00 2001 From: Matthias van de Meent Date: Fri, 3 Jan 2025 20:25:44 +0100 Subject: [PATCH 1/4] Use timeout = max(0, difference) instead of min(0, difference) Using `min(0, ...)` causes us to fail to wait in most situations, so a lack of data would be a hot wait loop, which is bad. --- pgxn/neon/libpagestore.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pgxn/neon/libpagestore.c b/pgxn/neon/libpagestore.c index 88d0a5292bf7..6dfdedee3867 100644 --- a/pgxn/neon/libpagestore.c +++ b/pgxn/neon/libpagestore.c @@ -694,7 +694,7 @@ call_PQgetCopyData(shardno_t shard_no, char **buffer) WaitEvent event; long timeout; - timeout = Min(0, LOG_INTERVAL_US - INSTR_TIME_GET_MICROSEC(since_last_log)); + timeout = Max(0, LOG_INTERVAL_US - INSTR_TIME_GET_MICROSEC(since_last_log)); /* Sleep until there's something to do */ (void) WaitEventSetWait(shard->wes_read, timeout, &event, 1, From ca18a91075a0a3fea2b0a7ae571f76c1ff08db45 Mon Sep 17 00:00:00 2001 From: Matthias van de Meent Date: Fri, 3 Jan 2025 21:37:10 +0100 Subject: [PATCH 2/4] Provide milliseconds-based timeout in libpagestore While microseconds would be preferred, WaitEventSetWait only accepts a timeout in milliseconds. --- pgxn/neon/libpagestore.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/pgxn/neon/libpagestore.c b/pgxn/neon/libpagestore.c index 6dfdedee3867..0f635dbb4d73 100644 --- a/pgxn/neon/libpagestore.c +++ b/pgxn/neon/libpagestore.c @@ -680,7 +680,7 @@ call_PQgetCopyData(shardno_t shard_no, char **buffer) * but in the cases that take exceptionally long, it's useful to log the * exact timestamps. */ -#define LOG_INTERVAL_US UINT64CONST(10 * 1000000) +#define LOG_INTERVAL_MS UINT64CONST(10 * 1000) INSTR_TIME_SET_CURRENT(now); start_ts = last_log_ts = now; @@ -694,7 +694,7 @@ call_PQgetCopyData(shardno_t shard_no, char **buffer) WaitEvent event; long timeout; - timeout = Max(0, LOG_INTERVAL_US - INSTR_TIME_GET_MICROSEC(since_last_log)); + timeout = Max(0, LOG_INTERVAL_MS - INSTR_TIME_GET_MILLISEC(since_last_log)); /* Sleep until there's something to do */ (void) WaitEventSetWait(shard->wes_read, timeout, &event, 1, From 88b74b59d29c66b429d99e26076ec234d775f195 Mon Sep 17 00:00:00 2001 From: Matthias van de Meent Date: Fri, 3 Jan 2025 21:42:01 +0100 Subject: [PATCH 3/4] Update libpagestore.c GH editor was doing bad stuff. Adjust last location with LOG_INTERVAL_US, hopefully. --- pgxn/neon/libpagestore.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pgxn/neon/libpagestore.c b/pgxn/neon/libpagestore.c index 0f635dbb4d73..47f4025bab10 100644 --- a/pgxn/neon/libpagestore.c +++ b/pgxn/neon/libpagestore.c @@ -723,7 +723,7 @@ call_PQgetCopyData(shardno_t shard_no, char **buffer) INSTR_TIME_SET_CURRENT(now); since_last_log = now; INSTR_TIME_SUBTRACT(since_last_log, last_log_ts); - if (INSTR_TIME_GET_MICROSEC(since_last_log) >= LOG_INTERVAL_US) + if (INSTR_TIME_GET_MILLISEC(since_last_log) >= LOG_INTERVAL_MS) { since_start = now; INSTR_TIME_SUBTRACT(since_start, start_ts); From 881967146ff2021b5fc513d0fb61fef4ea7d7aa6 Mon Sep 17 00:00:00 2001 From: Matthias van de Meent Date: Mon, 6 Jan 2025 21:29:48 +0100 Subject: [PATCH 4/4] Update libpagestore.c Use int64 constant instead of uint64 constant, allowing values <0 and allowing MAX(0, ...) to work. --- pgxn/neon/libpagestore.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pgxn/neon/libpagestore.c b/pgxn/neon/libpagestore.c index 47f4025bab10..fa2a570ea88d 100644 --- a/pgxn/neon/libpagestore.c +++ b/pgxn/neon/libpagestore.c @@ -680,7 +680,7 @@ call_PQgetCopyData(shardno_t shard_no, char **buffer) * but in the cases that take exceptionally long, it's useful to log the * exact timestamps. */ -#define LOG_INTERVAL_MS UINT64CONST(10 * 1000) +#define LOG_INTERVAL_MS INT64CONST(10 * 1000) INSTR_TIME_SET_CURRENT(now); start_ts = last_log_ts = now;