Do we need to handle orphaned prepared transactions in the server?

First and foremost is to define what an orphaned transaction is. At
this stage, I believe any prepared transaction that has been there
for more than X time may be considered as an orphan. X may be defined
as an integer in seconds (a GUC perhaps). May be there are better
ways to define this. Please feel free to chime in.

What about specifying a timeout when starting the prepared transaction?

I can imagine situations where a timeout of hours might be needed/anticipated
(e.g. really slow external systems) and situations where the developer
knows that the other systems are never slower than a few seconds.

Something like:

prepare transaction 42 timeout interval '2 days';

or

prepare transaction 42 timeout interval '30 second';

Or maybe even with a fixed timestamp instead of an interval?

prepare transaction 42 timeout timestamp '2020-01-30 14:00:00';

Thomas

On Wed, 22 Jan 2020 at 09:02, Hamid Akhtar <hamid.akhtar@gmail.com> wrote:

At this stage, I'm not sure of the scale of changes this will require, however, I wanted to get an understanding and consensus on whether (a) this is something we should work on, and (b) whether an approach to implementing a timeout makes sense.

Please feel free to share your thoughts here.

The intended use case of two phase transactions is ensuring atomic
durability of transactions across multiple database systems. This
necessarily means that there needs to be a failure tolerant agent that
ensures there is consensus about the status of the transaction and
then executes that consensus across all systems. In other words, there
needs to be a transaction manager for prepared statements to actually
fulfil their purpose. Therefore I think that unilaterally timing out
prepared statements is just shifting the consequences of a broken
client from availability to durability. But if durability was never a
concern, why is the client even using prepared statements?

Citing the documentation:

PREPARE TRANSACTION is not intended for use in applications or interactive sessions. Its purpose is to allow an external transaction manager to perform atomic global transactions across multiple databases or other transactional resources. Unless you're writing a transaction manager, you probably shouldn't be using PREPARE TRANSACTION.

Regards,
Ants Aasma

On Wed, 22 Jan 2020 at 16:45, Ants Aasma <ants@cybertec.at> wrote:

The intended use case of two phase transactions is ensuring atomic
durability of transactions across multiple database systems.

Exactly. I was trying to find a good way to say this.

It doesn't make much sense to embed a 2PC resolver in Pg unless it's
an XA coordinator or similar. And generally it doesn't make sense for
the distributed transaction coordinator to reside alongside one of the
datasources being managed anyway, especially where failover and HA are
in the picture.

I *can* see it being useful, albeit rather heavyweight, to implement
an XA coordinator on top of PostgreSQL. Mostly for HA and replication
reasons. But generally you'd use postgres instances for the HA
coordinator and the DB(s) in which 2PC txns are being managed. While
you could run them in the same instance it'd probably mostly be for
toy-scale PoC/demo/testing use.

So I don't really see the point of doing anything with 2PC xacts
within Pg proper. It's the job of the app that prepares the 2PC xacts,
and if that app is unable to resolve them for some reason there's no
generally-correct action to take without administrator action.

Craig Ringer <craig@2ndquadrant.com> writes:

So I don't really see the point of doing anything with 2PC xacts
within Pg proper. It's the job of the app that prepares the 2PC xacts,
and if that app is unable to resolve them for some reason there's no
generally-correct action to take without administrator action.

Right. It's the XA transaction manager's job not to forget uncommitted
transactions. Reasoning as though no TM exists is not only not very
relevant, but it might lead you to put in features that actually
make the TM's job harder. In particular, a timeout (or any other
mechanism that leads PG to abort or commit a prepared transaction
of its own accord) does that.

Or another way to put it: the fundamental premise of a prepared
transaction is that it will be possible to commit it on-demand with
extremely low chance of failure. Designing in a reason why we'd
fail to be able to do that would be an anti-feature.

regards, tom lane

Tom Lane schrieb am 22.01.2020 um 16:05:

Craig Ringer <craig@2ndquadrant.com> writes:

So I don't really see the point of doing anything with 2PC xacts
within Pg proper. It's the job of the app that prepares the 2PC xacts,
and if that app is unable to resolve them for some reason there's no
generally-correct action to take without administrator action.

Right. It's the XA transaction manager's job not to forget uncommitted
transactions. Reasoning as though no TM exists is not only not very
relevant, but it might lead you to put in features that actually
make the TM's job harder. In particular, a timeout (or any other
mechanism that leads PG to abort or commit a prepared transaction
of its own accord) does that.

Or another way to put it: the fundamental premise of a prepared
transaction is that it will be possible to commit it on-demand with
extremely low chance of failure. Designing in a reason why we'd
fail to be able to do that would be an anti-feature.

That's a fair point, but the reality is that not all XA transaction managers
do a good job with that.

Having somthing on the database side that can handle that in
exceptional cases would be very welcome.

