Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Manually delete physical files for ENR entries when leaving scope. #450

Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions src/backend/access/transam/xact.c
Original file line number Diff line number Diff line change
Expand Up @@ -2356,7 +2356,7 @@ CommitTransaction(void)
* Other backends will observe the attendant catalog changes and not
* attempt to access affected files.
*/
smgrDoPendingDeletes(true);
smgrDoPendingDeletes(true, false);

/*
* Send out notification signals to other backends (and do other
Expand Down Expand Up @@ -2871,7 +2871,7 @@ AbortTransaction(void)
ResourceOwnerRelease(TopTransactionResourceOwner,
RESOURCE_RELEASE_AFTER_LOCKS,
false, true);
smgrDoPendingDeletes(false);
smgrDoPendingDeletes(false, false);

AtEOXact_GUC(false, 1);
AtEOXact_SPI(false);
Expand Down
72 changes: 68 additions & 4 deletions src/backend/catalog/storage.c
Original file line number Diff line number Diff line change
Expand Up @@ -74,6 +74,7 @@ typedef struct PendingRelSync
} PendingRelSync;

static PendingRelDelete *pendingDeletes = NULL; /* head of linked list */
static PendingRelDelete *pendingENRDeletes = NULL;
static HTAB *pendingSyncHash = NULL;


Expand Down Expand Up @@ -248,6 +249,59 @@ RelationDropStorage(Relation rel)
RelationCloseSmgr(rel);
}

/*
* ENRDropStorage
* Schedule unlinking of physical storage at transaction commit.
*
* This is copied from RelationDropStorage, with references to pendingDeletes
* changed to pendingENRDeletes.
*/
void
ENRDropStorage(Relation rel)
{
PendingRelDelete *pending;

/* Add the relation to the list of stuff to delete at commit */
pending = (PendingRelDelete *)
MemoryContextAlloc(TopMemoryContext, sizeof(PendingRelDelete));
pending->rlocator = rel->rd_locator;
pending->backend = rel->rd_backend;
pending->atCommit = true; /* delete if commit */
pending->nestLevel = GetCurrentTransactionNestLevel();
pending->next = pendingENRDeletes;
pendingENRDeletes = pending;

/*
* BBF Table Variables were not registered in pendingDeletes in RelationCreateStorage().
* This allows us to skip files from being unlinked automatically during AbortTransaction().
* However, we need to unlink the files on explicit DROP TABLE command regardless
* if the transaction state is committing or aborting.
*/
if (sql_dialect == SQL_DIALECT_TSQL && RelationIsBBFTableVariable(rel))
{
pending = (PendingRelDelete *)
MemoryContextAlloc(TopMemoryContext, sizeof(PendingRelDelete));
pending->rlocator = rel->rd_locator;
pending->backend = rel->rd_backend;
pending->atCommit = false;
pending->nestLevel = GetCurrentTransactionNestLevel();
pending->next = pendingENRDeletes;
pendingENRDeletes = pending;
}

/*
* NOTE: if the relation was created in this transaction, it will now be
* present in the pending-delete list twice, once with atCommit true and
* once with atCommit false. Hence, it will be physically deleted at end
* of xact in either case (and the other entry will be ignored by
* smgrDoPendingDeletes, so no error will occur). We could instead remove
* the existing list entry and delete the physical file immediately, but
* for now I'll keep the logic simple.
*/

RelationCloseSmgr(rel);
}

