public inbox for [email protected]
help / color / mirror / Atom feedFrom: Kyotaro Horiguchi <[email protected]>
To: [email protected]
To: [email protected]
Subject: Re: BUG #18354: Aborted transaction aborted during cleanup when temp_file_limit exceeded
Date: Thu, 22 Feb 2024 11:46:00 +0900 (JST)
Message-ID: <[email protected]> (raw)
In-Reply-To: <[email protected]>
References: <[email protected]>
At Wed, 21 Feb 2024 12:00:01 +0000, PG Bug reporting form <[email protected]> wrote in
> triggers two errors, a warning, and an assertion failure:
> ERROR: temporary file size exceeds temp_file_limit (100kB)
> WARNING: AbortTransaction while in ABORT state
> ERROR: temporary file size exceeds temp_file_limit (100kB)
> server closed the connection unexpectedly
...
> For the second error:
> 2024-02-21 11:40:07.001 UTC|law|regression|65d5e116.13cfcb|ERROR: temporary
> file size exceeds temp_file_limit (100kB)
> 2024-02-21 11:40:07.001 UTC|law|regression|65d5e116.13cfcb|BACKTRACE:
> FileWrite at fd.c:2183:5
> BufFileDumpBuffer at buffile.c:537:18
> BufFileFlush at buffile.c:723:3
> BufFileClose at buffile.c:419:9
> tuplestore_end at tuplestore.c:459:5
> MemoryContextSwitchTo at palloc.h:142:23
> (inlined by) PortalDrop at portalmem.c:587:3
> AtCleanup_Portals at portalmem.c:907:3
Therefore, BufFileClose should not flush the content during error
handling. In the first place, tuplestore doesn't need to flush the
underlying files in _end and _clear. In this case, I would choose to
change the general behavior of tuplestore. The attached PoC patch
fixes the issue for me. It introduces a new "extended" function to
control flushing, avoiding the addition of an unnatural parameter to
BufFileClose. I suspect that it is usable in some other places, but I
haven't checked that.
regards.
--
Kyotaro Horiguchi
NTT Open Source Software Center
Attachments:
[text/x-patch] don_t_flush_tuplestore_at_end.diff (2.0K, 2-don_t_flush_tuplestore_at_end.diff)
download | inline diff:
diff --git a/src/backend/storage/file/buffile.c b/src/backend/storage/file/buffile.c
index 5315d8a714..9dae2e512f 100644
--- a/src/backend/storage/file/buffile.c
+++ b/src/backend/storage/file/buffile.c
@@ -410,11 +410,21 @@ BufFileExportFileSet(BufFile *file)
*/
void
BufFileClose(BufFile *file)
+{
+ BufFileCloseExtended(file, true);
+}
+
+/*
+ * Close a BufFile with specifying wheter the content is flushed out
+ */
+void
+BufFileCloseExtended(BufFile *file, bool flush)
{
int i;
- /* flush any unwritten data */
- BufFileFlush(file);
+ /* flush any unwritten data if requested */
+ if (flush)
+ BufFileFlush(file);
/* close and delete the underlying file(s) */
for (i = 0; i < file->numFiles; i++)
FileClose(file->files[i]);
diff --git a/src/backend/utils/sort/tuplestore.c b/src/backend/utils/sort/tuplestore.c
index 947a868e56..08c1bcced9 100644
--- a/src/backend/utils/sort/tuplestore.c
+++ b/src/backend/utils/sort/tuplestore.c
@@ -421,7 +421,7 @@ tuplestore_clear(Tuplestorestate *state)
TSReadPointer *readptr;
if (state->myfile)
- BufFileClose(state->myfile);
+ BufFileCloseExtended(state->myfile, false);
state->myfile = NULL;
if (state->memtuples)
{
@@ -455,7 +455,7 @@ tuplestore_end(Tuplestorestate *state)
int i;
if (state->myfile)
- BufFileClose(state->myfile);
+ BufFileCloseExtended(state->myfile, false);
if (state->memtuples)
{
for (i = state->memtupdeleted; i < state->memtupcount; i++)
diff --git a/src/include/storage/buffile.h b/src/include/storage/buffile.h
index 5f6d7c8e3f..d4796f9cb4 100644
--- a/src/include/storage/buffile.h
+++ b/src/include/storage/buffile.h
@@ -38,6 +38,7 @@ typedef struct BufFile BufFile;
extern BufFile *BufFileCreateTemp(bool interXact);
extern void BufFileClose(BufFile *file);
+extern void BufFileCloseExtended(BufFile *file, bool flush);
extern pg_nodiscard size_t BufFileRead(BufFile *file, void *ptr, size_t size);
extern void BufFileReadExact(BufFile *file, void *ptr, size_t size);
extern size_t BufFileReadMaybeEOF(BufFile *file, void *ptr, size_t size, bool eofOK);
reply
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Reply to all the recipients using the --to and --cc options:
reply via email
To: [email protected]
Cc: [email protected], [email protected], [email protected]
Subject: Re: BUG #18354: Aborted transaction aborted during cleanup when temp_file_limit exceeded
In-Reply-To: <[email protected]>
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
This inbox is served by agora; see mirroring instructions
for how to clone and mirror all data and code used for this inbox