(In Oracle you can't sometimes even run DML on tables where you have orphaned
XA transactions - which is extremely annoying, because by default
only the DBA can clean that up)

Thomas

Thomas Kellerer <shammat@gmx.net> writes:

Tom Lane schrieb am 22.01.2020 um 16:05:

Right. It's the XA transaction manager's job not to forget uncommitted
transactions. Reasoning as though no TM exists is not only not very
relevant, but it might lead you to put in features that actually
make the TM's job harder. In particular, a timeout (or any other
mechanism that leads PG to abort or commit a prepared transaction
of its own accord) does that.

That's a fair point, but the reality is that not all XA transaction managers
do a good job with that.

If you've got a crappy XA manager, you should get a better one, not
ask us to put in features that make PG unsafe to use with well-designed
XA managers.

regards, tom lane

On Wed, Jan 22, 2020 at 10:22:21AM -0500, Tom Lane wrote:

Thomas Kellerer <shammat@gmx.net> writes:

Tom Lane schrieb am 22.01.2020 um 16:05:

Right. It's the XA transaction manager's job not to forget uncommitted
transactions. Reasoning as though no TM exists is not only not very
relevant, but it might lead you to put in features that actually
make the TM's job harder. In particular, a timeout (or any other
mechanism that leads PG to abort or commit a prepared transaction
of its own accord) does that.

That's a fair point, but the reality is that not all XA transaction managers
do a good job with that.

If you've got a crappy XA manager, you should get a better one, not
ask us to put in features that make PG unsafe to use with well-designed
XA managers.

I think the big question is whether we want to make active prepared
transactions more visible to administrators, either during server start
or idle duration.

--
Bruce Momjian <bruce@momjian.us> http://momjian.us
EnterpriseDB http://enterprisedb.com

+ As you are, so once was I.  As I am, so you will be. +
+                      Ancient Roman grave inscription +

Bruce Momjian <bruce@momjian.us> writes:

I think the big question is whether we want to make active prepared
transactions more visible to administrators, either during server start
or idle duration.

There's already the pg_prepared_xacts view ...

regards, tom lane

On Wed, 22 Jan 2020 at 23:22, Tom Lane <tgl@sss.pgh.pa.us> wrote:

Thomas Kellerer <shammat@gmx.net> writes:

Tom Lane schrieb am 22.01.2020 um 16:05:

Right. It's the XA transaction manager's job not to forget uncommitted
transactions. Reasoning as though no TM exists is not only not very
relevant, but it might lead you to put in features that actually
make the TM's job harder. In particular, a timeout (or any other
mechanism that leads PG to abort or commit a prepared transaction
of its own accord) does that.

That's a fair point, but the reality is that not all XA transaction managers
do a good job with that.

If you've got a crappy XA manager, you should get a better one, not
ask us to put in features that make PG unsafe to use with well-designed
XA managers.

Agreed. Or use some bespoke script that does the cleanup that you
think is appropriate for your particular environment and set of bugs.

--
Craig Ringer http://www.2ndQuadrant.com/
2ndQuadrant - PostgreSQL Solutions for the Enterprise

On Thu, 23 Jan 2020 at 01:20, Tom Lane <tgl@sss.pgh.pa.us> wrote:

Bruce Momjian <bruce@momjian.us> writes:

I think the big question is whether we want to make active prepared
transactions more visible to administrators, either during server start
or idle duration.

There's already the pg_prepared_xacts view ...

I think Bruce has a point here. We shouldn't go around "resolving"
prepared xacts, but the visibility of them is a problem for users.
I've seen that myself quite enough times, even now that they cannot be
used by default.

Our monitoring and admin views are not keeping up with Pg's
complexity. Resource retention is one area where that's becoming a
usability and admin challenge. If a user has growing bloat (and have
managed to figure that out, since we don't make it easy to do that
either) or unexpected WAL retention they may find it hard to quickly
work out why.

We could definitely improve on that by exposing a view that integrates
everything that holds down xmin and catalog_xmin. It'd show

* the datfrozenxid and datminmxid for the oldest database
* if that database is the current database, the relation(s) with the
oldest relfrozenxid and relminmxd
* ... and the catalog relation(s) with the oldest relfrozenxid and
relminmxid if greater
* the absolute xid and xid-age positions of entries in pg_replication_slots
* pg_stat_replication connections (joined to pg_stat_replication if
connected) with their feedback xmin
* pg_stat_activity backend_xid and backend_xmin for the backend(s)
with oldest values; this may be different sets of backends
* pg_prepared_xacts entries by oldest xid

... probably sorted by xid age.

It'd be good to expose some internal state too, which would usually
correspond to the oldest values found in the above, but is useful for
cross-checking:

* RecentGlobalXmin and RecentGlobalDataXmin to show the xmin and
catalog_xmin actually used
* procArray->replication_slot_xmin and procArray->replication_slot_catalog_xmin

