Automatic notification of top transaction IDs
(Re-sending this email, because the Commitfest app mistakenly [3]See Emails section in https://commitfest.postgresql.org/33/3198/
considered previous email [4]/messages/by-id/CABwTF4VS+HVm11XRE_Yv0vGmG=5kpYdx759RyJEp9F+fiLTU=Q@mail.gmail.com to be part of the old thread, whereas it
should not be considered that way)
I came across this thread [1]subject was: Re: Disallow cancellation of waiting for synchronous replication thread: /messages/by-id/C1F7905E-5DB2-497D-ABCC-E14D4DEE506C@yandex-team.ru to disallow canceling a transaction not
yet confirmed by a synchronous replica. I think my proposed patch
might help that case as well, hence adding all involved in that thread
to BCC, for one-time notification.
As mentioned in that thread, when sending a cancellation signal, the
client cannot be sure if the cancel signal was honored, and if the
transaction was cancelled successfully. In the attached patch, the
backend emits a NotificationResponse containing the current full
transaction id. It does so only if the relevant GUC is enabled, and
when the top-transaction is being assigned the ID.
This information can be useful to the client, when:
i) it wants to cancel a transaction _after_ issuing a COMMIT, and
ii) it wants to check the status of its transaction that it sent
COMMIT for, but never received a response (perhaps because the server
crashed).
Additionally, this information can be useful for middleware, like
Transaction Processing Monitors, which can now transparently (without
any change in application code) monitor the status of transactions (by
watching for the transaction status indicator in the ReadyForQuery
protocol message). They can use the transaction ID from the
NotificationResponse to open a watcher, and on seeing either an 'E' or
'I' payload in subsequent ReadyForQuery messages, close the watcher.
On server crash, or other adverse events, they can then use the
transaction IDs still being watched to check status of those
transactions, and take appropriate actions, e.g. retry any aborted
transactions.
We cannot use the elog() mechanism for this notification because it is
sensitive to the value of client_min_messages. Hence I used the NOTIFY
infrastructure for this message. I understand that this usage violates
some expectations as to how NOTIFY messages are supposed to behave
(see [2]At present, NotificationResponse can only be sent outside a transaction, and thus it will not occur in the middle of a command-response series, though it might occur just before ReadyForQuery. It is unwise to design frontend logic that assumes that, however. Good practice is to be able to accept NotificationResponse at any point in the protocol. below), but I think these are acceptable violations; open to
hearing if/why this might not be acceptable, and any possible
alternatives.
I'm not very familiar with the parallel workers infrastructure, so the
patch is missing any consideration for those.
Reviews welcome.
[1]: subject was: Re: Disallow cancellation of waiting for synchronous replication thread: /messages/by-id/C1F7905E-5DB2-497D-ABCC-E14D4DEE506C@yandex-team.ru
replication
thread: /messages/by-id/C1F7905E-5DB2-497D-ABCC-E14D4DEE506C@yandex-team.ru
[2]: At present, NotificationResponse can only be sent outside a transaction, and thus it will not occur in the middle of a command-response series, though it might occur just before ReadyForQuery. It is unwise to design frontend logic that assumes that, however. Good practice is to be able to accept NotificationResponse at any point in the protocol.
At present, NotificationResponse can only be sent outside a
transaction, and thus it will not occur in the middle of a
command-response series, though it might occur just before ReadyForQuery.
It is unwise to design frontend logic that assumes that, however.
Good practice is to be able to accept NotificationResponse at any
point in the protocol.
[3]: See Emails section in https://commitfest.postgresql.org/33/3198/
See Emails section in https://commitfest.postgresql.org/33/3198/
The email [4]/messages/by-id/CABwTF4VS+HVm11XRE_Yv0vGmG=5kpYdx759RyJEp9F+fiLTU=Q@mail.gmail.com is considered a continuation of a previous thread, _and_
the 'Latest attachment' entry points to a different email, even though
my email [4]/messages/by-id/CABwTF4VS+HVm11XRE_Yv0vGmG=5kpYdx759RyJEp9F+fiLTU=Q@mail.gmail.com contained a patch.
[4]: /messages/by-id/CABwTF4VS+HVm11XRE_Yv0vGmG=5kpYdx759RyJEp9F+fiLTU=Q@mail.gmail.com
Best regards,
--
Gurjeet Singh http://gurjeet.singh.im/
The proposed patch is attached.
Best regards,
--
Gurjeet Singh http://gurjeet.singh.im/
Show quoted text
On Wed, Jun 30, 2021 at 5:56 PM Gurjeet Singh <gurjeet@singh.im> wrote:
(Re-sending this email, because the Commitfest app mistakenly [3]
considered previous email [4] to be part of the old thread, whereas it
should not be considered that way)I came across this thread [1] to disallow canceling a transaction not
yet confirmed by a synchronous replica. I think my proposed patch
might help that case as well, hence adding all involved in that thread
to BCC, for one-time notification.As mentioned in that thread, when sending a cancellation signal, the
client cannot be sure if the cancel signal was honored, and if the
transaction was cancelled successfully. In the attached patch, the
backend emits a NotificationResponse containing the current full
transaction id. It does so only if the relevant GUC is enabled, and
when the top-transaction is being assigned the ID.This information can be useful to the client, when:
i) it wants to cancel a transaction _after_ issuing a COMMIT, and
ii) it wants to check the status of its transaction that it sent
COMMIT for, but never received a response (perhaps because the server
crashed).Additionally, this information can be useful for middleware, like
Transaction Processing Monitors, which can now transparently (without
any change in application code) monitor the status of transactions (by
watching for the transaction status indicator in the ReadyForQuery
protocol message). They can use the transaction ID from the
NotificationResponse to open a watcher, and on seeing either an 'E' or
'I' payload in subsequent ReadyForQuery messages, close the watcher.
On server crash, or other adverse events, they can then use the
transaction IDs still being watched to check status of those
transactions, and take appropriate actions, e.g. retry any aborted
transactions.We cannot use the elog() mechanism for this notification because it is
sensitive to the value of client_min_messages. Hence I used the NOTIFY
infrastructure for this message. I understand that this usage violates
some expectations as to how NOTIFY messages are supposed to behave
(see [2] below), but I think these are acceptable violations; open to
hearing if/why this might not be acceptable, and any possible
alternatives.I'm not very familiar with the parallel workers infrastructure, so the
patch is missing any consideration for those.Reviews welcome.
[1]: subject was: Re: Disallow cancellation of waiting for synchronous
replication
thread: /messages/by-id/C1F7905E-5DB2-497D-ABCC-E14D4DEE506C@yandex-team.ru[2]:
At present, NotificationResponse can only be sent outside a
transaction, and thus it will not occur in the middle of a
command-response series, though it might occur just before ReadyForQuery.
It is unwise to design frontend logic that assumes that, however.
Good practice is to be able to accept NotificationResponse at any
point in the protocol.[3]:
See Emails section in https://commitfest.postgresql.org/33/3198/
The email [4] is considered a continuation of a previous thread, _and_
the 'Latest attachment' entry points to a different email, even though
my email [4] contained a patch.[4]: /messages/by-id/CABwTF4VS+HVm11XRE_Yv0vGmG=5kpYdx759RyJEp9F+fiLTU=Q@mail.gmail.com
Best regards,
--
Gurjeet Singh http://gurjeet.singh.im/
Attachments:
notify_xid.patchapplication/octet-stream; name=notify_xid.patchDownload
diff --git a/doc/src/sgml/protocol.sgml b/doc/src/sgml/protocol.sgml
index bc2a2feb0b..fad49aba82 100644
--- a/doc/src/sgml/protocol.sgml
+++ b/doc/src/sgml/protocol.sgml
@@ -1315,6 +1315,17 @@ SELCT 1/0;<!-- this typo is intentional -->
channel name.
</para>
+ <para>
+ When the parameter <varname>listen_transaction_id</varname> is enabled, at
+ the begining of a transaction (not a subtransaction) the backend sends a
+ NotificationResponse message containing the transaction ID on the <literal>
+ _my_transaction_id<literal> channel. This may be useful for the frontend to
+ use after a crash-recovery, to inspect the state of their transaction from
+ before the crash; see <function>pg_xact_status()</function>. Unlike other
+ NotificationResponse messaegs, the process ID in this message is the same as
+ that of the current backend.
+ </para>
+
<note>
<para>
At present, NotificationResponse can only be sent outside a
diff --git a/src/backend/access/transam/xact.c b/src/backend/access/transam/xact.c
index 441445927e..e80dfb3a7d 100644
--- a/src/backend/access/transam/xact.c
+++ b/src/backend/access/transam/xact.c
@@ -68,6 +68,8 @@
#include "utils/timeout.h"
#include "utils/timestamp.h"
+extern char *FullTransactionIdToStr(FullTransactionId fxid);
+
/*
* User-tweakable parameters
*/
@@ -107,6 +109,7 @@ bool bsysscan = false;
* XactTopFullTransactionId stores the XID of our toplevel transaction, which
* will be the same as TopTransactionStateData.fullTransactionId in an
* ordinary backend; but in a parallel backend, which does not have the entire
+ * ordinary backend; but in a parallel backend, which does not have the entire
* transaction state, it will instead be copied from the backend that started
* the parallel operation.
*
@@ -714,6 +717,21 @@ AssignTransactionId(TransactionState s)
TopTransactionStateData.didLogXid = true;
}
}
+
+ // NOTIFY FrontEnd, if it wants to know the top transaction's ID.
+ if (!isSubXact && listen_transaction_id)
+ {
+ char *xidStr;
+
+ Assert(s->parent == NULL);
+
+ // Should we Assert(!IsParallelWorker()) here?
+
+ xidStr = FullTransactionIdToStr(s->fullTransactionId);
+
+ NotifyMyFrontEnd("_my_transaction_id", xidStr, MyProcPid);
+ pfree(xidStr);
+ }
}
/*
@@ -6005,7 +6023,7 @@ xact_redo(XLogReaderState *record)
}
else if (info == XLOG_XACT_COMMIT_PREPARED)
{
- xl_xact_commit *xlrec = (xl_xact_commit *) XLogRecGetData(record);
+ xl_xact_commit *xlrec = (xl_xact_commit *) XLogRecGetData(record);
xl_xact_parsed_commit parsed;
ParseCommitRecord(XLogRecGetInfo(record), xlrec, &parsed);
diff --git a/src/backend/utils/adt/xid.c b/src/backend/utils/adt/xid.c
index 24c1c93732..755cc53d36 100644
--- a/src/backend/utils/adt/xid.c
+++ b/src/backend/utils/adt/xid.c
@@ -161,13 +161,19 @@ xid8in(PG_FUNCTION_ARGS)
PG_RETURN_FULLTRANSACTIONID(FullTransactionIdFromU64(pg_strtouint64(str, NULL, 0)));
}
+char *
+FullTransactionIdToStr(FullTransactionId fxid)
+{
+ char *result = (char *) palloc(21);
+ snprintf(result, 21, UINT64_FORMAT, U64FromFullTransactionId(fxid));
+ return result;
+}
+
Datum
xid8out(PG_FUNCTION_ARGS)
{
FullTransactionId fxid = PG_GETARG_FULLTRANSACTIONID(0);
- char *result = (char *) palloc(21);
-
- snprintf(result, 21, UINT64_FORMAT, U64FromFullTransactionId(fxid));
+ char *result = FullTransactionIdToStr(fxid);
PG_RETURN_CSTRING(result);
}
diff --git a/src/backend/utils/misc/guc.c b/src/backend/utils/misc/guc.c
index 68b62d523d..1df14b9745 100644
--- a/src/backend/utils/misc/guc.c
+++ b/src/backend/utils/misc/guc.c
@@ -947,11 +947,21 @@ static const unit_conversion time_unit_conversion_table[] =
* variable_is_guc_list_quote() in src/bin/pg_dump/dumputils.c.
*/
+bool listen_transaction_id = false;
/******** option records follow ********/
static struct config_bool ConfigureNamesBool[] =
{
+ {
+ {"listen_transaction_id", PGC_USERSET, UNGROUPED,
+ gettext_noop("Emit top-level transaction ID on transaction start."),
+ NULL
+ },
+ &listen_transaction_id,
+ false,
+ NULL, NULL, NULL
+ },
{
{"enable_seqscan", PGC_USERSET, QUERY_TUNING_METHOD,
gettext_noop("Enables the planner's use of sequential-scan plans."),
diff --git a/src/include/utils/guc.h b/src/include/utils/guc.h
index a7c3a4958e..ed86a7a16d 100644
--- a/src/include/utils/guc.h
+++ b/src/include/utils/guc.h
@@ -278,6 +278,8 @@ extern int tcp_keepalives_interval;
extern int tcp_keepalives_count;
extern int tcp_user_timeout;
+extern bool listen_transaction_id;
+
#ifdef TRACE_SORT
extern bool trace_sort;
#endif
On Thu, Jul 1, 2021 at 6:41 AM Gurjeet Singh <gurjeet@singh.im> wrote:
The proposed patch is attached.
There is one compilation warning:
xid.c:165:1: warning: no previous prototype for
‘FullTransactionIdToStr’ [-Wmissing-prototypes]
165 | FullTransactionIdToStr(FullTransactionId fxid)
| ^~~~~~~~~~~~~~~~~~~~~~
There are few compilation issues in documentation:
/usr/bin/xmllint --path . --noout --valid postgres.sgml
protocol.sgml:1327: parser error : Opening and ending tag mismatch:
literal line 1322 and para
</para>
^
protocol.sgml:1339: parser error : Opening and ending tag mismatch:
literal line 1322 and sect2
</sect2>
^
protocol.sgml:1581: parser error : Opening and ending tag mismatch:
para line 1322 and sect1
</sect1>
^
protocol.sgml:7893: parser error : Opening and ending tag mismatch:
sect2 line 1322 and chapter
</chapter>
^
protocol.sgml:7894: parser error : chunk is not well balanced
^
postgres.sgml:253: parser error : Failure to process entity protocol
&protocol;
^
postgres.sgml:253: parser error : Entity 'protocol' not defined
&protocol;
^
Regards,
Vignesh
Greetings,
I simply tested it and it works well. But I got a compilation warning,
should we move the definition of function FullTransactionIdToStr to the
"transam.h"?
--
There is no royal road to learning.
HighGo Software Co.
The following review has been posted through the commitfest application:
make installcheck-world: tested, passed
Implements feature: tested, passed
Spec compliant: tested, passed
Documentation: tested, passed
Hello
This patch applies fine to master branch and the regression tests are passing.
Regarding the parallel worker case, the AssignTransactionId() function is already handling that and it will error out if IsParallelWorker() is true. In a normal case, this function is only called by the main backend, and the parallel workers will synchronize the transaction ID when they are spawned and they will not call this function anyway.
thank you
Cary Huang
----------------
HighGo Software Canada
www.highgo.ca
On 22 Jul 2021, at 07:28, vignesh C <vignesh21@gmail.com> wrote:
On Thu, Jul 1, 2021 at 6:41 AM Gurjeet Singh <gurjeet@singh.im> wrote:
The proposed patch is attached.
There is one compilation warning:
xid.c:165:1: warning: no previous prototype for
‘FullTransactionIdToStr’ [-Wmissing-prototypes]
165 | FullTransactionIdToStr(FullTransactionId fxid)
| ^~~~~~~~~~~~~~~~~~~~~~There are few compilation issues in documentation:
/usr/bin/xmllint --path . --noout --valid postgres.sgml
protocol.sgml:1327: parser error : Opening and ending tag mismatch:
literal line 1322 and para
</para>
^
protocol.sgml:1339: parser error : Opening and ending tag mismatch:
literal line 1322 and sect2
</sect2>
^
protocol.sgml:1581: parser error : Opening and ending tag mismatch:
para line 1322 and sect1
</sect1>
^
protocol.sgml:7893: parser error : Opening and ending tag mismatch:
sect2 line 1322 and chapter
</chapter>
^
protocol.sgml:7894: parser error : chunk is not well balanced^
postgres.sgml:253: parser error : Failure to process entity protocol
&protocol;
^
postgres.sgml:253: parser error : Entity 'protocol' not defined
&protocol;
^
The above compiler warning and documentation compilation errors haven't been
addressed still, so I'm marking this patch Returned with Feedback. Please feel
free to open a new entry for an updated patch.
--
Daniel Gustafsson https://vmware.com/
On Wed, Jun 30, 2021 at 8:56 PM Gurjeet Singh <gurjeet@singh.im> wrote:
As mentioned in that thread, when sending a cancellation signal, the
client cannot be sure if the cancel signal was honored, and if the
transaction was cancelled successfully. In the attached patch, the
backend emits a NotificationResponse containing the current full
transaction id. It does so only if the relevant GUC is enabled, and
when the top-transaction is being assigned the ID.
There's nothing to keep a client that wants this information from just
using SELECT txid_current() to get it, so this doesn't really seem
worth it to me. It's true that it could be convenient for someone not
to need to issue an SQL query to get the information and instead just
get it automatically, but I don't think that minor convenience is
enough to justify a new feature of this type.
Also, your 8-line documentation changes contains two spelling
mistakes, and you've used // comments which are not project style in
two places. It's a good idea to check over your patches for such
simple mistakes before submitting them.
--
Robert Haas
EDB: http://www.enterprisedb.com