LSN as a recovery target
Hi all,
Today somebody has pointed me out that it could be interesting to be
able to recovery up to a given LSN position. One argument behind that
was to allow a maximum of things to recover up to the point where a
relation block got corrupted by a specific record because of a broken
segment. So that would be simply having recovery_target_lsn in
recovery.conf similar to what we have now.
Thoughts?
--
Michael
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
On 24 May 2016 at 09:12, Michael Paquier <michael.paquier@gmail.com> wrote:
Hi all,
Today somebody has pointed me out that it could be interesting to be
able to recovery up to a given LSN position. One argument behind that
was to allow a maximum of things to recover up to the point where a
relation block got corrupted by a specific record because of a broken
segment. So that would be simply having recovery_target_lsn in
recovery.conf similar to what we have now.
Thoughts?
Sounds useful, if somewhat niche. I've needed that in the past, and just
zeroed the WAL segment after the end of the target record. Not super
user-friendly.
--
Craig Ringer http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Training & Services
On Mon, May 23, 2016 at 6:25 PM, Craig Ringer <craig@2ndquadrant.com> wrote:
On 24 May 2016 at 09:12, Michael Paquier <michael.paquier@gmail.com> wrote:
Hi all,
Today somebody has pointed me out that it could be interesting to be
able to recovery up to a given LSN position. One argument behind that
was to allow a maximum of things to recover up to the point where a
relation block got corrupted by a specific record because of a broken
segment. So that would be simply having recovery_target_lsn in
recovery.conf similar to what we have now.
Thoughts?Sounds useful, if somewhat niche. I've needed that in the past, and just
zeroed the WAL segment after the end of the target record. Not super
user-friendly.
Yeah, that's really something that covers only a narrow case, though
if we don't have it when we need it we're limited to some hacks.
Perhaps people who have the advanced level to use such a thing have
the level to use hacks anyway..
--
Michael
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
Re: Michael Paquier 2016-05-24 <CAB7nPqQRXsC8=ozh6GpjLnpZ=MeooUZOaAbzx28n2bjSMv2B4g@mail.gmail.com>
Yeah, that's really something that covers only a narrow case, though
if we don't have it when we need it we're limited to some hacks.
Perhaps people who have the advanced level to use such a thing have
the level to use hacks anyway..
I'd think recovery_target_lsn would be more useful in practice than
the existing recovery_target_xid. So I don't see why it shouldn't just
be added, also given it's likely very unobtrusive to do so.
Christoph
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
Christoph Berg wrote:
Re: Michael Paquier 2016-05-24 <CAB7nPqQRXsC8=ozh6GpjLnpZ=MeooUZOaAbzx28n2bjSMv2B4g@mail.gmail.com>
Yeah, that's really something that covers only a narrow case, though
if we don't have it when we need it we're limited to some hacks.
Perhaps people who have the advanced level to use such a thing have
the level to use hacks anyway..I'd think recovery_target_lsn would be more useful in practice than
the existing recovery_target_xid. So I don't see why it shouldn't just
be added, also given it's likely very unobtrusive to do so.
Also, see
/messages/by-id/56BD0E4E.5050503@2ndquadrant.com
--
�lvaro Herrera http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
On Tue, May 24, 2016 at 9:29 AM, Alvaro Herrera
<alvherre@2ndquadrant.com> wrote:
Christoph Berg wrote:
Re: Michael Paquier 2016-05-24 <CAB7nPqQRXsC8=ozh6GpjLnpZ=MeooUZOaAbzx28n2bjSMv2B4g@mail.gmail.com>
Yeah, that's really something that covers only a narrow case, though
if we don't have it when we need it we're limited to some hacks.
Perhaps people who have the advanced level to use such a thing have
the level to use hacks anyway..I'd think recovery_target_lsn would be more useful in practice than
the existing recovery_target_xid. So I don't see why it shouldn't just
be added, also given it's likely very unobtrusive to do so.
Looking at xlog.c it is not that complicated, and we could add tests
in 003_recovery_targets.pl at the same time. Perhaps somebody looking
for a first participation would be interested in this small project?
--
Michael
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
On Wed, May 25, 2016 at 1:32 AM, Michael Paquier
<michael.paquier@gmail.com> wrote:
On Tue, May 24, 2016 at 9:29 AM, Alvaro Herrera
<alvherre@2ndquadrant.com> wrote:Christoph Berg wrote:
Re: Michael Paquier 2016-05-24 <CAB7nPqQRXsC8=ozh6GpjLnpZ=MeooUZOaAbzx28n2bjSMv2B4g@mail.gmail.com>
Yeah, that's really something that covers only a narrow case, though
if we don't have it when we need it we're limited to some hacks.
Perhaps people who have the advanced level to use such a thing have
the level to use hacks anyway..I'd think recovery_target_lsn would be more useful in practice than
the existing recovery_target_xid. So I don't see why it shouldn't just
be added, also given it's likely very unobtrusive to do so.Looking at xlog.c it is not that complicated, and we could add tests
in 003_recovery_targets.pl at the same time. Perhaps somebody looking
for a first participation would be interested in this small project?
Oh, well. I have implemented it as attached by introducing
recovery_target_lsn as a new recovery parameter. This takes into
account recovery_target_inclusive and stops at the precise point of a
record without being influenced by the xact records, in a way similar
recovery_target_name. Tests and documentation are added, and this is
part of the next CF.
--
Michael
Attachments:
recovery-target-lsn.patchbinary/octet-stream; name=recovery-target-lsn.patchDownload+112-5
On 06/09/2016 02:33 PM, Michael Paquier wrote:
On Wed, May 25, 2016 at 1:32 AM, Michael Paquier
<michael.paquier@gmail.com> wrote:On Tue, May 24, 2016 at 9:29 AM, Alvaro Herrera
<alvherre@2ndquadrant.com> wrote:Christoph Berg wrote:
Re: Michael Paquier 2016-05-24 <CAB7nPqQRXsC8=ozh6GpjLnpZ=MeooUZOaAbzx28n2bjSMv2B4g@mail.gmail.com>
Yeah, that's really something that covers only a narrow case, though
if we don't have it when we need it we're limited to some hacks.
Perhaps people who have the advanced level to use such a thing have
the level to use hacks anyway..I'd think recovery_target_lsn would be more useful in practice than
the existing recovery_target_xid. So I don't see why it shouldn't just
be added, also given it's likely very unobtrusive to do so.Looking at xlog.c it is not that complicated, and we could add tests
in 003_recovery_targets.pl at the same time. Perhaps somebody looking
for a first participation would be interested in this small project?Oh, well. I have implemented it as attached by introducing
recovery_target_lsn as a new recovery parameter. This takes into
account recovery_target_inclusive and stops at the precise point of a
record without being influenced by the xact records, in a way similar
recovery_target_name. Tests and documentation are added, and this is
part of the next CF.
Hi,
I reviewed this patch rebased to deal with
f6ced51f9188ad5806219471a0b40a91dde923aa, and minor adjustment (see below)
It do the job. However if you use an incorrect recovery_target_lsn you
get this message :
"FATAL: invalid input syntax for type pg_lsn: "0RT5/4""
I added a PG_TRY/PG_CATCH section in order to get a more explicit message :
FATAL: wrong recovery_target_lsn (must be pg_lsn type)
I am not sure if it is the best solution?
Thanks to Julien Rouhaud for his help.
--
Adrien NAYRAT
Attachments:
recovery-target-lsn-2.patchtext/x-patch; name=recovery-target-lsn-2.patchDownload+123-5
On Fri, Aug 19, 2016 at 10:47 PM, Adrien Nayrat
<adrien.nayrat@dalibo.com> wrote:
I reviewed this patch rebased to deal with
f6ced51f9188ad5806219471a0b40a91dde923aa, and minor adjustment (see below)
Thanks!
It do the job. However if you use an incorrect recovery_target_lsn you
get this message :
"FATAL: invalid input syntax for type pg_lsn: "0RT5/4""I added a PG_TRY/PG_CATCH section in order to get a more explicit message :
FATAL: wrong recovery_target_lsn (must be pg_lsn type)I am not sure if it is the best solution?
Using a PG_TRY/CATCH block the way you do to show to user a different
error message while the original one is actually correct does not
sound like a good idea to me. It complicates the code and the original
pattern matches what is already done for timestamps, where in case of
error you'd get that:
FATAL: invalid input syntax for type timestamp with time zone: "aa"
I think that a better solution would be to make clearer in the docs
that pg_lsn is used here. First, in recovery.conf.sample, let's use
pg_lsn and not LSN. Then, in the sgml docs, let's add a reference to
the page of pg_lsn as well:
https://www.postgresql.org/docs/devel/static/datatype-pg-lsn.html
Now we have that:
+ <para>
+ This parameter specifies the LSN of the write-ahead log stream up to
+ which recovery will proceed. The precise stopping point is also
+ influenced by <xref linkend="recovery-target-inclusive">.
+ </para>
Perhaps we could just add an additional sentence: "This parameter
value is parsed using the system datatype <link=pg_lsn>". Or something
like that. Thoughts?
I'll think about that a bit more and send a new patch. I have switched
the patch to "waiting on author" for now, just to not forget about
that.
--
Michael
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
On 20/08/16 02:13, Michael Paquier wrote:
On Fri, Aug 19, 2016 at 10:47 PM, Adrien Nayrat
<adrien.nayrat@dalibo.com> wrote:I reviewed this patch rebased to deal with
f6ced51f9188ad5806219471a0b40a91dde923aa, and minor adjustment (see below)Thanks!
It do the job. However if you use an incorrect recovery_target_lsn you
get this message :
"FATAL: invalid input syntax for type pg_lsn: "0RT5/4""I added a PG_TRY/PG_CATCH section in order to get a more explicit message :
FATAL: wrong recovery_target_lsn (must be pg_lsn type)I am not sure if it is the best solution?
Using a PG_TRY/CATCH block the way you do to show to user a different
error message while the original one is actually correct does not
sound like a good idea to me. It complicates the code and the original
pattern matches what is already done for timestamps, where in case of
error you'd get that:
FATAL: invalid input syntax for type timestamp with time zone: "aa"I think that a better solution would be to make clearer in the docs
that pg_lsn is used here. First, in recovery.conf.sample, let's use
pg_lsn and not LSN. Then, in the sgml docs, let's add a reference to
the page of pg_lsn as well:
https://www.postgresql.org/docs/devel/static/datatype-pg-lsn.htmlNow we have that: + <para> + This parameter specifies the LSN of the write-ahead log stream up to + which recovery will proceed. The precise stopping point is also + influenced by <xref linkend="recovery-target-inclusive">. + </para> Perhaps we could just add an additional sentence: "This parameter value is parsed using the system datatype <link=pg_lsn>". Or something like that. Thoughts?
+1
If we want to specifically name the recovery_target_lsn in the message,
we could probably do it using context.
--
Petr Jelinek http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Training & Services
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
On Sat, Aug 20, 2016 at 10:44 AM, Petr Jelinek <petr@2ndquadrant.com> wrote:
If we want to specifically name the recovery_target_lsn in the message, we
could probably do it using context.
So that would be basically assigning error_context_stack for each item
parsed for recovery.conf? That seems a bit narrow as usually
recovery.conf is not that long..
--
Michael
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
On 20/08/2016 15:41, Michael Paquier wrote:
On Sat, Aug 20, 2016 at 10:44 AM, Petr Jelinek <petr@2ndquadrant.com> wrote:
If we want to specifically name the recovery_target_lsn in the message, we
could probably do it using context.So that would be basically assigning error_context_stack for each item
parsed for recovery.conf? That seems a bit narrow as usually
recovery.conf is not that long..
That'd still be an improvement, since the error message doesn't even
mention that the error comes from recovery.conf. I agree it can't come
from many places, but it may be useful for some people.
--
Julien Rouhaud
http://dalibo.com - http://dalibo.org
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
On Sat, Aug 20, 2016 at 10:44 AM, Petr Jelinek <petr@2ndquadrant.com> wrote:
On 20/08/16 02:13, Michael Paquier wrote:
On Fri, Aug 19, 2016 at 10:47 PM, Adrien Nayrat
<adrien.nayrat@dalibo.com> wrote:
Using a PG_TRY/CATCH block the way you do to show to user a different
error message while the original one is actually correct does not
sound like a good idea to me. It complicates the code and the original
pattern matches what is already done for timestamps, where in case of
error you'd get that:
FATAL: invalid input syntax for type timestamp with time zone: "aa"I think that a better solution would be to make clearer in the docs
that pg_lsn is used here. First, in recovery.conf.sample, let's use
pg_lsn and not LSN. Then, in the sgml docs, let's add a reference to
the page of pg_lsn as well:
https://www.postgresql.org/docs/devel/static/datatype-pg-lsn.htmlNow we have that: + <para> + This parameter specifies the LSN of the write-ahead log stream up to + which recovery will proceed. The precise stopping point is also + influenced by <xref linkend="recovery-target-inclusive">. + </para> Perhaps we could just add an additional sentence: "This parameter value is parsed using the system datatype <link=pg_lsn>". Or something like that. Thoughts?+1
So here is the patch resulting in that. After thinking more about that
I have arrived at the conclusion to not use LSN in recovery.conf, but
"WAL position (LSN)", and also mention in the docs that pg_lsn is used
to parse the parameter value. A couple of log messages are changed
similarly. It definitely makes it more understandable for users to
speak of WAL positions.
I also noticed that the top block of "Recovery Target Settings" needs
to mention recovery_target_lsn, aka when name, xid, lsn and time are
combined in the same recovery.conf, only the last value will be used.
--
Michael
Attachments:
recovery-target-lsn-3.patchapplication/x-download; name=recovery-target-lsn-3.patchDownload+119-9
On 08/20/2016 04:16 PM, Michael Paquier wrote:
On Sat, Aug 20, 2016 at 10:44 AM, Petr Jelinek <petr@2ndquadrant.com> wrote:
On 20/08/16 02:13, Michael Paquier wrote:
On Fri, Aug 19, 2016 at 10:47 PM, Adrien Nayrat
<adrien.nayrat@dalibo.com> wrote:
Using a PG_TRY/CATCH block the way you do to show to user a different
error message while the original one is actually correct does not
sound like a good idea to me. It complicates the code and the original
pattern matches what is already done for timestamps, where in case of
error you'd get that:
FATAL: invalid input syntax for type timestamp with time zone: "aa"I think that a better solution would be to make clearer in the docs
that pg_lsn is used here. First, in recovery.conf.sample, let's use
pg_lsn and not LSN. Then, in the sgml docs, let's add a reference to
the page of pg_lsn as well:
https://www.postgresql.org/docs/devel/static/datatype-pg-lsn.htmlNow we have that: + <para> + This parameter specifies the LSN of the write-ahead log stream up to + which recovery will proceed. The precise stopping point is also + influenced by <xref linkend="recovery-target-inclusive">. + </para> Perhaps we could just add an additional sentence: "This parameter value is parsed using the system datatype <link=pg_lsn>". Or something like that. Thoughts?+1
So here is the patch resulting in that. After thinking more about that
I have arrived at the conclusion to not use LSN in recovery.conf, but
"WAL position (LSN)", and also mention in the docs that pg_lsn is used
to parse the parameter value. A couple of log messages are changed
similarly. It definitely makes it more understandable for users to
speak of WAL positions.
Good, I prefer this.
I also noticed that the top block of "Recovery Target Settings" needs
to mention recovery_target_lsn, aka when name, xid, lsn and time are
combined in the same recovery.conf, only the last value will be used.
Good catch!
As Julien said, there is nothing to notice that error comes from
recovery.conf.
My fear would be that an user encounters an error like this. Il will be
difficult to link to the recovery.conf.
For the others settings (xid, timeline,name) there is an explicit
message that notice error is in recovery.conf.
I see it is not the case for recovery_target_time.
Should we modify only the documentation or shoud we try to find a
solution to point the origin of error?
--
Adrien NAYRAT
http://dalibo.com - http://dalibo.org
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
On Mon, Aug 22, 2016 at 7:12 PM, Adrien Nayrat <adrien.nayrat@dalibo.com> wrote:
As Julien said, there is nothing to notice that error comes from
recovery.conf.
My fear would be that an user encounters an error like this. Il will be
difficult to link to the recovery.conf.
Thinking a bit wider than that, we may want to know such context for
normal GUC parameters as well, and that's not the case now. Perhaps
there is actually a reason why that's not done for GUCs, but it seems
that it would be useful there as well. That would give another reason
to move all that under the GUC umbrella.
For the others settings (xid, timeline,name) there is an explicit
message that notice error is in recovery.conf.I see it is not the case for recovery_target_time.
Yes, in this case the parameter value is parsed using an datatype _in
function call. And the error message depends on that..
Should we modify only the documentation or should we try to find a
solution to point the origin of error?
The patch as proposed is complicated enough I think, and it would be
good to keep things simple if we can. So having something in the docs
looks fine to me, and that's actually the reference to pg_lsn used to
parse the parameter value.
--
Michael
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
On 8/22/16 8:28 AM, Michael Paquier wrote:
Thinking a bit wider than that, we may want to know such context for
normal GUC parameters as well, and that's not the case now. Perhaps
there is actually a reason why that's not done for GUCs, but it seems
that it would be useful there as well.
GUC parsing generally needs, or used to need, to work under more
constrained circumstances, e.g., no full memory management. That's not
a reason not to try this, but there might be non-obvious problems.
--
Peter Eisentraut http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
On Mon, Aug 22, 2016 at 8:28 AM, Michael Paquier
<michael.paquier@gmail.com> wrote:
On Mon, Aug 22, 2016 at 7:12 PM, Adrien Nayrat <adrien.nayrat@dalibo.com> wrote:
As Julien said, there is nothing to notice that error comes from
recovery.conf.
My fear would be that an user encounters an error like this. Il will be
difficult to link to the recovery.conf.Thinking a bit wider than that, we may want to know such context for
normal GUC parameters as well, and that's not the case now. Perhaps
there is actually a reason why that's not done for GUCs, but it seems
that it would be useful there as well. That would give another reason
to move all that under the GUC umbrella.
Maybe so, but that's been tried multiple times without success. If
you think an error context is useful here, and I bet it is, I'd say
just add it and be done with it.
--
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
On Tue, Aug 23, 2016 at 12:49 AM, Robert Haas <robertmhaas@gmail.com> wrote:
On Mon, Aug 22, 2016 at 8:28 AM, Michael Paquier
<michael.paquier@gmail.com> wrote:On Mon, Aug 22, 2016 at 7:12 PM, Adrien Nayrat <adrien.nayrat@dalibo.com> wrote:
As Julien said, there is nothing to notice that error comes from
recovery.conf.
My fear would be that an user encounters an error like this. Il will be
difficult to link to the recovery.conf.Thinking a bit wider than that, we may want to know such context for
normal GUC parameters as well, and that's not the case now. Perhaps
there is actually a reason why that's not done for GUCs, but it seems
that it would be useful there as well. That would give another reason
to move all that under the GUC umbrella.Maybe so, but that's been tried multiple times without success. If
you think an error context is useful here, and I bet it is, I'd say
just add it and be done with it.
This has finished by being less ugly than I thought, so I implemented
it as attached. Patch 0001 introduces recovery_target_lsn, and patch
0002 sets up an error context callback generating things like that on
failure:
FATAL: invalid input syntax for type pg_lsn: "popo"
CONTEXT: line 11 of configuration file "recovery.conf", parameter
"recovery_target_lsn"
--
Michael
Attachments:
0001-Introduce-recovery_target_lsn-in-recovery.conf.patchtext/plain; charset=US-ASCII; name=0001-Introduce-recovery_target_lsn-in-recovery.conf.patchDownload+119-10
0002-Introduce-error-callback-when-parsing-recovery.conf.patchtext/plain; charset=US-ASCII; name=0002-Introduce-error-callback-when-parsing-recovery.conf.patchDownload+25-1
On 23/08/16 09:33, Michael Paquier wrote:
On Tue, Aug 23, 2016 at 12:49 AM, Robert Haas <robertmhaas@gmail.com> wrote:
On Mon, Aug 22, 2016 at 8:28 AM, Michael Paquier
<michael.paquier@gmail.com> wrote:On Mon, Aug 22, 2016 at 7:12 PM, Adrien Nayrat <adrien.nayrat@dalibo.com> wrote:
As Julien said, there is nothing to notice that error comes from
recovery.conf.
My fear would be that an user encounters an error like this. Il will be
difficult to link to the recovery.conf.Thinking a bit wider than that, we may want to know such context for
normal GUC parameters as well, and that's not the case now. Perhaps
there is actually a reason why that's not done for GUCs, but it seems
that it would be useful there as well. That would give another reason
to move all that under the GUC umbrella.Maybe so, but that's been tried multiple times without success. If
you think an error context is useful here, and I bet it is, I'd say
just add it and be done with it.This has finished by being less ugly than I thought, so I implemented
it as attached. Patch 0001 introduces recovery_target_lsn, and patch
0002 sets up an error context callback generating things like that on
failure:
FATAL: invalid input syntax for type pg_lsn: "popo"
CONTEXT: line 11 of configuration file "recovery.conf", parameter
"recovery_target_lsn"
Looks very reasonable to me (both patches). Thanks for doing that.
I am inclined to mark this as ready for committer.
--
Petr Jelinek http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Training & Services
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
On 08/23/2016 10:39 AM, Petr Jelinek wrote:
On 23/08/16 09:33, Michael Paquier wrote:
On Tue, Aug 23, 2016 at 12:49 AM, Robert Haas <robertmhaas@gmail.com>
wrote:On Mon, Aug 22, 2016 at 8:28 AM, Michael Paquier
<michael.paquier@gmail.com> wrote:On Mon, Aug 22, 2016 at 7:12 PM, Adrien Nayrat
<adrien.nayrat@dalibo.com> wrote:As Julien said, there is nothing to notice that error comes from
recovery.conf.
My fear would be that an user encounters an error like this. Il
will be
difficult to link to the recovery.conf.Thinking a bit wider than that, we may want to know such context for
normal GUC parameters as well, and that's not the case now. Perhaps
there is actually a reason why that's not done for GUCs, but it seems
that it would be useful there as well. That would give another reason
to move all that under the GUC umbrella.Maybe so, but that's been tried multiple times without success. If
you think an error context is useful here, and I bet it is, I'd say
just add it and be done with it.This has finished by being less ugly than I thought, so I implemented
it as attached. Patch 0001 introduces recovery_target_lsn, and patch
0002 sets up an error context callback generating things like that on
failure:
FATAL: invalid input syntax for type pg_lsn: "popo"
CONTEXT: line 11 of configuration file "recovery.conf", parameter
"recovery_target_lsn"
Good! Message is clear now for recovery_target_lsn and recovery_target_time.
Thanks for your work.
Looks very reasonable to me (both patches). Thanks for doing that.
I am inclined to mark this as ready for committer.
+1 everything if fine for me.
--
Adrien NAYRAT
http://dalibo.com - http://dalibo.org
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers