Re: Hot standby, dropping a tablespace

When replaying a DROP TABLE SPACE, you first try to remove the
directory, and if that fails, you assume that it's because it's in use
as a temp tablespace in a read-only transaction. You then call
ResolveRecoveryConflictWithVirtualXIDs to kill such transactions, and
try removing the directory again. But
ResolveRecoveryConflictWithVirtualXIDs doesn't wait for the target
transaction to die anymore (or at least it shouldn't, as we discussed
earlier), so that doesn't work AFAICS.

One quick work around would be to simply not respect temp_tablespace
during recovery...

--
Heikki Linnakangas
EnterpriseDB http://www.enterprisedb.com

On Sat, 2009-01-24 at 21:58 +0200, Heikki Linnakangas wrote:
> When replaying a DROP TABLE SPACE, you first try to remove the
> directory, and if that fails, you assume that it's because it's in use
> as a temp tablespace in a read-only transaction.

That sounds like you think there another conclusion?

> You then call
> ResolveRecoveryConflictWithVirtualXIDs to kill such transactions, and
> try removing the directory again. But
> ResolveRecoveryConflictWithVirtualXIDs doesn't wait for the target
> transaction to die anymore (or at least it shouldn't, as we discussed
> earlier), so that doesn't work AFAICS.

The FATAL errors inflicted should be fairly quick to take effect, so
waiting should not be a problem. We can make waiting for FATAL errors
the standard response.

> One quick work around would be to simply not respect temp_tablespace
> during recovery...

No, don't think that's acceptable.

--
Simon Riggs www.2ndQuadrant.com
PostgreSQL Training, Services and Support

Simon Riggs wrote:
> On Sat, 2009-01-24 at 21:58 +0200, Heikki Linnakangas wrote:
>> When replaying a DROP TABLE SPACE, you first try to remove the
>> directory, and if that fails, you assume that it's because it's in use
>> as a temp tablespace in a read-only transaction.
>
> That sounds like you think there another conclusion?

No, I think that assumption is correct.

>> You then call
>> ResolveRecoveryConflictWithVirtualXIDs to kill such transactions, and
>> try removing the directory again. But
>> ResolveRecoveryConflictWithVirtualXIDs doesn't wait for the target
>> transaction to die anymore (or at least it shouldn't, as we discussed
>> earlier), so that doesn't work AFAICS.
>
> The FATAL errors inflicted should be fairly quick to take effect, so
> waiting should not be a problem. We can make waiting for FATAL errors
> the standard response.

The call in tablespace replay uses ERROR, not FATAL. You don't need to
kill the whole session, just the current transaction.

It seems more and more to me that the FATAL and ERROR cases are really
quite different. Looking at the callers, there's four different needs
for GetConflictingVirtualXIDs+ResolveRecoveryConflictWithVirtualXIDs:

1. Kill all connections to a given database. Used when replaying DROP
DATABASE.

2. Kill all connections by given user. Hmm, not used for anything,
actually. Should remove the roleId argument from GetConflictingVirtualXIDs.

3. Kill all transactions using given tablespace as temp tablespace, FROP
TABLESPACE.

4. Mark all transactions that still see a given XID as running for
termination if they try to access a buffer with conflicting LSN (VACUUM,
btree-deletes).

All callers call GetConflictingVirtualXIDs first, and then
ResolveRecoveryConflictWithVirtualXIDs. That's a bit cumbersome; none of
the callers do anything else with the virtualxid array they get from
GetConflictingVirtualXIDs than pass it on to
ResolveRecoveryConflictWithVirtualXIDs. I'm thinking of an interface
consisting of three functions, replacing the current
GetConflictingVirtualXIDs and ResolveRecoveryConflictWithVirtualXIDs
functions:

/* Kill all connections to given database, wait for them to die */
void KillConnectionsToDatabase(Oid dbOid)

/* Kill all transactions that use given tablespace, wait for them to die */
void KillTransactionsUsingTablespace(Oid tablespaceOid)

/* Signal all backends that with xmin < limitXid that they need to abort
if they access a page with LSN >= conflict_lsn. (Or wait for them to
end, if max_standby_delay > 0). */
void ResolveRecoveryConflicts(Oid dbOid, TransactionId limitXid,
XLogRecPtr conflict_lsn, char *reason)

--
Heikki Linnakangas
EnterpriseDB http://www.enterprisedb.com

On 2009-01-25, at 09:04, Simon Riggs wrote:

>
> On Sat, 2009-01-24 at 21:58 +0200, Heikki Linnakangas wrote:
>> When replaying a DROP TABLE SPACE, you first try to remove the
>> directory, and if that fails, you assume that it's because it's in
>> use
>> as a temp tablespace in a read-only transaction.
>
> That sounds like you think there another conclusion?

What if subdirectory of that directory is owned by root ?
Say I create /home/gj/tablespace1 . Surely emptying it is possible,
but should it remove the dir altogether ?
It is possible to create tablespace in that directory, even so
postgres user doesn't own /home/gj/ directory. So why shouldn't it be
possible to drop it ?

On Sun, 2009-01-25 at 11:28 +0200, Heikki Linnakangas wrote:

> >> You then call
> >> ResolveRecoveryConflictWithVirtualXIDs to kill such transactions, and
> >> try removing the directory again. But
> >> ResolveRecoveryConflictWithVirtualXIDs doesn't wait for the target
> >> transaction to die anymore (or at least it shouldn't, as we discussed
> >> earlier), so that doesn't work AFAICS.
> >
> > The FATAL errors inflicted should be fairly quick to take effect, so
> > waiting should not be a problem. We can make waiting for FATAL errors
> > the standard response.
>
> The call in tablespace replay uses ERROR, not FATAL. You don't need to
> kill the whole session, just the current transaction.