I'm not sure whether WAL retention (lsn tracking) should be in the
same view or a different one, but I lean toward different.

I already have another TODO kicking around for me to write a view that
generates a blocking locks graph, since pg_locks is really more of a
building block than a directly useful view for admins to understand
the system's state. And if that's not enough I also want to write a
decent bloat-checking view to include in the system views, since IMO
lock-blocking, bloat, and resource retention are real monitoring pain
points right now.

--
Craig Ringer http://www.2ndQuadrant.com/
2ndQuadrant - PostgreSQL Solutions for the Enterprise

On Thu, Jan 23, 2020 at 12:56:41PM +0800, Craig Ringer wrote:

We could definitely improve on that by exposing a view that integrates
everything that holds down xmin and catalog_xmin. It'd show

* the datfrozenxid and datminmxid for the oldest database
* if that database is the current database, the relation(s) with the
oldest relfrozenxid and relminmxd
* ... and the catalog relation(s) with the oldest relfrozenxid and
relminmxid if greater
* the absolute xid and xid-age positions of entries in pg_replication_slots
* pg_stat_replication connections (joined to pg_stat_replication if
connected) with their feedback xmin
* pg_stat_activity backend_xid and backend_xmin for the backend(s)
with oldest values; this may be different sets of backends
* pg_prepared_xacts entries by oldest xid

It seems to me that what you are describing here is a set of
properties good for a monitoring tool that we don't necessarily need
to maintain in core. There are already tools able to do that in ways
I think are better than what we could ever design, like
check_pgactivity and such. And there are years of experience behind
that from the maintainers of such scripts and/or extensions.

The argument about Postgres getting more and more complex is true as
the code grows, but I am not really convinced that we need to make it
grow more with more layers that we think are good, because we may
finish by piling up stuff which are not actually that good in the long
term. I'd rather just focus in the core code on the basics with views
that map directly to what we have in memory and/or disk.
--
Michael

On Thu, 23 Jan 2020 at 15:04, Michael Paquier <michael@paquier.xyz> wrote:

It seems to me that what you are describing here is a set of
properties good for a monitoring tool that we don't necessarily need
to maintain in core. There are already tools able to do that in ways
I think are better than what we could ever design, like
check_pgactivity and such.

I really have to disagree here.

Relying on external tools gives users who already have to piece
together a lot of fragments even more moving parts to keep track of.
It introduces more places where new server releases may not be
supported in a timely manner by various tools users rely on. More
places where users may get wrong or incomplete information from
outdated or incorrect tools. I cite the monstrosity that
"check_postgres.pl" has become as a specific example of why pushing
our complexity onto external tools is not always the right answer.

We already have a number of views that prettify information to help
administrators operate the server. You could argue that
pg_stat_activity and pg_stat_replication are unnecessary for example;
users should use external tools to query pg_stat_get_activity(),
pg_stat_get_wal_senders(), pg_authid and pg_database directly to get
the information they need. Similarly, we could do away with
pg_stat_user_indexes and the like, as they're just convenience views
over lower level information exposed by the server.

But can you really imagine using postgres day to day without pg_stat_activity?

It is my firm opinion that visibility into locking behaviour and lock
waits is of a similar level of importance. So is giving users some way
to get insight into table and index bloat on our MVCC database. With
the enormous uptake of various forms of replication and HA it's also
important that users also be able to see what's affecting resource
retention - holding down vacuum, retaining WAL, etc.

The server knows more than any tools. Views in the server can also be
maintained along with the server to address changes in how it manages
things like resource retention, so external tools get a more
consistent insight into server behaviour.

I'd rather just focus in the core code on the basics with views
that map directly to what we have in memory and/or disk.

Per above, I just can't agree with this. PostgreSQL is a system with
end users who need to interact with it, most of whom will not know how
its innards work. If we're going to position it even more as a
component in some larger stack such that it's not expected to really
be used standalone, then we should make some effort to guide users
toward the other components they will need *in our own documentation*
and ensure they're tested and maintained.

Proposals to do that with HA and failover tooling, backup tooling etc
have never got off the ground. I think we do users a great disservice
there personally. I don't expect any proposal to bless specific
monitoring tools to be any more successful.

More importantly, I fail to see why every monitoring tool should
reinvent the same information collection queries and views, each with
their own unique bugs and quirks, when we can provide information
users need directly from the server.

In any case I guess it's all hot air unless I pony up a patch to show
how I think it should work.

--
Craig Ringer http://www.2ndQuadrant.com/
2ndQuadrant - PostgreSQL Solutions for the Enterprise

So having seen the feedback on this thread, and I tend to agree with most
of what has been said here, I also agree that the server core isn't really
the ideal place to handle the orphan prepared transactions.

Ideally, these must be handled by a transaction manager, however, I do
believe that we cannot let database suffer for failing of an external
software, and we did a similar change through introduction of idle in
transaction timeout behavior. That said, implementing something similar for
this feature is too much of an overhead both in terms of code complexity
and resources utilisation (if the feature is implemented).

I'm currently working on other options to tackle this problem.

On Tue, 28 Jan 2020 at 9:04 AM, Craig Ringer <craig@2ndquadrant.com> wrote:

On Thu, 23 Jan 2020 at 15:04, Michael Paquier <michael@paquier.xyz> wrote:

It seems to me that what you are describing here is a set of
properties good for a monitoring tool that we don't necessarily need
to maintain in core. There are already tools able to do that in ways
I think are better than what we could ever design, like
check_pgactivity and such.

I really have to disagree here.

Relying on external tools gives users who already have to piece
together a lot of fragments even more moving parts to keep track of.
It introduces more places where new server releases may not be
supported in a timely manner by various tools users rely on. More
places where users may get wrong or incomplete information from
outdated or incorrect tools. I cite the monstrosity that
"check_postgres.pl" has become as a specific example of why pushing
our complexity onto external tools is not always the right answer.

We already have a number of views that prettify information to help
administrators operate the server. You could argue that
pg_stat_activity and pg_stat_replication are unnecessary for example;
users should use external tools to query pg_stat_get_activity(),
pg_stat_get_wal_senders(), pg_authid and pg_database directly to get
the information they need. Similarly, we could do away with
pg_stat_user_indexes and the like, as they're just convenience views
over lower level information exposed by the server.

But can you really imagine using postgres day to day without
pg_stat_activity?

It is my firm opinion that visibility into locking behaviour and lock
waits is of a similar level of importance. So is giving users some way
to get insight into table and index bloat on our MVCC database. With
the enormous uptake of various forms of replication and HA it's also
important that users also be able to see what's affecting resource
retention - holding down vacuum, retaining WAL, etc.

The server knows more than any tools. Views in the server can also be
maintained along with the server to address changes in how it manages
things like resource retention, so external tools get a more
consistent insight into server behaviour.

I'd rather just focus in the core code on the basics with views
that map directly to what we have in memory and/or disk.

Per above, I just can't agree with this. PostgreSQL is a system with
end users who need to interact with it, most of whom will not know how
its innards work. If we're going to position it even more as a
component in some larger stack such that it's not expected to really
be used standalone, then we should make some effort to guide users
toward the other components they will need *in our own documentation*
and ensure they're tested and maintained.

Proposals to do that with HA and failover tooling, backup tooling etc
have never got off the ground. I think we do users a great disservice
there personally. I don't expect any proposal to bless specific
monitoring tools to be any more successful.

More importantly, I fail to see why every monitoring tool should
reinvent the same information collection queries and views, each with
their own unique bugs and quirks, when we can provide information
users need directly from the server.

In any case I guess it's all hot air unless I pony up a patch to show
how I think it should work.

--
Craig Ringer http://www.2ndQuadrant.com/
2ndQuadrant - PostgreSQL Solutions for the Enterprise

--

Highgo Software (Canada/China/Pakistan)
URL : www.highgo.ca
ADDR: 10318 WHALLEY BLVD, Surrey, BC
CELL：+923335449950 EMAIL: mailto:hamid.akhtar@highgo.ca
SKYPE: engineeredvirus

On Thu, 30 Jan 2020 at 02:04, Hamid Akhtar <hamid.akhtar@gmail.com> wrote:

So having seen the feedback on this thread, and I tend to agree with most of what has been said here, I also agree that the server core isn't really the ideal place to handle the orphan prepared transactions.

Ideally, these must be handled by a transaction manager, however, I do believe that we cannot let database suffer for failing of an external software, and we did a similar change through introduction of idle in transaction timeout behavior.

The difference, IMO, is that idle-in-transaction aborts don't affect
anything we've promised to be durable.

Once you PREPARE TRANSACTION the DB has made a promise that that txn
is durable. We don't have any consistent feedback channel to back to
applications and say "Hey, if you're not going to finish this up we
need to get rid of it soon, ok?". If a distributed transaction manager
gets consensus for commit and goes to COMMIT PREPARED a previously
prepared txn only to find that it has vanished, that's a major
problem, and one that may bring the entire DTM to a halt until the
admin can intervene.

This isn't like idle-in-transaction aborts. It's closer to something
like uncommitting a previously committed transaction.

I do think it'd make sense to ensure that the documentation clearly
highlights the impact of abandoned prepared xacts on server resource
retention and performance, preferably with pointers to appropriate
views. I haven't reviewed the docs to see how clear that is already.

