apply worker misses closing partition leaves
Hackorum builds and tests every patch posted to the lists, not only commitfest submissions. This is Hackorum's own CI rather than the PostgreSQL project's, and it is still under testing - please report anything that looks wrong.
This thread has been committed, so CI has stopped here. Anything below is the last result it produced.
You can run a PostgreSQL built from this patch straight from Docker, with no checkout and no build:
docker run --rm -p 5432:5432 ghcr.io/hackorum-dev/postgres-patch:t253451psql -h localhost -U postgresBuilt from patchset v2 (message #2), August 26, 2026 at 06:13 AM.
Every patchset is also pushed to a branch of our PostgreSQL fork, so you can check out the same tree CI built. Without a PostgreSQL checkout:
git clone --branch t253451_2 https://github.com/hackorum-dev/postgres.gitIn a checkout you already have, add the fork once:
git remote add hackorum https://github.com/hackorum-dev/postgres.gitthen, for this patchset and every later one:
git fetch hackorum t253451_2 && git checkout t253451_2Patchset v2 (message #2) is on t253451_2
Dear hackers,
(CC: David, who is a committer for 39dcfda)
While working on the parallel apply, I found that 013_partition.pl raises WARNINGs
like [1]``` 2026-08-17 18:12:41.780 JST logical replication apply worker[150182] WARNING: resource was not closed: relation "tab1_2_2" 2026-08-17 18:12:41.780 JST logical replication apply worker[150182] CONTEXT: processing remote data for replication origin "pg_16438" during message type "COMMIT" in transaction 707, finished at 0/017E3E50 ```. Per my research it happened after the 39dcfda. The warning indicates the
partition leaf opened by the worker is not expressly closed.
IIUC, it can happen if partition leaves have after triggers. The apply worker
has to obtain ResultRelInfo for the relation in
finish_edata()->AfterTriggerEndQuery()->ExecGetTriggerResultRel(), and it firstly
checks the cached result in EState::es_opened_result_relations.
The commit 39dcfda made the condition for re-use the cache stricter (requires
matching rootRelInfo as well) and the apply worker could not match the condition.
In this case the relation would be opened but nobody close: the resource
owner finally handles it.
IIUC, the difference between normal and partition leaves is how they are cached
in es_tuple_routing_result_relations. Normal relations are stored with
ResultRelInfo::ri_RootResultRelInfo = NULL, and partition ones have valid value.
afterTriggerInvokeEvents() always passes rootRelInfo = NULL, so only leaves
miss the cache.
I'm not familiar with codes around triggers. But if we try to fix on logical
replication side, relations opened for triggers can be closed in finish_edata().
See attached.
[1]: ``` 2026-08-17 18:12:41.780 JST logical replication apply worker[150182] WARNING: resource was not closed: relation "tab1_2_2" 2026-08-17 18:12:41.780 JST logical replication apply worker[150182] CONTEXT: processing remote data for replication origin "pg_16438" during message type "COMMIT" in transaction 707, finished at 0/017E3E50 ```
```
2026-08-17 18:12:41.780 JST logical replication apply worker[150182] WARNING: resource was not closed: relation "tab1_2_2"
2026-08-17 18:12:41.780 JST logical replication apply worker[150182] CONTEXT: processing remote data for replication origin "pg_16438" during message type "COMMIT" in transaction 707, finished at 0/017E3E50
```
Best regards,
Hayato Kuroda
FUJITSU LIMITED
On Mon, 17 Aug 2026 at 23:53, Hayato Kuroda (Fujitsu)
<kuroda.hayato@fujitsu.com> wrote:
While working on the parallel apply, I found that 013_partition.pl raises WARNINGs
like [1]. Per my research it happened after the 39dcfda. The warning indicates the
partition leaf opened by the worker is not expressly closed.
I'm not familiar with codes around triggers. But if we try to fix on logical
replication side, relations opened for triggers can be closed in finish_edata().
See attached.
Thanks for the report and patch. I had a look and confirm that the
warnings appear without the patch and are gone with the patch.
However, I don't think it's a good idea to code the loop over
estate->es_trig_target_relations the way you have it. Accessing
EState's es_trig_target_relations field from worker.c to close open
resources feels wrong.
I modified your patch and added a new external function named
ExecCloseTrigTargetRelations() and modified finish_edata() to call
that instead. I also adjusted the 013_partition.pl test to grep the
logs to ensure no more resource leaks. I did wonder if something like
that should appear somewhere more generic so it applies to more tests,
but I didn't study the TAP test harness enough to understand where
that might go.
The leak message did change in b8bff07da, so I've attached two
patches, v16 for 15-16 and v20 for 17-master.
David
Attachments:
t253451_2v16-0001-Close-relations-opened-specifically-for-AFTER-tr.patchapplication/octet-stream; name=v16-0001-Close-relations-opened-specifically-for-AFTER-tr.patchDownload+32-5
v20-0001-Close-relations-opened-specifically-for-AFTER-tr.patchapplication/octet-stream; name=v20-0001-Close-relations-opened-specifically-for-AFTER-tr.patchDownload+32-5
Hi,
On Monday, August 24, 2026 1:48 PM David Rowley <dgrowleyml@gmail.com> wrote:
On Mon, 17 Aug 2026 at 23:53, Hayato Kuroda (Fujitsu)
<kuroda.hayato@fujitsu.com> wrote:While working on the parallel apply, I found that 013_partition.pl
raises WARNINGs like [1]. Per my research it happened after the
39dcfda. The warning indicates the partition leaf opened by the worker isnot expressly closed.
I'm not familiar with codes around triggers. But if we try to fix on
logical replication side, relations opened for triggers can be closed infinish_edata().
See attached.
Thanks for the report and patch. I had a look and confirm that the warnings
appear without the patch and are gone with the patch.
However, I don't think it's a good idea to code the loop over
estate->es_trig_target_relations the way you have it. Accessing
EState's es_trig_target_relations field from worker.c to close open resources
feels wrong.I modified your patch and added a new external function named
ExecCloseTrigTargetRelations() and modified finish_edata() to call that
instead. I also adjusted the 013_partition.pl test to grep the logs to ensure no
more resource leaks.
The fix looks good to me. I just noticed one nit in the test:
+ ok(!$node->log_contains(qr/resource was not closed/),
+ 'unclosed resources on ' . $node->name);I think the test message should be: "no unclosed resources on".
Best Regards,
Zhijie Hou
On Mon, 24 Aug 2026 at 22:33, Zhijie Hou (Fujitsu)
<houzj.fnst@fujitsu.com> wrote:
The fix looks good to me. I just noticed one nit in the test:
+ ok(!$node->log_contains(qr/resource was not closed/), + 'unclosed resources on ' . $node->name);I think the test message should be: "no unclosed resources on".
Sorry, I must be missing something here. You want the message that
complains when there *is* a resource leak WARNING say "no unclosed
resources on $server"?
Maybe if that message is confusing, then it might be better to write
"resource leak detected on $server". Is that any better?
David
On Monday, August 24, 2026 8:36 PM David Rowley <dgrowleyml@gmail.com> wrote:
On Mon, 24 Aug 2026 at 22:33, Zhijie Hou (Fujitsu) <houzj.fnst@fujitsu.com>
wrote:The fix looks good to me. I just noticed one nit in the test:
+ ok(!$node->log_contains(qr/resource was not closed/), + 'unclosed resources on ' . $node->name);I think the test message should be: "no unclosed resources on".
Sorry, I must be missing something here. You want the message that
complains when there *is* a resource leak WARNING say "no unclosed
resources on $server"?Maybe if that message is confusing, then it might be better to write "resource
leak detected on $server". Is that any better?
IIUC, the test is intended to ensure there is no resource leak.
ok(!$node->log_contains(qr/resource was not closed/),
Since the test verifies the absence of the warning message, the test message
should reflect that positive outcome(it's output when there is no such
warning), right ?
BTW, the v16 patch's message looks correct ("no unclosed resources on ")
Best Regards,
Zhijie Hou
Hi David,
Thanks for updating the patch. They basically LGTM, but I suggest
you to unify changes in 013_partition.pl.
Best regards,
Hayato Kuroda
FUJITSU LIMITED
On Tue, 25 Aug 2026 at 14:12, Hayato Kuroda (Fujitsu)
<kuroda.hayato@fujitsu.com> wrote:
Thanks for updating the patch. They basically LGTM, but I suggest
you to unify changes in 013_partition.pl.
OK, now pushed. I had misassumed the purpose of the ok() message and
had thought it was a failure text to display, rather than a test name.
I did reword it slightly as I didn't like the double negative of "no unclosed".
Thanks for the report and patch.
David