Hi,

Thanks for sharing the test case.
Unfortunately I donot have a powerful machine which would generate
such large number of spill files. But I created a patch as per your
suggestion in point(2) in thread [1]. Can you test with this patch on
your machine?

With this patch instead of calling unlink for every wal segment, we
are first reading the directory and filtering the files related to our
transaction and then unlinking those files.
You can apply the patch on your publisher source code and check. I
have created this patch on top of Postgres 15.6.

[1]: 
https://www.postgresql.org/message-id/1430556325.185731745.1731484846410.javamail.zim...@meteo.fr


Thanks and Regards,
Shlok Kyal

Thanks for the parch.

I tried it, but it does not compile.

Attached another version that I tested on PostgreSQL 17.2.

This is much worse: it deletes only 3 files / s !

The problem here, is that for a given xid, there is just one spill file to 
delete.
ReorderBufferRestoreCleanup is called over a million times, so for each call,
it has to open the directory and filter the one file to delete.

By the way, you don't need a powerful machine to test, as spill files are very 
small.

Marc
--- a/src/backend/replication/logical/reorderbuffer.c	2024-12-12 10:21:12.962202250 +0100
+++ b/src/backend/replication/logical/reorderbuffer.c	2024-12-12 11:40:24.217634527 +0100
@@ -4567,27 +4567,35 @@
 static void
 ReorderBufferRestoreCleanup(ReorderBuffer *rb, ReorderBufferTXN *txn)
 {
-	XLogSegNo	first;
-	XLogSegNo	cur;
-	XLogSegNo	last;
+	DIR *spill_dir;
+	struct dirent *spill_de;
+	char path[MAXPGPATH];
+	char spill_path[MAXPGPATH];
+	char file_filter[MAXPGPATH];
+	int len_filter;
+ 
+	snprintf(spill_path, MAXPGPATH, "pg_replslot/%s", 
+			NameStr(MyReplicationSlot->data.name));
 
-	Assert(txn->first_lsn != InvalidXLogRecPtr);
-	Assert(txn->final_lsn != InvalidXLogRecPtr);
-
-	XLByteToSeg(txn->first_lsn, first, wal_segment_size);
-	XLByteToSeg(txn->final_lsn, last, wal_segment_size);
-
-	/* iterate over all possible filenames, and delete them */
-	for (cur = first; cur <= last; cur++)
-	{
-		char		path[MAXPGPATH];
-
-		ReorderBufferSerializedPath(path, MyReplicationSlot, txn->xid, cur);
-		if (unlink(path) != 0 && errno != ENOENT)
-			ereport(ERROR,
-					(errcode_for_file_access(),
-					 errmsg("could not remove file \"%s\": %m", path)));
-	}
+ 	snprintf(file_filter, sizeof(file_filter), "xid-%u-", txn->xid);
+	len_filter = strlen(file_filter);
+ 
+	spill_dir = AllocateDir(spill_path);
+	while ((spill_de = ReadDirExtended(spill_dir, spill_path, INFO)) != NULL)
+ 	{
+		/* only look at names that can be ours */
+		if (strncmp(spill_de->d_name, file_filter, len_filter) == 0)
+		{
+			snprintf(path, sizeof(path), "%s/%s", spill_path,
+					spill_de->d_name);
+ 
+			if (unlink(path) != 0)
+				ereport(ERROR,
+						(errcode_for_file_access(),
+						 errmsg("could not remove file \"%s\": %m", path)));
+		}
+ 	}
+	FreeDir(spill_dir);
 }
 
 /*

Reply via email to