I can also see an argument for a periodic log message (maybe from
vacuum?) warning when old prepared xacts hold xmin down. Including one
sent to the client application when an explicit VACUUM is executed.
(In fact, it'd make sense to generalise that for all xmin-retention).

But I'm really not a fan of aborting such txns. If you operate with
some kind of broken global transaction manager that can forget or
abandon prepared xacts, then fix it, or adopt site-local periodic
cleanup tasks that understand your site's needs.

--
Craig Ringer http://www.2ndQuadrant.com/
2ndQuadrant - PostgreSQL Solutions for the Enterprise

On Thu, Jan 30, 2020 at 8:28 AM Craig Ringer <craig@2ndquadrant.com> wrote:

On Thu, 30 Jan 2020 at 02:04, Hamid Akhtar <hamid.akhtar@gmail.com> wrote:

So having seen the feedback on this thread, and I tend to agree with

most of what has been said here, I also agree that the server core isn't
really the ideal place to handle the orphan prepared transactions.

Ideally, these must be handled by a transaction manager, however, I do

believe that we cannot let database suffer for failing of an external
software, and we did a similar change through introduction of idle in
transaction timeout behavior.

The difference, IMO, is that idle-in-transaction aborts don't affect
anything we've promised to be durable.

Once you PREPARE TRANSACTION the DB has made a promise that that txn
is durable. We don't have any consistent feedback channel to back to
applications and say "Hey, if you're not going to finish this up we
need to get rid of it soon, ok?". If a distributed transaction manager
gets consensus for commit and goes to COMMIT PREPARED a previously
prepared txn only to find that it has vanished, that's a major
problem, and one that may bring the entire DTM to a halt until the
admin can intervene.

This isn't like idle-in-transaction aborts. It's closer to something
like uncommitting a previously committed transaction.

I do think it'd make sense to ensure that the documentation clearly
highlights the impact of abandoned prepared xacts on server resource
retention and performance, preferably with pointers to appropriate
views. I haven't reviewed the docs to see how clear that is already.

Having seen the documentation, IMHO the document does contain enough
information for users to understand what issues can be caused by these
orphaned prepared transactions.

I can also see an argument for a periodic log message (maybe from
vacuum?) warning when old prepared xacts hold xmin down. Including one
sent to the client application when an explicit VACUUM is executed.
(In fact, it'd make sense to generalise that for all xmin-retention).

I think that opens up the debate on what we really mean by "old" and
whether that requires a syntax change when creating a prepared
transactions as Thomas Kellerer suggested earlier?

I agree that vacuum should periodically throw warnings for any prepared
xacts that are holding xmin down.

Generalising it for all xmin-retention is a fair idea IMHO, though that
does increase the overall scope here. A vacuum process should (ideally)
periodically throw out warnings for anything that is preventing it
(including
orphaned prepared transactions) from doing its routine work so that
somebody can take necessary actions.

But I'm really not a fan of aborting such txns. If you operate with
some kind of broken global transaction manager that can forget or
abandon prepared xacts, then fix it, or adopt site-local periodic
cleanup tasks that understand your site's needs.

--
Craig Ringer http://www.2ndQuadrant.com/
2ndQuadrant - PostgreSQL Solutions for the Enterprise

--
Highgo Software (Canada/China/Pakistan)
URL : www.highgo.ca
ADDR: 10318 WHALLEY BLVD, Surrey, BC
CELL：+923335449950 EMAIL: mailto:hamid.akhtar@highgo.ca
SKYPE: engineeredvirus

All,

Attached is version 1 of POC patch for notifying of orphaned
prepared transactions via warnings emitted to a client
application and/or log file. It applies to PostgreSQL branch
"master" on top of "e2e02191" commit.

I've tried to keep the patch as less invasive as I could with
minimal impact on vacuum processes, so the performance impact
and the changes are minimal in that area of PostgreSQL core.

- What's in this Patch:

This patch throws warnings when an autovacuum worker encounters
an orphaned prepared transaction. It also throws warnings to a
client when a vacuum command is issued. This patch also
introduces two new GUCs:

(1) max_age_prepared_xacts
- The age after creation of a prepared transaction after which it
will be considered an orphan.

(2) prepared_xacts_vacuum_warn_timeout
- The timeout period for an autovacuum (essentially any of its
worker) to check for orphaned prepared transactions and throw
warnings if any are found.

- What This Patch Does:

If the GUCs are enabled (set to a value higher than -1), an
autovacuum worker running in the background checks if the
timeout has expired. If so, it checks if there are any orphaned
prepared transactions (i.e. their age has exceeded
max_age_prepared_xacts). If it finds any, it throws a warning for
every such transaction. It also emits the total number of orphaned
prepared transactions if one or more are found.

When a vacuum command is issued from within a client, say psql,
in that case, we skip the vacuum timeout check and simply scan
for any orphaned prepared transactions. Warnings are emitted to
the client and log file if any are found.

- About the New GUCs:

= max_age_prepared_xacts:
Sets maximum age after which a prepared transaction is considered an
orphan. It applies when "prepared transactions" are enabled. The
age for a transaction is calculated from the time it was created to
the current time. If this value is specified without units, it is taken
as milliseconds. The default value is -1 which allows prepared
transactions to live forever.

= prepared_xacts_vacuum_warn_timeout:
Sets timeout after which vacuum starts throwing warnings for every
prepared transactions that has exceeded maximum age defined by
"max_age_prepared_xacts". If this value is specified without units,
it is taken as milliseconds. The default value of -1 will disable
this warning mechanism. Setting a too value could potentially fill
up log with orphaned prepared transaction warnings, so this
parameter must be set to a value that is reasonably large to not
fill up log file, but small enough to notify of long running and
potential orphaned prepared transactions. There is no additional
timer or worker introduced with this change. Whenever a vacuum
worker runs, it first checks for any orphaned prepared transactions.
So at best, this GUC serves as a guideline for a vacuum worker
if a warning should be thrown to log file or a client issuing
vacuum command.

- What this Patch Does Not Cover:

The warning is not thrown when user either runs vacuumdb or passes
individual relations to be vacuum. Specifically in case of vacuumdb,
it breaks down a vacuum command to an attribute-wise vacuum command.
So the vacuum command is indirectly run many times. Considering that
we want to emit warnings for every manual vacuum command, this simply
floods the terminal and log with orphaned prepared transactions
warnings. We could potentially handle that, but the overhead of
that seemed too much to me (and I've not invested any time looking
to fix that either). Hence, warnings are not thrown when user runs
vacuumdb and relation specific vacuum.

On Fri, Jan 31, 2020 at 7:02 PM Hamid Akhtar <hamid.akhtar@gmail.com> wrote:

On Thu, Jan 30, 2020 at 8:28 AM Craig Ringer <craig@2ndquadrant.com>
wrote:

On Thu, 30 Jan 2020 at 02:04, Hamid Akhtar <hamid.akhtar@gmail.com>
wrote:

So having seen the feedback on this thread, and I tend to agree with

most of what has been said here, I also agree that the server core isn't
really the ideal place to handle the orphan prepared transactions.

Ideally, these must be handled by a transaction manager, however, I do

believe that we cannot let database suffer for failing of an external
software, and we did a similar change through introduction of idle in
transaction timeout behavior.

The difference, IMO, is that idle-in-transaction aborts don't affect
anything we've promised to be durable.

Once you PREPARE TRANSACTION the DB has made a promise that that txn
is durable. We don't have any consistent feedback channel to back to
applications and say "Hey, if you're not going to finish this up we
need to get rid of it soon, ok?". If a distributed transaction manager
gets consensus for commit and goes to COMMIT PREPARED a previously
prepared txn only to find that it has vanished, that's a major
problem, and one that may bring the entire DTM to a halt until the
admin can intervene.

This isn't like idle-in-transaction aborts. It's closer to something
like uncommitting a previously committed transaction.

I do think it'd make sense to ensure that the documentation clearly
highlights the impact of abandoned prepared xacts on server resource
retention and performance, preferably with pointers to appropriate
views. I haven't reviewed the docs to see how clear that is already.

Having seen the documentation, IMHO the document does contain enough
information for users to understand what issues can be caused by these
orphaned prepared transactions.

I can also see an argument for a periodic log message (maybe from
vacuum?) warning when old prepared xacts hold xmin down. Including one
sent to the client application when an explicit VACUUM is executed.
(In fact, it'd make sense to generalise that for all xmin-retention).

I think that opens up the debate on what we really mean by "old" and
whether that requires a syntax change when creating a prepared
transactions as Thomas Kellerer suggested earlier?

I agree that vacuum should periodically throw warnings for any prepared
xacts that are holding xmin down.

Generalising it for all xmin-retention is a fair idea IMHO, though that
does increase the overall scope here. A vacuum process should (ideally)
periodically throw out warnings for anything that is preventing it
(including
orphaned prepared transactions) from doing its routine work so that
somebody can take necessary actions.

But I'm really not a fan of aborting such txns. If you operate with
some kind of broken global transaction manager that can forget or
abandon prepared xacts, then fix it, or adopt site-local periodic
cleanup tasks that understand your site's needs.

--
Craig Ringer http://www.2ndQuadrant.com/
2ndQuadrant - PostgreSQL Solutions for the Enterprise

--
Highgo Software (Canada/China/Pakistan)
URL : www.highgo.ca
ADDR: 10318 WHALLEY BLVD, Surrey, BC
CELL：+923335449950 EMAIL: mailto:hamid.akhtar@highgo.ca
SKYPE: engineeredvirus

--
Highgo Software (Canada/China/Pakistan)
URL : www.highgo.ca
ADDR: 10318 WHALLEY BLVD, Surrey, BC
CELL：+923335449950 EMAIL: mailto:hamid.akhtar@highgo.ca
SKYPE: engineeredvirus

Here is the v2 of the same patch after rebasing it and running it through
pgindent. There are no other code changes.

On Wed, Feb 19, 2020 at 8:04 PM Hamid Akhtar <hamid.akhtar@gmail.com> wrote:

All,

Attached is version 1 of POC patch for notifying of orphaned
prepared transactions via warnings emitted to a client
application and/or log file. It applies to PostgreSQL branch
"master" on top of "e2e02191" commit.

I've tried to keep the patch as less invasive as I could with
minimal impact on vacuum processes, so the performance impact
and the changes are minimal in that area of PostgreSQL core.

- What's in this Patch:

This patch throws warnings when an autovacuum worker encounters
an orphaned prepared transaction. It also throws warnings to a
client when a vacuum command is issued. This patch also
introduces two new GUCs:

(1) max_age_prepared_xacts
- The age after creation of a prepared transaction after which it
will be considered an orphan.

(2) prepared_xacts_vacuum_warn_timeout
- The timeout period for an autovacuum (essentially any of its
worker) to check for orphaned prepared transactions and throw
warnings if any are found.

- What This Patch Does:

If the GUCs are enabled (set to a value higher than -1), an
autovacuum worker running in the background checks if the
timeout has expired. If so, it checks if there are any orphaned
prepared transactions (i.e. their age has exceeded
max_age_prepared_xacts). If it finds any, it throws a warning for
every such transaction. It also emits the total number of orphaned
prepared transactions if one or more are found.

When a vacuum command is issued from within a client, say psql,
in that case, we skip the vacuum timeout check and simply scan
for any orphaned prepared transactions. Warnings are emitted to
the client and log file if any are found.

- About the New GUCs:

= max_age_prepared_xacts:
Sets maximum age after which a prepared transaction is considered an
orphan. It applies when "prepared transactions" are enabled. The
age for a transaction is calculated from the time it was created to
the current time. If this value is specified without units, it is taken
as milliseconds. The default value is -1 which allows prepared
transactions to live forever.

= prepared_xacts_vacuum_warn_timeout:
Sets timeout after which vacuum starts throwing warnings for every
prepared transactions that has exceeded maximum age defined by
"max_age_prepared_xacts". If this value is specified without units,
it is taken as milliseconds. The default value of -1 will disable
this warning mechanism. Setting a too value could potentially fill
up log with orphaned prepared transaction warnings, so this
parameter must be set to a value that is reasonably large to not
fill up log file, but small enough to notify of long running and
potential orphaned prepared transactions. There is no additional
timer or worker introduced with this change. Whenever a vacuum
worker runs, it first checks for any orphaned prepared transactions.
So at best, this GUC serves as a guideline for a vacuum worker
if a warning should be thrown to log file or a client issuing
vacuum command.

- What this Patch Does Not Cover:

The warning is not thrown when user either runs vacuumdb or passes
individual relations to be vacuum. Specifically in case of vacuumdb,
it breaks down a vacuum command to an attribute-wise vacuum command.
So the vacuum command is indirectly run many times. Considering that
we want to emit warnings for every manual vacuum command, this simply
floods the terminal and log with orphaned prepared transactions
warnings. We could potentially handle that, but the overhead of
that seemed too much to me (and I've not invested any time looking
to fix that either). Hence, warnings are not thrown when user runs
vacuumdb and relation specific vacuum.

On Fri, Jan 31, 2020 at 7:02 PM Hamid Akhtar <hamid.akhtar@gmail.com>
wrote:

On Thu, Jan 30, 2020 at 8:28 AM Craig Ringer <craig@2ndquadrant.com>
wrote:

On Thu, 30 Jan 2020 at 02:04, Hamid Akhtar <hamid.akhtar@gmail.com>
wrote:

So having seen the feedback on this thread, and I tend to agree with

most of what has been said here, I also agree that the server core isn't
really the ideal place to handle the orphan prepared transactions.

Ideally, these must be handled by a transaction manager, however, I do

believe that we cannot let database suffer for failing of an external
software, and we did a similar change through introduction of idle in
transaction timeout behavior.

The difference, IMO, is that idle-in-transaction aborts don't affect
anything we've promised to be durable.

Once you PREPARE TRANSACTION the DB has made a promise that that txn
is durable. We don't have any consistent feedback channel to back to
applications and say "Hey, if you're not going to finish this up we
need to get rid of it soon, ok?". If a distributed transaction manager
gets consensus for commit and goes to COMMIT PREPARED a previously
prepared txn only to find that it has vanished, that's a major
problem, and one that may bring the entire DTM to a halt until the
admin can intervene.

This isn't like idle-in-transaction aborts. It's closer to something
like uncommitting a previously committed transaction.

I do think it'd make sense to ensure that the documentation clearly
highlights the impact of abandoned prepared xacts on server resource
retention and performance, preferably with pointers to appropriate
views. I haven't reviewed the docs to see how clear that is already.

Having seen the documentation, IMHO the document does contain enough
information for users to understand what issues can be caused by these
orphaned prepared transactions.

I can also see an argument for a periodic log message (maybe from
vacuum?) warning when old prepared xacts hold xmin down. Including one
sent to the client application when an explicit VACUUM is executed.
(In fact, it'd make sense to generalise that for all xmin-retention).

I think that opens up the debate on what we really mean by "old" and
whether that requires a syntax change when creating a prepared
transactions as Thomas Kellerer suggested earlier?

I agree that vacuum should periodically throw warnings for any prepared
xacts that are holding xmin down.

Generalising it for all xmin-retention is a fair idea IMHO, though that
does increase the overall scope here. A vacuum process should (ideally)
periodically throw out warnings for anything that is preventing it
(including
orphaned prepared transactions) from doing its routine work so that
somebody can take necessary actions.

But I'm really not a fan of aborting such txns. If you operate with
some kind of broken global transaction manager that can forget or
abandon prepared xacts, then fix it, or adopt site-local periodic
cleanup tasks that understand your site's needs.

--
Craig Ringer http://www.2ndQuadrant.com/
2ndQuadrant - PostgreSQL Solutions for the Enterprise

--
Highgo Software (Canada/China/Pakistan)
URL : www.highgo.ca
ADDR: 10318 WHALLEY BLVD, Surrey, BC
CELL：+923335449950 EMAIL: mailto:hamid.akhtar@highgo.ca
SKYPE: engineeredvirus

--
Highgo Software (Canada/China/Pakistan)
URL : www.highgo.ca
ADDR: 10318 WHALLEY BLVD, Surrey, BC
CELL：+923335449950 EMAIL: mailto:hamid.akhtar@highgo.ca
SKYPE: engineeredvirus

--
Highgo Software (Canada/China/Pakistan)
URL : www.highgo.ca
ADDR: 10318 WHALLEY BLVD, Surrey, BC
CELL：+923335449950 EMAIL: mailto:hamid.akhtar@highgo.ca
SKYPE: engineeredvirus

On Mon, Mar 2, 2020 at 05:42:11PM +0500, Hamid Akhtar wrote:

Here is the v2 of the same patch after rebasing it and running it through
pgindent. There are no other code changes.

The paragraph about max_age_prepared_xacts doesn't define what is the
effect of treating a transaction as orphaned.

--
Bruce Momjian <bruce@momjian.us> https://momjian.us
EnterpriseDB https://enterprisedb.com

+ As you are, so once was I.  As I am, so you will be. +
+                      Ancient Roman grave inscription +

On Mon, 2 Mar 2020 at 21:42, Hamid Akhtar <hamid.akhtar@gmail.com> wrote:

Here is the v2 of the same patch after rebasing it and running it through pgindent. There are no other code changes.

Thank you for working on this. I think what this patch tries to
achieve would be helpful to inform orphaned prepared transactions that
can be cause of inefficient vacuum to users.

As far as I read the patch, the setting this feature using newly added
parameters seems to be complicated to me. IIUC, even if a prepared
transactions is enough older than max_age_prepared_xacts, we don't
warn if it doesn't elapsed prepared_xacts_vacuum_warn_timeout since
when the "first" prepared transaction is created. And the first
prepared transaction means that the first entry for
TwoPhaseStateData->prepXacts. Therefore, if there is always more than
one prepared transaction, we don't warn for at least
prepared_xacts_vacuum_warn_timeout seconds even if the first added
prepared transaction is already removed. So I'm not sure how we can
think the setting value of prepared_xacts_vacuum_warn_timeout.

Regarding the warning message, I wonder if the current message is too
detailed. If we want to inform that there is orphaned prepared
transactions to users, it seems to me that it's enough to report the
existence (and possibly the number of orphaned prepared transactions),
rather than individual details.

Given that the above things, we can simply think this feature; we can
have only max_age_prepared_xacts, and autovacuum checks the minimum of
prepared_at of prepared transactions, and compare it to
max_age_prepared_xacts. We can warn if (CurrentTimestamp -
min(prepared_at)) > max_age_prepared_xacts. In addition, if we also
want to control this behavior by the age of xid, we can have another
GUC parameter for comparing the age(min(xid of prepared transactions))
to that value.

Finally, regarding the name of parameters, when we mention the age of
transaction it means the age of xid of the transaction, not the time.
Please refer to other GUC parameter having "age" in its name such as
autovacuum_freeze_max_age, vacuum_freeze_min_age. The patch adds
max_age_prepared_xacts but I think it should be renamed. For example,
prepared_xact_warn_min_duration is for time and
prepared_xact_warn_max_age for age.

Regards,

--
Masahiko Sawada http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services