/*
* RelationPreserveStorage
* Mark a relation as not to be deleted after all.
Expand Down Expand Up @@ -664,9 +718,14 @@ RestorePendingSyncs(char *startAddress)
* no physical storage in any fork. In particular, it's possible that we're
* cleaning up an old temporary relation for which RemovePgTempFiles has
* already recovered the physical storage.
*
* Second Note: Since TSQL ENR entries (table variables and temp tables) need to
* be cleaned up at end of scope rather than end of transaction, we also maintain
* a separate list of pendingENRDeletes which can be separately processed by
* calling this function with forENR=true.
*/
void
smgrDoPendingDeletes(bool isCommit)
smgrDoPendingDeletes(bool isCommit, bool forENR)
{
int nestLevel = GetCurrentTransactionNestLevel();
PendingRelDelete *pending;
Expand All @@ -677,7 +736,7 @@ smgrDoPendingDeletes(bool isCommit)
SMgrRelation *srels = NULL;

prev = NULL;
for (pending = pendingDeletes; pending != NULL; pending = next)
for (pending = (forENR ? pendingENRDeletes : pendingDeletes); pending != NULL; pending = next)
{
next = pending->next;
if (pending->nestLevel < nestLevel)
Expand All @@ -691,7 +750,12 @@ smgrDoPendingDeletes(bool isCommit)
if (prev)
prev->next = next;
else
pendingDeletes = next;
{
if (forENR)
pendingENRDeletes = next;
else
pendingDeletes = next;
}
/* do deletion if called for */
if (pending->atCommit == isCommit)
{
Expand Down Expand Up @@ -970,7 +1034,7 @@ AtSubCommit_smgr(void)
void
AtSubAbort_smgr(void)
{
smgrDoPendingDeletes(false);
smgrDoPendingDeletes(false, false);
}

void
Expand Down
16 changes: 16 additions & 0 deletions src/backend/utils/misc/queryenvironment.c
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,7 @@
#include "catalog/pg_attrdef.h"
#include "catalog/pg_shdepend.h"
#include "catalog/pg_index_d.h"
#include "catalog/storage.h"
#include "parser/parser.h" /* only needed for GUC variables */
#include "utils/inval.h"
#include "utils/guc.h"
Expand Down Expand Up @@ -1543,6 +1544,17 @@ bool useTempOidBufferForOid(Oid relId)
&& get_ENR_withoid(currentQueryEnv, relId, ENR_TSQL_TEMP);
}

/*
* Helper function to drop physical storage for an ENR entry.
*/
static void
ENRDelete(Oid relid)
{
Relation rel = relation_open(relid, AccessExclusiveLock);
ENRDropStorage(rel);
relation_close(rel, NoLock);
}

/*
* Drop all the temp tables registered as ENR in the given query environment.
*/
Expand Down Expand Up @@ -1572,6 +1584,10 @@ ENRDropTempTables(QueryEnvironment *queryEnv)
object.objectSubId = 0;
object.objectId = enr->md.reliddesc;
add_exact_object_address(&object, objects);
/*
* Also register the physical file to be deleted at the end of scope.
*/
ENRDelete(enr->md.reliddesc);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why not delete the file right here rather than adding it to delete list and later calling smgrDoPendingDeletes. We can write a new function to delete a file in SMGR

Copy link
Contributor Author

@Sairakan Sairakan Sep 26, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Let me look into this. The original reasoning behind doing it this way is that I am not sure how much of the storage deletion needs to be copied, vs just immediately calling smgr_close() on the relation

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've implemented this suggestion - now the files are immediately cleaned up during this function, which has the added benefit of not needing to modify any other function signatures or anything else in the code.

}

/*
Expand Down
3 changes: 2 additions & 1 deletion src/include/catalog/storage.h
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@ extern SMgrRelation RelationCreateStorage(RelFileLocator rlocator,
char relpersistence,
bool register_delete);
extern void RelationDropStorage(Relation rel);
extern void ENRDropStorage(Relation rel);
extern void RelationPreserveStorage(RelFileLocator rlocator, bool atCommit);
extern void RelationPreTruncate(Relation rel);
extern void RelationTruncate(Relation rel, BlockNumber nblocks);
Expand All @@ -40,7 +41,7 @@ extern void RestorePendingSyncs(char *startAddress);
* These functions used to be in storage/smgr/smgr.c, which explains the
* naming
*/
extern void smgrDoPendingDeletes(bool isCommit);
extern void smgrDoPendingDeletes(bool isCommit, bool forENR);
extern void smgrDoPendingSyncs(bool isCommit, bool isParallelWorker);
extern int smgrGetPendingDeletes(bool forCommit, RelFileLocator **ptr);
extern void AtSubCommit_smgr(void);
Expand Down
Loading