Hot standby, recent changes

From: Heikki Linnakangas <heikki(dot)linnakangas(at)enterprisedb(dot)com>
To: Simon Riggs <simon(at)2ndquadrant(dot)com>
Cc: PostgreSQL-development <pgsql-hackers(at)postgresql(dot)org>
Subject: Hot standby, recent changes
Date: 2009-12-06 10:32:04
Message-ID: 4B1B8824.2070808@enterprisedb.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

1. The XLogFlush() call you added to dbase_redo doesn't help where it
is. You need to call XLogFlush() after the *commit* record of the DROP
DATABASE. The idea is minimize the window where the files have already
been deleted but the entry in pg_database is still visible, if someone
kills the standby and starts it up as a new master. This isn't really
hot standby's fault, you have the same window with PITR, so I think it
would be better to handle this as a separate patch. Also, please handle
all the commands that use ForceSyncCommit().

2. "Allow RULEs ON INSERT, ON UPDATE and ON DELETE iff they generate
only SELECT statements that act INSTEAD OF the actual event." This
affects any read-only transaction, not just hot standby, so I think this
should be discussed and changed as separate patch.

3. The "Out of lock mem killer" in StandbyAcquireAccessExclusiveLock is
quite harsh. It aborts all read-only transactions. It should be enough
to kill just one random one, or maybe the one that's holding most locks.
Also, if there still isn't enough shared memory after killing all
backends, it goes into an infinite loop. I guess that shouldn't happen,
but it seems a bit squishy anyway. It would be nice to differentiate
between "out of shared mem" and "someone else is holding the lock" more
accurately. Maybe add a new return value to LockAcquire() for "out of
shared mem".

4. Need to handle the case where master is started up with
wal_standby_info=true, shut down, and restarted with
wal_standby_info=false, while the standby server runs continuously. And
the code in StartupXLog() to initialize recovery snapshot from a
shutdown checkpoint needs to check that too.

5. You removed this comment from KnownAssignedXidsAdd:

- /*
- * XXX: We should check that we don't exceed maxKnownAssignedXids.
- * Even though the hash table might hold a few more entries than that,
- * we use fixed-size arrays of that size elsewhere and expected all
- * entries in the hash table to fit.
- */

I think the issue still exists. The comment refers to
KnownAssignedXidsGet, which takes as an argument an array that has to be
large enough to hold all entries in the known-assigned hash table. If
there's more entries than expected in the hash table,
KnownAssignedXidsGet will overrun the array. Perhaps
KnownAssignedXidsGet should return a palloc'd array instead or
something, but I don't think it's fine as it is.

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

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Hannu Krosing 2009-12-06 10:43:27 Re: Cancelling idle in transaction state
Previous Message Kurt Harriman 2009-12-06 10:21:42 Re: Patch: Remove gcc dependency in definition of inline functions