Обсуждение: max_worker_processes on the standby
Hi,
"25.5.3. Administrator's Overview" in the document
-----------------------------------------------------
The setting of some parameters on the standby will need reconfiguration
if they have been changed on the primary. For these parameters,
the value on the standby must be equal to or greater than the value
on the primary. If these parameters are not set high enough then
the standby will refuse to start. Higher values can then be supplied
and the server restarted to begin recovery again. These parameters are:
max_connections
max_prepared_transactions
max_locks_per_transaction
-----------------------------------------------------
I found that the value of max_worker_processes on the standby also
must be equal to or greater than the value on the master. So we should
just add max_worker_processes to this paragraph. Patch attached.
Regards,
--
Fujii Masao
Вложения
Fujii Masao wrote: > Hi, > > "25.5.3. Administrator's Overview" in the document > ----------------------------------------------------- > The setting of some parameters on the standby will need reconfiguration > if they have been changed on the primary. For these parameters, > the value on the standby must be equal to or greater than the value > on the primary. If these parameters are not set high enough then > the standby will refuse to start. Higher values can then be supplied > and the server restarted to begin recovery again. These parameters are: > max_connections > max_prepared_transactions > max_locks_per_transaction > ----------------------------------------------------- > > I found that the value of max_worker_processes on the standby also > must be equal to or greater than the value on the master. So we should > just add max_worker_processes to this paragraph. Patch attached. True. Also track_commit_timestamp. Can you add a comment somewhere in CheckRequiredParameterValues(void) that the set of parameters is listed in section such-and-such in the docs, so that next time there's a higher chance that the docs are kept up to date? -- Álvaro Herrera http://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
On Wed, Jul 15, 2015 at 5:57 PM, Alvaro Herrera <alvherre@2ndquadrant.com> wrote: > Fujii Masao wrote: >> Hi, >> >> "25.5.3. Administrator's Overview" in the document >> ----------------------------------------------------- >> The setting of some parameters on the standby will need reconfiguration >> if they have been changed on the primary. For these parameters, >> the value on the standby must be equal to or greater than the value >> on the primary. If these parameters are not set high enough then >> the standby will refuse to start. Higher values can then be supplied >> and the server restarted to begin recovery again. These parameters are: >> max_connections >> max_prepared_transactions >> max_locks_per_transaction >> ----------------------------------------------------- >> >> I found that the value of max_worker_processes on the standby also >> must be equal to or greater than the value on the master. So we should >> just add max_worker_processes to this paragraph. Patch attached. > > True. Also track_commit_timestamp. Yes, but I intentionally did not include track_commit_timestamp in the patch because it's not easy for me to document the hot standby condition of track_commit_timestamp unless I read the code more. One example which makes me a bit confusing is; both master and standby are running fine with track_commit_timestamp disabled, then I enable it only on the master. That is, the value of track_commit_timestamp is not the same between master and standby. No error happens in this case. According to the code of xlog_redo(), the commit timestamp tracking mechanism is activated in this case. However, after that, if standby is restarted, standby emits an error because the value of track_commit_timestamp is not the same between master and standby. Simple question is; why do we need to cause the standby fail in this case? Since I'm not familiar with the code of track_commit_timestamp yet, I'm not sure whether this behavior is valid or not. > Can you add a comment somewhere in > CheckRequiredParameterValues(void) that the set of parameters is listed > in section such-and-such in the docs, so that next time there's a higher > chance that the docs are kept up to date? +1 What about the attached patch? Regards, -- Fujii Masao
Вложения
On Thu, Jul 16, 2015 at 12:07 AM, Fujii Masao <masao.fujii@gmail.com> wrote: > One example which makes me a bit confusing is; both master and > standby are running fine with track_commit_timestamp disabled, > then I enable it only on the master. That is, the value of > track_commit_timestamp is not the same between master and standby. > No error happens in this case. According to the code of xlog_redo(), > the commit timestamp tracking mechanism is activated in this case. > However, after that, if standby is restarted, standby emits an error > because the value of track_commit_timestamp is not the same between > master and standby. Simple question is; why do we need to cause > the standby fail in this case? Since I'm not familiar with the code of > track_commit_timestamp yet, I'm not sure whether this behavior is > valid or not. Hmm, that seems like awfully weird behavior. -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company
Robert Haas wrote: > On Thu, Jul 16, 2015 at 12:07 AM, Fujii Masao <masao.fujii@gmail.com> wrote: > > One example which makes me a bit confusing is; both master and > > standby are running fine with track_commit_timestamp disabled, > > then I enable it only on the master. That is, the value of > > track_commit_timestamp is not the same between master and standby. > > No error happens in this case. According to the code of xlog_redo(), > > the commit timestamp tracking mechanism is activated in this case. Well. We could have the standby fail (i.e. stop replication) when it receives the WAL record indicating that the master turned it on. But that would be very unfriendly, so we chose to make it follow the master config instead. We're okay with this part, yes? > > However, after that, if standby is restarted, standby emits an error > > because the value of track_commit_timestamp is not the same between > > master and standby. Simple question is; why do we need to cause > > the standby fail in this case? Since I'm not familiar with the code of > > track_commit_timestamp yet, I'm not sure whether this behavior is > > valid or not. > > Hmm, that seems like awfully weird behavior. The alternative is to turn the feature on automatically if it sees that the master also has it on, i.e. the value would not be what the config file says it is. Doing this would be a bit surprising IMO, but given the behavior above maybe it's better than the current behavior. -- Álvaro Herrera http://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
On Tue, Aug 4, 2015 at 12:41 AM, Alvaro Herrera <alvherre@2ndquadrant.com> wrote: > Robert Haas wrote: >> On Thu, Jul 16, 2015 at 12:07 AM, Fujii Masao <masao.fujii@gmail.com> wrote: >> > One example which makes me a bit confusing is; both master and >> > standby are running fine with track_commit_timestamp disabled, >> > then I enable it only on the master. That is, the value of >> > track_commit_timestamp is not the same between master and standby. >> > No error happens in this case. According to the code of xlog_redo(), >> > the commit timestamp tracking mechanism is activated in this case. > > Well. We could have the standby fail (i.e. stop replication) when it > receives the WAL record indicating that the master turned it on. But > that would be very unfriendly, so we chose to make it follow the > master config instead. We're okay with this part, yes? > >> > However, after that, if standby is restarted, standby emits an error >> > because the value of track_commit_timestamp is not the same between >> > master and standby. Simple question is; why do we need to cause >> > the standby fail in this case? Since I'm not familiar with the code of >> > track_commit_timestamp yet, I'm not sure whether this behavior is >> > valid or not. >> >> Hmm, that seems like awfully weird behavior. > > The alternative is to turn the feature on automatically if it sees that > the master also has it on, i.e. the value would not be what the config > file says it is. Doing this would be a bit surprising IMO, but given > the behavior above maybe it's better than the current behavior. I think it's totally reasonable for the standby to follow the master's behavior rather than the config file. That should be documented, but otherwise, no problem. If it were technologically possible for the standby to follow the config file rather than the master in all cases, that would be fine, too. But the current behavior is somewhere in the middle, and that doesn't seem like a good plan. -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company
Adding CC to hackers, since this is clearly not just a docs issue. Also CCing Petr and Craig since they are the ones that know how this is used in BDR. Robert Haas wrote: > On Tue, Aug 4, 2015 at 12:41 AM, Alvaro Herrera > <alvherre@2ndquadrant.com> wrote: > > The alternative is to turn the feature on automatically if it sees that > > the master also has it on, i.e. the value would not be what the config > > file says it is. Doing this would be a bit surprising IMO, but given > > the behavior above maybe it's better than the current behavior. > > I think it's totally reasonable for the standby to follow the master's > behavior rather than the config file. That should be documented, but > otherwise, no problem. If it were technologically possible for the > standby to follow the config file rather than the master in all cases, > that would be fine, too. But the current behavior is somewhere in the > middle, and that doesn't seem like a good plan. So I discussed this with Petr. He points out that if we make the standby follows the master, then the problem would be the misbehavior that results once the standby is promoted: at that point the standby would no longer "follow the master" and would start with the feature turned off, which could be disastrous (depending on what are you using the commit timestamps for). To solve that problem, you could suggest that if we see the setting turned on in pg_control then we should follow that instead of the config file; but then the problem is that there's no way to turn the feature off. And things are real crazy by then. Given this, we're leaning towards the idea that the standby should not try to follow the master at all. Instead, an extension that wants to use this stuff can check the value for itself, and raise a fatal error if it's not already turned on the config file. That way, a lot of the strange corner cases disappear. -- Álvaro Herrera http://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
On 2015-08-05 00:13, Alvaro Herrera wrote: > Robert Haas wrote: >> On Tue, Aug 4, 2015 at 12:41 AM, Alvaro Herrera >> <alvherre@2ndquadrant.com> wrote: > >>> The alternative is to turn the feature on automatically if it sees that >>> the master also has it on, i.e. the value would not be what the config >>> file says it is. Doing this would be a bit surprising IMO, but given >>> the behavior above maybe it's better than the current behavior. >> >> I think it's totally reasonable for the standby to follow the master's >> behavior rather than the config file. That should be documented, but >> otherwise, no problem. If it were technologically possible for the >> standby to follow the config file rather than the master in all cases, >> that would be fine, too. But the current behavior is somewhere in the >> middle, and that doesn't seem like a good plan. > > So I discussed this with Petr. He points out that if we make the > standby follows the master, then the problem would be the misbehavior > that results once the standby is promoted: at that point the standby > would no longer "follow the master" and would start with the feature > turned off, which could be disastrous (depending on what are you using > the commit timestamps for). > > Given this, we're leaning towards the idea that the standby should not > try to follow the master at all. Instead, an extension that wants to > use this stuff can check the value for itself, and raise a fatal error > if it's not already turned on the config file. That way, a lot of the > strange corner cases disappear. > Actually, after thinking bit more about this I think the behavior of these two will be similar - you suddenly lose the commit timestamp info. The difference is that with fist option you'll lose it after restart while with second one you lose it immediately after promotion since there was never any info on the slave. Extensions can do sanity checking in both scenarios. The way I see it the first option has following advantages: - it's smaller change - it's more consistent with how wal_log_hints behaves - fixing the config does not require server restart since the in-memory state was set from WAL record automatically However the second option has also some: - one can have slave which doesn't have overhead of the commit timestamp SLRU if they don't need it there - it's theoretically easier to notice that the track_commit_timestamps is off in config because the the SQL interface will complain if called on the slave So +0.5 from me towards following master and removing the error message -- Petr Jelinek http://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Training & Services
On Tue, Aug 4, 2015 at 6:13 PM, Alvaro Herrera <alvherre@2ndquadrant.com> wrote: >> I think it's totally reasonable for the standby to follow the master's >> behavior rather than the config file. That should be documented, but >> otherwise, no problem. If it were technologically possible for the >> standby to follow the config file rather than the master in all cases, >> that would be fine, too. But the current behavior is somewhere in the >> middle, and that doesn't seem like a good plan. > > So I discussed this with Petr. He points out that if we make the > standby follows the master, then the problem would be the misbehavior > that results once the standby is promoted: at that point the standby > would no longer "follow the master" and would start with the feature > turned off, which could be disastrous (depending on what are you using > the commit timestamps for). That seems like an imaginary problem. If it's critical to have commit timestamps, don't turn them off on the standby. > To solve that problem, you could suggest that if we see the setting > turned on in pg_control then we should follow that instead of the config > file; but then the problem is that there's no way to turn the feature > off. And things are real crazy by then. There's no existing precedent for a feature that lets the standby be different from the master *in any way*. So I don't see why we should start here. I think the reasonable definition is that the GUC controls whether the master tries to update the SLRU (and generate appropriate WAL records, presumably). The standby should not get a choice about whether to replay those WAL records. Note that if you do allow the standby to decide not to replay the WAL records for this feature, then the data on the standby could be partially there but not completely there after promotion, because the DBA might have flipped the switch on and off at different times. -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company
On Sat, Aug 8, 2015 at 11:02 PM, Robert Haas <robertmhaas@gmail.com> wrote: > On Tue, Aug 4, 2015 at 6:13 PM, Alvaro Herrera <alvherre@2ndquadrant.com> wrote: >>> I think it's totally reasonable for the standby to follow the master's >>> behavior rather than the config file. That should be documented, but >>> otherwise, no problem. If it were technologically possible for the >>> standby to follow the config file rather than the master in all cases, >>> that would be fine, too. But the current behavior is somewhere in the >>> middle, and that doesn't seem like a good plan. >> >> So I discussed this with Petr. He points out that if we make the >> standby follows the master, then the problem would be the misbehavior >> that results once the standby is promoted: at that point the standby >> would no longer "follow the master" and would start with the feature >> turned off, which could be disastrous (depending on what are you using >> the commit timestamps for). > > That seems like an imaginary problem. If it's critical to have commit > timestamps, don't turn them off on the standby. > >> To solve that problem, you could suggest that if we see the setting >> turned on in pg_control then we should follow that instead of the config >> file; but then the problem is that there's no way to turn the feature >> off. And things are real crazy by then. > > There's no existing precedent for a feature that lets the standby be > different from the master *in any way*. So I don't see why we should > start here. I think the reasonable definition is that the GUC > controls whether the master tries to update the SLRU (and generate > appropriate WAL records, presumably). The standby should not get a > choice about whether to replay those WAL records. +1 I added this to the 9.5 open item list. Regards, -- Fujii Masao
On Thu, Jul 16, 2015 at 1:07 PM, Fujii Masao <masao.fujii@gmail.com> wrote: > On Wed, Jul 15, 2015 at 5:57 PM, Alvaro Herrera > <alvherre@2ndquadrant.com> wrote: >> Fujii Masao wrote: >>> Hi, >>> >>> "25.5.3. Administrator's Overview" in the document >>> ----------------------------------------------------- >>> The setting of some parameters on the standby will need reconfiguration >>> if they have been changed on the primary. For these parameters, >>> the value on the standby must be equal to or greater than the value >>> on the primary. If these parameters are not set high enough then >>> the standby will refuse to start. Higher values can then be supplied >>> and the server restarted to begin recovery again. These parameters are: >>> max_connections >>> max_prepared_transactions >>> max_locks_per_transaction >>> ----------------------------------------------------- >>> >>> I found that the value of max_worker_processes on the standby also >>> must be equal to or greater than the value on the master. So we should >>> just add max_worker_processes to this paragraph. Patch attached. >> >> True. Also track_commit_timestamp. > > Yes, but I intentionally did not include track_commit_timestamp in > the patch because it's not easy for me to document the hot standby > condition of track_commit_timestamp unless I read the code more. > > One example which makes me a bit confusing is; both master and > standby are running fine with track_commit_timestamp disabled, > then I enable it only on the master. That is, the value of > track_commit_timestamp is not the same between master and standby. > No error happens in this case. According to the code of xlog_redo(), > the commit timestamp tracking mechanism is activated in this case. > However, after that, if standby is restarted, standby emits an error > because the value of track_commit_timestamp is not the same between > master and standby. Simple question is; why do we need to cause > the standby fail in this case? Since I'm not familiar with the code of > track_commit_timestamp yet, I'm not sure whether this behavior is > valid or not. > >> Can you add a comment somewhere in >> CheckRequiredParameterValues(void) that the set of parameters is listed >> in section such-and-such in the docs, so that next time there's a higher >> chance that the docs are kept up to date? > > +1 > > What about the attached patch? Applied the patch. Regards, -- Fujii Masao
On 2015-09-03 15:03, Fujii Masao wrote: > On Sat, Aug 8, 2015 at 11:02 PM, Robert Haas <robertmhaas@gmail.com> wrote: >> There's no existing precedent for a feature that lets the standby be >> different from the master *in any way*. So I don't see why we should >> start here. I think the reasonable definition is that the GUC >> controls whether the master tries to update the SLRU (and generate >> appropriate WAL records, presumably). The standby should not get a >> choice about whether to replay those WAL records. > > +1 > > I added this to the 9.5 open item list. > I see I forgot to send patch for this, so here it is. It just removes the on start check for track_commit_timestamp being same in config and control file. -- Petr Jelinek http://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Training & Services
Вложения
oonishitk@nttdata.co.jp wrote: > The below error messages were shown in standby server log: > > FATAL: could not access status of transaction 9009 > DETAIL: Could not read from file "pg_commit_ts/0000" at offset 90112: Success. > CONTEXT: xlog redo Transaction/COMMIT: 2015-09-30 15:52:41.924141+09 > LOG: startup process (PID 23199) exited with exit code 1 > LOG: terminating any other active server processes > > Before this FATAL, there were some INFO but annoying messages: > > LOG: file "pg_commit_ts/0000" doesn't exist, reading as zeroes Here's a patch. I went over the commit_ts.c code a few more times. I eventually realized that we were trying to update the value of the GUC, which is a rather unreliable thing to do; this was made worse by the fact that we were updating it in one process only. I thought it was better to have a separate boolean flag, affecting the recovery process only, which is set at startup time or when the XLOG_PARAMETER_CHANGE message is received. The module is enabled if either the GUC is set or we see that the master has the module enabled. This only enables it as far as replaying xlog records though: if you use the SQL interface, it will still raise an error that you cannot read values unless the GUC is enabled. This seems fine to me. A curious but benign effect of this patch is that if you have the module disabled in the master but enabled in the standby, you can actually query the commit times in the standby, and they will correspond to whatever the master used in the commit xlog record. Other small changes: - Moved some code out of xlog_redo into a new public commit_ts.c routine; made ActivateCommitTs and Deactivate* statics. - In the previous commit I added an assert that we're not writing xlog and replaying xlog at the same time. This is pointless because xlog.c already complains about that, so this commit takes it out again. -- Álvaro Herrera http://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
Вложения
> Here's a patch. Thank you! With this patch, the standby server down disappears in my environment. Regards, ======== Takashi Ohnishi oonishitk@nttdata.co.jp -----Original Message----- From: Alvaro Herrera [mailto:alvherre@2ndquadrant.com] Sent: Thursday, October 01, 2015 7:48 AM To: Fujii Masao <masao.fujii@gmail.com>; SPS 大西 高史(三技術) <oonishitk@nttdata.co.jp> Cc: pgsql-docs <pgsql-docs@postgresql.org>; pgsql-hackers@postgresql.org Subject: Re: [DOCS] max_worker_processes on the standby oonishitk@nttdata.co.jp wrote: > The below error messages were shown in standby server log: > > FATAL: could not access status of transaction 9009 > DETAIL: Could not read from file "pg_commit_ts/0000" at offset 90112: Success. > CONTEXT: xlog redo Transaction/COMMIT: 2015-09-30 15:52:41.924141+09 > LOG: startup process (PID 23199) exited with exit code 1 > LOG: terminating any other active server processes > > Before this FATAL, there were some INFO but annoying messages: > > LOG: file "pg_commit_ts/0000" doesn't exist, reading as zeroes Here's a patch. I went over the commit_ts.c code a few more times. I eventually realized that we were trying to update the value of theGUC, which is a rather unreliable thing to do; this was made worse by the fact that we were updating it in one processonly. I thought it was better to have a separate boolean flag, affecting the recovery process only, which is set at startup timeor when the XLOG_PARAMETER_CHANGE message is received. The module is enabled if either the GUC is set or we see thatthe master has the module enabled. This only enables it as far as replaying xlog records though: if you use the SQL interface, it will still raise an errorthat you cannot read values unless the GUC is enabled. This seems fine to me. A curious but benign effect of this patch is that if you have the module disabled in the master but enabled in the standby,you can actually query the commit times in the standby, and they will correspond to whatever the master used in thecommit xlog record. Other small changes: - Moved some code out of xlog_redo into a new public commit_ts.c routine; made ActivateCommitTs and Deactivate* statics. - In the previous commit I added an assert that we're not writing xlog and replaying xlog at the same time. This is pointlessbecause xlog.c already complains about that, so this commit takes it out again. -- Álvaro Herrera http://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
On Thu, Oct 1, 2015 at 7:48 AM, Alvaro Herrera <alvherre@2ndquadrant.com> wrote: > oonishitk@nttdata.co.jp wrote: > >> The below error messages were shown in standby server log: >> >> FATAL: could not access status of transaction 9009 >> DETAIL: Could not read from file "pg_commit_ts/0000" at offset 90112: Success. >> CONTEXT: xlog redo Transaction/COMMIT: 2015-09-30 15:52:41.924141+09 >> LOG: startup process (PID 23199) exited with exit code 1 >> LOG: terminating any other active server processes >> >> Before this FATAL, there were some INFO but annoying messages: >> >> LOG: file "pg_commit_ts/0000" doesn't exist, reading as zeroes > > Here's a patch. I've not read the patch yet, but the patched server with track_commit_timestamp enabled caused the following PANIC error when I ran pgbench. PANIC: could not access status of transaction 2457 DETAIL: Could not read from file "pg_commit_ts/0000" at offset 24576: Success. STATEMENT: END; The procedure to reproduce the PANIC error is, 1. Enable track_commit_timestamp 2. Start up the server 3. Run pgbench -i -s10 4. Run pgbench -j 4 -c 4 -T 30 Regards, -- Fujii Masao
Fujii Masao wrote: > I've not read the patch yet, but the patched server with track_commit_timestamp > enabled caused the following PANIC error when I ran pgbench. Ah, that was a stupid typo: I used || instead of &&. Fixed that. I also changed DeactivateCommitTs() so that it removes all slru segments instead of leaving the last one around (which is what SimpleLruTruncate was doing). This was noticeable when you ran a server with the feature enabled (which created some files), then disabled it (which removed all but the last) and ran for some more xacts; then enabled it again (which created a new file, far ahead from the previously existing one). Since the feature has enough protections that it doesn't have a problem with no files at all being present, this works correctly. Note no wal-logging of this operation: it's not necessary AFAICS because the same deactivation routine would be called again in recovery and in XLOG_PARAMETER_CHANGE, so it should be safe. And pushed. Thanks! -- Álvaro Herrera http://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
On Fri, Oct 2, 2015 at 3:12 AM, Alvaro Herrera <alvherre@2ndquadrant.com> wrote:
> Fujii Masao wrote:
>
>> I've not read the patch yet, but the patched server with track_commit_timestamp
>> enabled caused the following PANIC error when I ran pgbench.
>
> Ah, that was a stupid typo: I used || instead of &&. Fixed that.
>
> I also changed DeactivateCommitTs() so that it removes all slru segments
> instead of leaving the last one around (which is what SimpleLruTruncate
> was doing). This was noticeable when you ran a server with the feature
> enabled (which created some files), then disabled it (which removed all
> but the last) and ran for some more xacts; then enabled it again (which
> created a new file, far ahead from the previously existing one). Since
> the feature has enough protections that it doesn't have a problem with
> no files at all being present, this works correctly. Note no
> wal-logging of this operation: it's not necessary AFAICS because the
> same deactivation routine would be called again in recovery and in
> XLOG_PARAMETER_CHANGE, so it should be safe.
What happens if pg_xact_commit_timestamp() is called in standby after
track_commit_timestamp is disabled in master, DeactivateCommitTs() is
called and all commit_ts files are removed in standby? I tried that case
and got the following assertion failure.
TRAP: FailedAssertion("!(((oldestCommitTs) != ((TransactionId) 0)) ==
((newestCommitTs) != ((TransactionId) 0)))", File: "commit_ts.c",
Line: 307)
LOG: server process (PID 11160) was terminated by signal 6: Aborted
DETAIL: Failed process was running: select num,
pg_xact_commit_timestamp(num::text::xid) from generate_series(1750,
1800) num
The steps to reproduce the problem is
1. Set up replication with track_commit_timestamp enabled.
2. Run several write transactions.
3. Disable track_commit_timestamp only in master and wait for
XLOG_PARAMETER_CHANGE record to be replayed in standby.
4. Run pg_xact_commit_timestamp() in standby.
Regards,
--
Fujii Masao
Fujii Masao wrote: > What happens if pg_xact_commit_timestamp() is called in standby after > track_commit_timestamp is disabled in master, DeactivateCommitTs() is > called and all commit_ts files are removed in standby? I tried that case > and got the following assertion failure. Ah. So the standby needs to keep the module activated if it's enabled locally, even when it receives a message that the master turned it off. Here's a patch. Thanks for your continued testing! -- Álvaro Herrera http://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
Вложения
On Fri, Oct 2, 2015 at 10:58 AM, Alvaro Herrera <alvherre@2ndquadrant.com> wrote: > Fujii Masao wrote: > >> What happens if pg_xact_commit_timestamp() is called in standby after >> track_commit_timestamp is disabled in master, DeactivateCommitTs() is >> called and all commit_ts files are removed in standby? I tried that case >> and got the following assertion failure. > > Ah. So the standby needs to keep the module activated if it's enabled > locally, even when it receives a message that the master turned it off. > Here's a patch. The standby can have the feature enabled even though the master has it disabled? That seems like it can only lead to heartache. -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company
Robert Haas wrote: > The standby can have the feature enabled even though the master has it > disabled? That seems like it can only lead to heartache. Can you elaborate? -- Álvaro Herrera http://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
On Fri, Oct 2, 2015 at 2:59 PM, Alvaro Herrera <alvherre@2ndquadrant.com> wrote: > Robert Haas wrote: >> The standby can have the feature enabled even though the master has it >> disabled? That seems like it can only lead to heartache. > > Can you elaborate? Sort of. Our rule up until now has always been that the standby is an exact copy of the master. I suspect deviating from that behavior will introduce bugs. I suspect having the standby make data changes that aren't WAL-logged will introduce bugs; not to be unkind, but that certainly seems like a lesson to take from what happened with multixacts. I haven't looked at this code well enough to guess specifically what will go wrong. But consider people turning the feature on and off repeatedly on the master, and separately on the standby, combined with crashes on the standby that restart replay from earlier points (possibly with settings that have changed in the meantime). Are we really sure that we're never going to end up with the wrong files, or inconsistent ones, on the standby? I have a really hard time believing that's going to work out. -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company
On Fri, Oct 2, 2015 at 11:58 PM, Alvaro Herrera <alvherre@2ndquadrant.com> wrote: > Fujii Masao wrote: > >> What happens if pg_xact_commit_timestamp() is called in standby after >> track_commit_timestamp is disabled in master, DeactivateCommitTs() is >> called and all commit_ts files are removed in standby? I tried that case >> and got the following assertion failure. > > Ah. So the standby needs to keep the module activated if it's enabled > locally, even when it receives a message that the master turned it off. > Here's a patch. I'm afraid that this behavior might confuse the users. Please imagine the following scenario. 1. start up the server with track_commit_timestamp disabled 2. run several transactions 3. shut down the server with immediate mode 4. restart the server with track_commit_timestamp enabled 5. run "SELECT pg_last_committed_xact()" 6. run "SELECT pg_xact_commit_timestamp(xid) FROM pg_last_committed_xact()" 7. restart the server 8. run "SELECT pg_last_committed_xact()" Firstly, in #5, pg_last_committed_xact() returns the XID and timestamp of the last transaction which was executed in #2 (i.e., while track_commit_timestamp was disabled). This is confusing. I think that both pg_last_committed_xact() and pg_xact_commit_timestamp() should return only the transaction which was executed with track_commit_timestamp enabled. Secondly, SELECT query in #6 returns NULL. This means that pg_xact_commit_timestamp() may not handle the transaction which pg_last_committed_xact() handles. This is also confusing. Finally, in #8, pg_last_committed_xact() returns NULL while it returned non-NULL before the restart. This is also confusing. Regards, -- Fujii Masao
On 2015-10-02 22:02, Robert Haas wrote: > On Fri, Oct 2, 2015 at 2:59 PM, Alvaro Herrera <alvherre@2ndquadrant.com> wrote: >> Robert Haas wrote: >>> The standby can have the feature enabled even though the master has it >>> disabled? That seems like it can only lead to heartache. >> >> Can you elaborate? > > Sort of. Our rule up until now has always been that the standby is an > exact copy of the master. I suspect deviating from that behavior will > introduce bugs. I suspect having the standby make data changes that > aren't WAL-logged will introduce bugs; not to be unkind, but that > certainly seems like a lesson to take from what happened with > multixacts. > I agree with that sentiment. Attached patch adds variable to the shmem which is used for module activation tracking - set to true in ActiveCommitTs() and false in DeactivateCommitTs(). All the checks inside the commit_ts code were changed to use this new variable. I also removed the static variable Alvaro added in previous commit because it's not needed anymore. The patch also does full cleanup of the shmem state in DeactivateCommitTs() so that standby does not have stale last committed transaction info after enable/disable/enable cycle on primary I also removed no longer used do_wal parameters in couple of functions. -- Petr Jelinek http://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Training & Services
Вложения
On Sat, Oct 17, 2015 at 5:37 PM, Petr Jelinek <petr@2ndquadrant.com> wrote: > I agree with that sentiment. > > Attached patch adds variable to the shmem which is used for module > activation tracking - set to true in ActiveCommitTs() and false in > DeactivateCommitTs(). All the checks inside the commit_ts code were changed > to use this new variable. I also removed the static variable Alvaro added in > previous commit because it's not needed anymore. That sounds good to me. On a quick read-through it looks OK too. -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company
Robert Haas wrote: > On Sat, Oct 17, 2015 at 5:37 PM, Petr Jelinek <petr@2ndquadrant.com> wrote: > > I agree with that sentiment. > > > > Attached patch adds variable to the shmem which is used for module > > activation tracking - set to true in ActiveCommitTs() and false in > > DeactivateCommitTs(). All the checks inside the commit_ts code were changed > > to use this new variable. I also removed the static variable Alvaro added in > > previous commit because it's not needed anymore. > > That sounds good to me. On a quick read-through it looks OK too. A revised version is attached. Two changes on top of Petr's patch: 1. In the two "get" routines, we were reading the flag without grabbing the lock. This is okay in a master server, because the flag cannot change in flight, but in a standby it is possible to have the module be deactivated while TS data is being queried. To fix this, simply move the check for the active shmem flag a few lines down to be inside the locked section. There are two other places that also read the flag without grabbing the lock. These look okay to me, so I added comments stating so. 2. In TransactionIdGetCommitTsData() we were grabbing lock, reading some data, releasing lock, then examining the "cached" value in shmem without a lock to see if it matched the function argument; if it's match, grab lock again and return the correct data. In the original coding this made sense because there was no locked section prior to reading the cache, but after the patch this was pointless. Make it simpler by moving the read of the cache inside the locked section too. -- Álvaro Herrera http://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
Вложения
Alvaro Herrera wrote: > Robert Haas wrote: > > On Sat, Oct 17, 2015 at 5:37 PM, Petr Jelinek <petr@2ndquadrant.com> wrote: > > > I agree with that sentiment. > > > > > > Attached patch adds variable to the shmem which is used for module > > > activation tracking - set to true in ActiveCommitTs() and false in > > > DeactivateCommitTs(). All the checks inside the commit_ts code were changed > > > to use this new variable. I also removed the static variable Alvaro added in > > > previous commit because it's not needed anymore. > > > > That sounds good to me. On a quick read-through it looks OK too. > > A revised version is attached. Pushed. -- Álvaro Herrera http://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
On Wed, Oct 28, 2015 at 3:07 AM, Alvaro Herrera
<alvherre@2ndquadrant.com> wrote:
> Alvaro Herrera wrote:
>> Robert Haas wrote:
>> > On Sat, Oct 17, 2015 at 5:37 PM, Petr Jelinek <petr@2ndquadrant.com> wrote:
>> > > I agree with that sentiment.
>> > >
>> > > Attached patch adds variable to the shmem which is used for module
>> > > activation tracking - set to true in ActiveCommitTs() and false in
>> > > DeactivateCommitTs(). All the checks inside the commit_ts code were changed
>> > > to use this new variable. I also removed the static variable Alvaro added in
>> > > previous commit because it's not needed anymore.
>> >
>> > That sounds good to me. On a quick read-through it looks OK too.
>>
>> A revised version is attached.
>
> Pushed.
I found another strange behavior on track_commit_timestamp.
Here are the steps to reproduce it.
1. Start the master and standby servers with track_commit_timestamp enabled.
Since committs is activated in standby, pg_last_committed_xact() can
successfully return the timestamp of last transaction as expected.
2. Disable track_commit_timestamp in the master and restart the master server.
The parameter-change WAL record is streamed to the standby and committs
is deactivated. pg_last_committed_xact() causes an ERROR in the standby.
3. Run checkpoint in the master.
4. Run restartpoint in the standby after the checkpoint WAL record generated
in #3 is replicated to the standby.
5. Restart the standby server.
Committs is activated in the standby because track_commit_timestamp is
enabled. Since there is no parameter-change WAL record since last
restartpoint, committs is not deactivated. So pg_last_committed_xact()
can successfully return the timestamp.
6. Enable track_commit_timestamp in the master and restart the master server.
7. Disable track_commit_timestamp in the master and restart the master server.
Back to the same situation as #2. That is, pg_last_committed_xact() causes
an ERROR.
8. Promote the standby server to new master.
Since committs is still inactive even after the promotion,
pg_last_committed_xact() keeps causing an ERROR though
track_commit_timestamp is on.
What I think strange is that pg_last_committed_xact() behaves differently
in #2, #5, and #7 though the settings of track_commit_timestamp are same
in both servers, i.e., it's disabled in the master but enabled in the standby.
I was thinking that whether committs is active or not should depend on
the setting of track_commit_timestamp *after* the promotion.
The behavior in #8 looked strange.
Regards,
--
Fujii Masao
On Thu, Oct 29, 2015 at 5:41 PM, Fujii Masao <masao.fujii@gmail.com> wrote: > I found another strange behavior on track_commit_timestamp. > Here are the steps to reproduce it. > > 1. Start the master and standby servers with track_commit_timestamp enabled. > Since committs is activated in standby, pg_last_committed_xact() can > successfully return the timestamp of last transaction as expected. > > 2. Disable track_commit_timestamp in the master and restart the master server. > The parameter-change WAL record is streamed to the standby and committs > is deactivated. pg_last_committed_xact() causes an ERROR in the standby. > > 3. Run checkpoint in the master. > > 4. Run restartpoint in the standby after the checkpoint WAL record generated > in #3 is replicated to the standby. > > 5. Restart the standby server. > Committs is activated in the standby because track_commit_timestamp is > enabled. This seems wrong already at this point. -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company
I paraphrase Fujii Masao, who wrote: > 1. Start the master and standby servers with track_commit_timestamp enabled. > 2. Disable track_commit_timestamp in the master and restart the master server. > 3. Run checkpoint in the master. > 4. Run restartpoint in the standby after the checkpoint WAL record generated > 5. Restart the standby server. > 6. Enable track_commit_timestamp in the master and restart the master server. > 7. Disable track_commit_timestamp in the master and restart the master server. > What I think strange is that pg_last_committed_xact() behaves differently > in #2, #5, and #7 though the settings of track_commit_timestamp are same > in both servers, i.e., it's disabled in the master but enabled in the standby. Interesting, thanks. You're right that this behaves oddly. I think in order to fix these two points (#5 and #7), we need to make the standby not honour the GUC at all, i.e. only heed what the master setting is. > 8. Promote the standby server to new master. > Since committs is still inactive even after the promotion, > pg_last_committed_xact() keeps causing an ERROR though > track_commit_timestamp is on. > > I was thinking that whether committs is active or not should depend on > the setting of track_commit_timestamp *after* the promotion. > The behavior in #8 looked strange. To fix this problem, we can re-run StartupCommitTs() after recovery is done, this time making sure to honour the GUC setting. I haven't tried the scenarios we fixed with the previous round of patching, but this patch seems to close the problems just reported. (My next step will be looking over the recovery test framework by Michael et al, so that I can apply a few tests for this stuff.) In the meantime, if you can look this over I would appreciate it. -- Álvaro Herrera http://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
Вложения
On 2015-11-16 22:43, Alvaro Herrera wrote: > I paraphrase Fujii Masao, who wrote: > >> 1. Start the master and standby servers with track_commit_timestamp enabled. >> 2. Disable track_commit_timestamp in the master and restart the master server. >> 3. Run checkpoint in the master. >> 4. Run restartpoint in the standby after the checkpoint WAL record generated >> 5. Restart the standby server. >> 6. Enable track_commit_timestamp in the master and restart the master server. >> 7. Disable track_commit_timestamp in the master and restart the master server. > >> What I think strange is that pg_last_committed_xact() behaves differently >> in #2, #5, and #7 though the settings of track_commit_timestamp are same >> in both servers, i.e., it's disabled in the master but enabled in the standby. > > Interesting, thanks. You're right that this behaves oddly. > > I think in order to fix these two points (#5 and #7), we need to make > the standby not honour the GUC at all, i.e. only heed what the master > setting is. > >> 8. Promote the standby server to new master. >> Since committs is still inactive even after the promotion, >> pg_last_committed_xact() keeps causing an ERROR though >> track_commit_timestamp is on. >> >> I was thinking that whether committs is active or not should depend on >> the setting of track_commit_timestamp *after* the promotion. >> The behavior in #8 looked strange. > > To fix this problem, we can re-run StartupCommitTs() after recovery is > done, this time making sure to honour the GUC setting. > > I haven't tried the scenarios we fixed with the previous round of > patching, but this patch seems to close the problems just reported. > (My next step will be looking over the recovery test framework by > Michael et al, so that I can apply a few tests for this stuff.) > In the meantime, if you can look this over I would appreciate it. > While this seems good, I'd code it slightly differently. I didn't like the addition of new bool when it's not really needed. This brings the question if we actually need the BootStrapCommitTs and StartupCommitTs functions which really don't do much though. -- Petr Jelinek http://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Training & Services
Вложения
Petr Jelinek wrote: > While this seems good, I'd code it slightly differently. I didn't like the > addition of new bool when it's not really needed. This brings the question > if we actually need the BootStrapCommitTs and StartupCommitTs functions > which really don't do much though. Thanks, it's certainly nice that this got simpler. (I'm not in love with the idea of having xlog.c know what flag needs to pass in each case, but I don't see any option that's more convenient.) We weren't quite there however -- namely this patch didn't close problem #8 in Fujii-san rundown. The problem is that when promoting, standbyState is not STANDBY_DISABLED but STANDBY_SNAPSHOT_READY (which is a bit surprising but not something this patch should fix). To fix this I took the StartupCommitTs() call out of that block, so that it runs inconditionally. I also changed the hint message: postgres=# select * from pg_last_committed_xact(); ERROR: could not get commit timestamp data HINT: Make sure the configuration parameter "track_commit_timestamp" is set in the master server. Otherwise this would be very confusing: postgres=# select * from pg_last_committed_xact(); ERROR: could not get commit timestamp data HINT: Make sure the configuration parameter "track_commit_timestamp" is set. postgres=# show track_commit_timestamp ; track_commit_timestamp ------------------------ on (1 fila) -- Álvaro Herrera http://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
On Tue, Nov 17, 2015 at 6:43 AM, Alvaro Herrera <alvherre@2ndquadrant.com> wrote: > I paraphrase Fujii Masao, who wrote: > >> 1. Start the master and standby servers with track_commit_timestamp enabled. >> 2. Disable track_commit_timestamp in the master and restart the master server. >> 3. Run checkpoint in the master. >> 4. Run restartpoint in the standby after the checkpoint WAL record generated >> 5. Restart the standby server. >> 6. Enable track_commit_timestamp in the master and restart the master server. >> 7. Disable track_commit_timestamp in the master and restart the master server. > >> What I think strange is that pg_last_committed_xact() behaves differently >> in #2, #5, and #7 though the settings of track_commit_timestamp are same >> in both servers, i.e., it's disabled in the master but enabled in the standby. > > Interesting, thanks. You're right that this behaves oddly. > > I think in order to fix these two points (#5 and #7), we need to make > the standby not honour the GUC at all, i.e. only heed what the master > setting is. > >> 8. Promote the standby server to new master. >> Since committs is still inactive even after the promotion, >> pg_last_committed_xact() keeps causing an ERROR though >> track_commit_timestamp is on. >> >> I was thinking that whether committs is active or not should depend on >> the setting of track_commit_timestamp *after* the promotion. >> The behavior in #8 looked strange. > > To fix this problem, we can re-run StartupCommitTs() after recovery is > done, this time making sure to honour the GUC setting. > > I haven't tried the scenarios we fixed with the previous round of > patching, but this patch seems to close the problems just reported. > (My next step will be looking over the recovery test framework by > Michael et al, so that I can apply a few tests for this stuff.) > In the meantime, if you can look this over I would appreciate it. Sorry for not reviewing the patch before you push it... In HEAD, I ran very simple test case: 1. enable track_commit_timestamp 2. start the server 3. run some transactions 4. execute pg_last_committed_xact() -- returns non-null values 5. shutdown the server with immdiate mode 6. restart the server -- crash recovery happens 7. execute pg_last_committed_xact() The last call of pg_last_committed_xact() returns NULL values, which means that the xid and timestamp information of the last committed transaction disappeared by crash recovery. Isn't this a bug? Regards, -- Fujii Masao
Fujii Masao wrote:
> Sorry for not reviewing the patch before you push it...
>
> In HEAD, I ran very simple test case:
>
> 1. enable track_commit_timestamp
> 2. start the server
> 3. run some transactions
> 4. execute pg_last_committed_xact() -- returns non-null values
> 5. shutdown the server with immdiate mode
> 6. restart the server -- crash recovery happens
> 7. execute pg_last_committed_xact()
>
> The last call of pg_last_committed_xact() returns NULL values, which means
> that the xid and timestamp information of the last committed transaction
> disappeared by crash recovery. Isn't this a bug?
Hm, not really, because the status of the "last" transaction is kept in
shared memory as a cache and not expected to live across a restart.
However, I tested the equivalent scenario:
alvherre=# create table fg();
CREATE TABLE
alvherre=# select ts.* from pg_class,pg_xact_commit_timestamp(xmin) ts where relname = 'fg';
ts
-------------------------------
2015-12-04 12:41:48.017976-03
(1 fila)
then crash the server, and after recovery the data is gone:
alvherre=# select ts.*, xmin, c.relname from pg_class c,pg_xact_commit_timestamp(xmin) ts where relname = 'fg';
ts | xmin | relname
----+------+---------
| 630 | fg
(1 fila)
Not sure what is going on; my reading of the code certainly says that
the data should be there. I'm looking into it.
I also noticed that I didn't actually push the whole of the patch
yesterday -- I neglected to "git add" the latest changes, the ones that
fix the promotion scenario :-( so the commit messages is misleading
because it describes something that's not there.
--
Álvaro Herrera http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
On Sat, Dec 5, 2015 at 12:56 AM, Alvaro Herrera <alvherre@2ndquadrant.com> wrote: > Fujii Masao wrote: > >> Sorry for not reviewing the patch before you push it... >> >> In HEAD, I ran very simple test case: >> >> 1. enable track_commit_timestamp >> 2. start the server >> 3. run some transactions >> 4. execute pg_last_committed_xact() -- returns non-null values >> 5. shutdown the server with immdiate mode >> 6. restart the server -- crash recovery happens >> 7. execute pg_last_committed_xact() >> >> The last call of pg_last_committed_xact() returns NULL values, which means >> that the xid and timestamp information of the last committed transaction >> disappeared by crash recovery. Isn't this a bug? > > Hm, not really, because the status of the "last" transaction is kept in > shared memory as a cache and not expected to live across a restart. > However, I tested the equivalent scenario: > > alvherre=# create table fg(); > CREATE TABLE > > alvherre=# select ts.* from pg_class,pg_xact_commit_timestamp(xmin) ts where relname = 'fg'; > ts > ------------------------------- > 2015-12-04 12:41:48.017976-03 > (1 fila) > > then crash the server, and after recovery the data is gone: > > alvherre=# select ts.*, xmin, c.relname from pg_class c,pg_xact_commit_timestamp(xmin) ts where relname = 'fg'; > ts | xmin | relname > ----+------+--------- > | 630 | fg > (1 fila) > > Not sure what is going on; my reading of the code certainly says that > the data should be there. I'm looking into it. > > I also noticed that I didn't actually push the whole of the patch > yesterday -- I neglected to "git add" the latest changes, the ones that > fix the promotion scenario :-( so the commit messages is misleading > because it describes something that's not there. So firstly you will push those "latest" changes soon? Regards, -- Fujii Masao
On Mon, Dec 7, 2015 at 8:33 AM, Fujii Masao <masao.fujii@gmail.com> wrote: > On Sat, Dec 5, 2015 at 12:56 AM, Alvaro Herrera > <alvherre@2ndquadrant.com> wrote: >> Fujii Masao wrote: >> >>> Sorry for not reviewing the patch before you push it... >>> >>> In HEAD, I ran very simple test case: >>> >>> 1. enable track_commit_timestamp >>> 2. start the server >>> 3. run some transactions >>> 4. execute pg_last_committed_xact() -- returns non-null values >>> 5. shutdown the server with immdiate mode >>> 6. restart the server -- crash recovery happens >>> 7. execute pg_last_committed_xact() >>> >>> The last call of pg_last_committed_xact() returns NULL values, which means >>> that the xid and timestamp information of the last committed transaction >>> disappeared by crash recovery. Isn't this a bug? >> >> Hm, not really, because the status of the "last" transaction is kept in >> shared memory as a cache and not expected to live across a restart. >> However, I tested the equivalent scenario: >> >> alvherre=# create table fg(); >> CREATE TABLE >> >> alvherre=# select ts.* from pg_class,pg_xact_commit_timestamp(xmin) ts where relname = 'fg'; >> ts >> ------------------------------- >> 2015-12-04 12:41:48.017976-03 >> (1 fila) >> >> then crash the server, and after recovery the data is gone: >> >> alvherre=# select ts.*, xmin, c.relname from pg_class c,pg_xact_commit_timestamp(xmin) ts where relname = 'fg'; >> ts | xmin | relname >> ----+------+--------- >> | 630 | fg >> (1 fila) >> >> Not sure what is going on; my reading of the code certainly says that >> the data should be there. I'm looking into it. >> >> I also noticed that I didn't actually push the whole of the patch >> yesterday -- I neglected to "git add" the latest changes, the ones that >> fix the promotion scenario :-( so the commit messages is misleading >> because it describes something that's not there. > > So firstly you will push those "latest" changes soon? It seems like these changes haven't been pushed yet, and unfortunately that's probably a beta blocker. -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company
Robert Haas wrote: > On Mon, Dec 7, 2015 at 8:33 AM, Fujii Masao <masao.fujii@gmail.com> wrote: > > So firstly you will push those "latest" changes soon? > > It seems like these changes haven't been pushed yet, and unfortunately > that's probably a beta blocker. I'm on this. -- Álvaro Herrera http://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
On Wed, Dec 9, 2015 at 4:21 PM, Alvaro Herrera <alvherre@2ndquadrant.com> wrote: > Robert Haas wrote: >> On Mon, Dec 7, 2015 at 8:33 AM, Fujii Masao <masao.fujii@gmail.com> wrote: > >> > So firstly you will push those "latest" changes soon? >> >> It seems like these changes haven't been pushed yet, and unfortunately >> that's probably a beta blocker. > > I'm on this. Uh, when are you going to do this? At this point we've probably lost another week getting rc1 out the door. -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company
Alvaro Herrera wrote: > Not sure what is going on; my reading of the code certainly says that > the data should be there. I'm looking into it. > > I also noticed that I didn't actually push the whole of the patch > yesterday -- I neglected to "git add" the latest changes, the ones that > fix the promotion scenario :-( so the commit messages is misleading > because it describes something that's not there. Pushed a fix. I also wrote some tests using the RecoveryNode stuff submitted by Michael Paquier. These aren't yet pushed, because we don't have the framework; once we have that I can push them too. As far as I can tell, these tests exercise all the cases that have been pointed out so far; I can see some of them fail if I run on previous commits. Thanks for the continued testing. -- Álvaro Herrera http://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services