Обсуждение: Hot Standby, max_connections and max_prepared_transactions
We discussed earlier that HS should continue to work even if max_connections was set differently on the primary and the standby. This now gives a situation where snapshots can be allowed, then disallowed for a while, then allowed again. Complication is that this will cause some connections to fail since we take a snapshot in postinit.c. (That is the part I just noticed in my self-review). Some queries will also fail. Sometimes, not all the time. This makes both behaviour and coding more complicated and my feeling is that if we are aiming for simplicity in all areas we should remove this. Currently max_prepared_transactions needs to be set correctly in recovery also, so this complex coding doesn't actually remove the need to set some parameters correctly. Not many people change them from the default in the first place, so I don't think its a big deal. And most people use the same postgresql.conf on the standby anyway. I propose we just accept that both max_connections and max_prepared_transactions need to be set correctly for recovery to work. This will make the state transitions more robust and it will avoid spurious and hard to test error messages. Any objections to me removing this slice of code from the patch? -- Simon Riggs www.2ndQuadrant.com
Simon Riggs wrote: > I propose we just accept that both max_connections and > max_prepared_transactions need to be set correctly for recovery to work. > This will make the state transitions more robust and it will avoid > spurious and hard to test error messages. > > Any objections to me removing this slice of code from the patch? Umm, what slice of code? I don't recall any code trying to make it work. -- Heikki Linnakangas EnterpriseDB http://www.enterprisedb.com
On Thu, 2009-09-03 at 22:22 +0300, Heikki Linnakangas wrote: > Simon Riggs wrote: > > I propose we just accept that both max_connections and > > max_prepared_transactions need to be set correctly for recovery to work. > > This will make the state transitions more robust and it will avoid > > spurious and hard to test error messages. > > > > Any objections to me removing this slice of code from the patch? > > Umm, what slice of code? I don't recall any code trying to make it work. Well, its there. Perhaps the full functionality has been clipped in recent changes, but there are still unwanted ramifications in the design that I think would be best to remove. No loss of functionality, just HS won't activate unless max_connections is set >= value on primary. Since max_prepared_transactions already has this same problem in the current code I see no reason to fuss. We can always put in more flexible code later. State change code in StartupXLog(), Snapshots disabled code in GetSnapshotData(), ProcArray->allowStandbySnapshots etc.. -- Simon Riggs www.2ndQuadrant.com
Simon Riggs <simon@2ndQuadrant.com> writes:
> On Thu, 2009-09-03 at 22:22 +0300, Heikki Linnakangas wrote:
>> Simon Riggs wrote:
>>> I propose we just accept that both max_connections and
>>> max_prepared_transactions need to be set correctly for recovery to work.
>>> This will make the state transitions more robust and it will avoid
>>> spurious and hard to test error messages.
>>> Any objections to me removing this slice of code from the patch?
>> Umm, what slice of code? I don't recall any code trying to make it work.
> Well, its there.
Just to be clear: you're proposing requiring that these be set the
same on master and slave? I don't have a problem with that, but
I do suggest that we must provide a mechanism to check it --- I don't
want DBAs to be faced with obscure failures when (not if) they
mess it up. Perhaps include the values in checkpoint WAL records?
regards, tom lane
On Fri, 2009-09-04 at 01:25 -0400, Tom Lane wrote: > Simon Riggs <simon@2ndQuadrant.com> writes: > > On Thu, 2009-09-03 at 22:22 +0300, Heikki Linnakangas wrote: > >> Simon Riggs wrote: > >>> I propose we just accept that both max_connections and > >>> max_prepared_transactions need to be set correctly for recovery to work. > >>> This will make the state transitions more robust and it will avoid > >>> spurious and hard to test error messages. > >>> Any objections to me removing this slice of code from the patch? > > >> Umm, what slice of code? I don't recall any code trying to make it work. > > > Well, its there. > > Just to be clear: you're proposing requiring that these be set the > same on master and slave? Yes, or more precisely: Slave value >= Master value > I don't have a problem with that, but > I do suggest that we must provide a mechanism to check it --- I don't > want DBAs to be faced with obscure failures when (not if) they > mess it up. Perhaps include the values in checkpoint WAL records? Good plan. We can generate an immediate message at startup. -- Simon Riggs www.2ndQuadrant.com
On Thu, Sep 3, 2009 at 5:50 PM, Simon Riggs<simon@2ndquadrant.com> wrote: > > On Thu, 2009-09-03 at 22:22 +0300, Heikki Linnakangas wrote: >> Simon Riggs wrote: >> > I propose we just accept that both max_connections and >> > max_prepared_transactions need to be set correctly for recovery to work. >> > This will make the state transitions more robust and it will avoid >> > spurious and hard to test error messages. >> > >> > Any objections to me removing this slice of code from the patch? >> >> Umm, what slice of code? I don't recall any code trying to make it work. > > Well, its there. Perhaps the full functionality has been clipped in > recent changes, but there are still unwanted ramifications in the design > that I think would be best to remove. No loss of functionality, just HS > won't activate unless max_connections is set >= value on primary. I'm not sure if you're referring to the work that I did, but if so it's not nearly that significant. I haven't done anything much beyond some code cleanup. Hopefully you're planning to keep that part, as it would be a shame if I had done it for nothing. Can't wait to see the revised patch. ...Robert
On Fri, 2009-09-04 at 09:26 -0400, Robert Haas wrote: > On Thu, Sep 3, 2009 at 5:50 PM, Simon Riggs<simon@2ndquadrant.com> wrote: > > > > On Thu, 2009-09-03 at 22:22 +0300, Heikki Linnakangas wrote: > >> Simon Riggs wrote: > >> > I propose we just accept that both max_connections and > >> > max_prepared_transactions need to be set correctly for recovery to work. > >> > This will make the state transitions more robust and it will avoid > >> > spurious and hard to test error messages. > >> > > >> > Any objections to me removing this slice of code from the patch? > >> > >> Umm, what slice of code? I don't recall any code trying to make it work. > > > > Well, its there. Perhaps the full functionality has been clipped in > > recent changes, but there are still unwanted ramifications in the design > > that I think would be best to remove. No loss of functionality, just HS > > won't activate unless max_connections is set >= value on primary. > > I'm not sure if you're referring to the work that I did, but if so > it's not nearly that significant. No, this was introduced in Jan/Feb. -- Simon Riggs www.2ndQuadrant.com
On Fri, 2009-09-04 at 09:26 -0400, Robert Haas wrote: > Hopefully you're planning to keep that part, as it > would be a shame if I had done it for nothing. Not promising anything in that regard, but I appreciate your efforts to assist. I doubt it was wasted effort in any sense. It will certainly be valuable to have another knowledgeable reviewer. I'm finding reviewing my own code considerably easier after the break in development. Not done yet, but making good progress every day. -- Simon Riggs www.2ndQuadrant.com