On Sun, 2009-12-06 at 12:32 +0200, Heikki Linnakangas wrote:
> 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.
OK
> 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.
I don't really understand the use case for shutting down the server and
then using it as a HS base backup. Why would anyone do that? Why would
they have their server down for potentially hours, when they can take
the backup while the server is up? If the server is idle, it can be
up-and-idle just as easily as down-and-idle, in which case we wouldn't
need to support this at all. Adding yards of code for this capability
isn't important to me. I'd rather strip the whole lot out than keep
fiddling with a low priority area. Please justify this as a real world
solution before we continue trying to support it.
> 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.
Same issue perhaps, different place.
-- Simon Riggs www.2ndQuadrant.com
