Skip to content

Use TopTransactionContext lifetime for PGSM local data#656

Draft
artemgavrilov wants to merge 1 commit into
mainfrom
memory-cleanup-idea
Draft

Use TopTransactionContext lifetime for PGSM local data#656
artemgavrilov wants to merge 1 commit into
mainfrom
memory-cleanup-idea

Conversation

@artemgavrilov
Copy link
Copy Markdown
Contributor

PG-0

Description

Links

Comment thread pg_stat_monitor.c
Comment on lines +394 to +396
if (MemoryContextIsValid(TopTransactionContext))
{
MemoryContextRegisterResetCallback(MessageContext, &mem_cxt_reset_callback);
MemoryContextRegisterResetCallback(TopTransactionContext, &mem_cxt_reset_callback);
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

It seems that TopTransactionContext always exists, no matter how query was executed.

Comment thread pg_stat_monitor.c
BufferUsage bufusage_start = pgBufferUsage;
WalUsage walusage;
WalUsage walusage_start = pgWalUsage;
pgsmEntry *entry = pgsm_create_hash_entry(0, queryId, NULL);
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

We cannot invoke this function here anymore, as some utility queries create and finish multiple transactions during its processing. Vacuum for example. But we still need username and application name that triggered the query. Without this SET application_name = new app will log new app as query tirgger.

Comment thread pg_stat_monitor.c
* because at this point app name may be changed by the query itself

/* Get the application name and set appid */
app_name_len = pg_get_application_name(app_name, APPLICATIONNAME_LEN);
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

This line updates global variable app_name, we shouldn't do it here as it affects what app name will be logged for SET application_name = new app query.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant