Thread: 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
Attachment
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