Yeh, yeh, it already does that, FATAL was just my typo. My bad.

> It seems more and more to me that the FATAL and ERROR cases are really
> quite different. Looking at the callers, there's four different needs
> for GetConflictingVirtualXIDs+ResolveRecoveryConflictWithVirtualXIDs:

I think the deferred case should be handled with a different function,
since it needs further work on it and that is best done as a second
function.

> 1. Kill all connections to a given database. Used when replaying DROP
> DATABASE.
>
> 2. Kill all connections by given user. Hmm, not used for anything,
> actually. Should remove the roleId argument from GetConflictingVirtualXIDs.

No, because we still need to add code to kill-connected-users if we drop
role.

> 3. Kill all transactions using given tablespace as temp tablespace, FROP
> TABLESPACE.
>
> 4. Mark all transactions that still see a given XID as running for
> termination if they try to access a buffer with conflicting LSN (VACUUM,
> btree-deletes).
>
> All callers call GetConflictingVirtualXIDs first, and then
> ResolveRecoveryConflictWithVirtualXIDs. That's a bit cumbersome; none of
> the callers do anything else with the virtualxid array they get from
> GetConflictingVirtualXIDs than pass it on to
> ResolveRecoveryConflictWithVirtualXIDs. I'm thinking of an interface
> consisting of three functions, replacing the current
> GetConflictingVirtualXIDs and ResolveRecoveryConflictWithVirtualXIDs
> functions:

I'm not sure I see any benefit in doing that.

The two big changes we did earlier were worthwhile but I'm concerned
about how much refactoring you want to do. It introduces new bugs each
time and currently they take time to isolate and fix - much of that work
is currently not very visible, but its a big effort. If I was happy we
had a perfect working solution and we were not under time pressure then
I'd say go for it as much as you like. I'd prefer it if we could get
everything correct before we put all the code in the right cupboards. I
know tidy-up-as-you-go is a good policy but I'd encourage you to do a
first pass looking for potential problems before we did that. Or maybe
we're there already, not sure.

--
Simon Riggs www.2ndQuadrant.com
PostgreSQL Training, Services and Support

Simon Riggs wrote:
>> 2. Kill all connections by given user. Hmm, not used for anything,
>> actually. Should remove the roleId argument from GetConflictingVirtualXIDs.
>
> No, because we still need to add code to kill-connected-users if we drop
> role.

Oh, I see, that's still a todo item. But we don't do that during normal
operation, so why should we during recovery?

>> I'm thinking of an interface
>> consisting of three functions, replacing the current
>> GetConflictingVirtualXIDs and ResolveRecoveryConflictWithVirtualXIDs
>> functions:
>
> I'm not sure I see any benefit in doing that.

It makes the code more readable.

--
Heikki Linnakangas
EnterpriseDB http://www.enterprisedb.com

On Sun, 2009-01-25 at 19:56 +0200, Heikki Linnakangas wrote:
> Simon Riggs wrote:
> >> 2. Kill all connections by given user. Hmm, not used for anything,
> >> actually. Should remove the roleId argument from GetConflictingVirtualXIDs.
> >
> > No, because we still need to add code to kill-connected-users if we drop
> > role.
>
> Oh, I see, that's still a todo item. But we don't do that during normal
> operation, so why should we during recovery?

LOL. Not many systems allow you to continue working after the access has
been removed. But not, as you say, a problem for Hot Standby unless that
changes.

--
Simon Riggs www.2ndQuadrant.com
PostgreSQL Training, Services and Support

On Sun, 2009-01-25 at 12:13 +0000, Grzegorz Jaskiewicz wrote:
> On 2009-01-25, at 09:04, Simon Riggs wrote:
>
> >
> > On Sat, 2009-01-24 at 21:58 +0200, Heikki Linnakangas wrote:
> >> When replaying a DROP TABLE SPACE, you first try to remove the
> >> directory, and if that fails, you assume that it's because it's in
> >> use
> >> as a temp tablespace in a read-only transaction.
> >
> > That sounds like you think there another conclusion?
>
> What if subdirectory of that directory is owned by root ?
> Say I create /home/gj/tablespace1 . Surely emptying it is possible,
> but should it remove the dir altogether ?
> It is possible to create tablespace in that directory, even so
> postgres user doesn't own /home/gj/ directory. So why shouldn't it be
> possible to drop it ?

This is not a question for the Hot Standby patch to answer. Recovery is
just trying to emulate what happens in normal running, nothing more.

--
Simon Riggs www.2ndQuadrant.com
PostgreSQL Training, Services and Support

Hi,

Simon Riggs wrote:
> On Sun, 2009-01-25 at 19:56 +0200, Heikki Linnakangas wrote:
>> Simon Riggs wrote:
>>>> 2. Kill all connections by given user. Hmm, not used for anything,
>>>> actually. Should remove the roleId argument from GetConflictingVirtualXIDs.
>>> No, because we still need to add code to kill-connected-users if we drop
>>> role.
>> Oh, I see, that's still a todo item. But we don't do that during normal
>> operation, so why should we during recovery?
> LOL. Not many systems allow you to continue working after the access has
> been removed. But not, as you say, a problem for Hot Standby unless that
> changes.
Just like around every unixish system?

Andres