Error code for checksum failure in origin.c
Hi.
In the code in src/backend/replication/logical/origin.c,
the error code "ERRCODE_CONFIGURATION_LIMIT_EXCEEDED" is given
when a checksum check results in an error,
but "ERRCODE_ DATA_CORRUPTED" seems to be more appropriate.
====================
diff --git a/src/backend/replication/logical/origin.c
b/src/backend/replication/logical/origin.c
index 2c191de..65dcd03 100644
--- a/src/backend/replication/logical/origin.c
+++ b/src/backend/replication/logical/origin.c
@@ -796,7 +796,7 @@ StartupReplicationOrigin(void)
FIN_CRC32C(crc);
if (file_crc != crc)
ereport(PANIC,
- (errcode(ERRCODE_CONFIGURATION_LIMIT_EXCEEDED),
+ (errcode(ERRCODE_DATA_CORRUPTED),
errmsg("replication slot checkpoint
has wrong checksum %u, expected %u",
crc, file_crc)));
====================
Thought?Best regards,
--
Tatsuhito Kasahara
kasahara.tatsuhito _at_ gmail.com
On Thu, Aug 26, 2021 at 3:18 PM Kasahara Tatsuhito
<kasahara.tatsuhito@gmail.com> wrote:
Hi.
In the code in src/backend/replication/logical/origin.c,
the error code "ERRCODE_CONFIGURATION_LIMIT_EXCEEDED" is given
when a checksum check results in an error,
but "ERRCODE_ DATA_CORRUPTED" seems to be more appropriate.
+1. Your observation looks correct to me and the error code suggested
by you seems appropriate. We use ERRCODE_DATA_CORRUPTED in
ReadTwoPhaseFile() for similar error.
--
With Regards,
Amit Kapila.
On 26 Aug 2021, at 12:03, Amit Kapila <amit.kapila16@gmail.com> wrote:
On Thu, Aug 26, 2021 at 3:18 PM Kasahara Tatsuhito
<kasahara.tatsuhito@gmail.com> wrote:Hi.
In the code in src/backend/replication/logical/origin.c,
the error code "ERRCODE_CONFIGURATION_LIMIT_EXCEEDED" is given
when a checksum check results in an error,
but "ERRCODE_ DATA_CORRUPTED" seems to be more appropriate.+1. Your observation looks correct to me and the error code suggested
by you seems appropriate. We use ERRCODE_DATA_CORRUPTED in
ReadTwoPhaseFile() for similar error.
Agreed, +1 for changing this.
--
Daniel Gustafsson https://vmware.com/
On Thu, Aug 26, 2021 at 4:11 PM Daniel Gustafsson <daniel@yesql.se> wrote:
On 26 Aug 2021, at 12:03, Amit Kapila <amit.kapila16@gmail.com> wrote:
On Thu, Aug 26, 2021 at 3:18 PM Kasahara Tatsuhito
<kasahara.tatsuhito@gmail.com> wrote:Hi.
In the code in src/backend/replication/logical/origin.c,
the error code "ERRCODE_CONFIGURATION_LIMIT_EXCEEDED" is given
when a checksum check results in an error,
but "ERRCODE_ DATA_CORRUPTED" seems to be more appropriate.+1. Your observation looks correct to me and the error code suggested
by you seems appropriate. We use ERRCODE_DATA_CORRUPTED in
ReadTwoPhaseFile() for similar error.Agreed, +1 for changing this.
I think we need to backpatch this till 9.6 as this is introduced by
commit 5aa2350426. Any objections to that?
--
With Regards,
Amit Kapila.
On Fri, Aug 27, 2021 at 10:02:26AM +0530, Amit Kapila wrote:
I think we need to backpatch this till 9.6 as this is introduced by
commit 5aa2350426. Any objections to that?
None.
--
Michael
On Fri, Aug 27, 2021 at 1:32 PM Amit Kapila <amit.kapila16@gmail.com> wrote:
On Thu, Aug 26, 2021 at 4:11 PM Daniel Gustafsson <daniel@yesql.se> wrote:
On 26 Aug 2021, at 12:03, Amit Kapila <amit.kapila16@gmail.com> wrote:
On Thu, Aug 26, 2021 at 3:18 PM Kasahara Tatsuhito
<kasahara.tatsuhito@gmail.com> wrote:Hi.
In the code in src/backend/replication/logical/origin.c,
the error code "ERRCODE_CONFIGURATION_LIMIT_EXCEEDED" is given
when a checksum check results in an error,
but "ERRCODE_ DATA_CORRUPTED" seems to be more appropriate.+1. Your observation looks correct to me and the error code suggested
by you seems appropriate. We use ERRCODE_DATA_CORRUPTED in
ReadTwoPhaseFile() for similar error.Agreed, +1 for changing this.
I think we need to backpatch this till 9.6 as this is introduced by
commit 5aa2350426.
+1
Best regards,
--
Tatsuhito Kasahara
kasahara.tatsuhito _at_ gmail.com
On 27 Aug 2021, at 06:32, Amit Kapila <amit.kapila16@gmail.com> wrote:
I think we need to backpatch this till 9.6 as this is introduced by
commit 5aa2350426. Any objections to that?
No, that seems appropriate.
--
Daniel Gustafsson https://vmware.com/
On Fri, Aug 27, 2021 at 12:47 PM Daniel Gustafsson <daniel@yesql.se> wrote:
On 27 Aug 2021, at 06:32, Amit Kapila <amit.kapila16@gmail.com> wrote:
I think we need to backpatch this till 9.6 as this is introduced by
commit 5aa2350426. Any objections to that?No, that seems appropriate.
Pushed.
--
With Regards,
Amit Kapila.
On Mon, Aug 30, 2021 at 5:35 PM Amit Kapila <amit.kapila16@gmail.com> wrote:
On Fri, Aug 27, 2021 at 12:47 PM Daniel Gustafsson <daniel@yesql.se> wrote:
On 27 Aug 2021, at 06:32, Amit Kapila <amit.kapila16@gmail.com> wrote:
I think we need to backpatch this till 9.6 as this is introduced by
commit 5aa2350426. Any objections to that?No, that seems appropriate.
Pushed.
Thanks !
--
Tatsuhito Kasahara
kasahara.tatsuhito _at_ gmail.com