pg.ddx.io pgsql-bugs@postgresql.org mailing list archive
help / color / mirror / Atom feedBUG #18486: Is there something wrong with the calculation in ReorderBufferChangeSize()?
4+ messages / 3 participants
[nested] [flat]
* BUG #18486: Is there something wrong with the calculation in ReorderBufferChangeSize()?
@ 2024-05-29 09:05 PG Bug reporting form <noreply@postgresql.org>
0 siblings, 1 reply; 4+ messages in thread
From: PG Bug reporting form @ 2024-05-29 09:05 UTC (permalink / raw)
To: pgsql-bugs@lists.postgresql.org; +Cc: xu.xw2008@163.com
The following bug has been logged on the website:
Bug reference: 18486
Logged by: Xingwang Xu
Email address: xu.xw2008@163.com
PostgreSQL version: 17beta1
Operating system: CentOS7.9
Description:
In the code related to logical replication, there is a function
ReorderBufferChangeSize(), which is used to calculate the size of a change
in memory.
When looking at the ReorderBufferChangeSize() function, I saw the following
code：
case REORDER_BUFFER_CHANGE_MESSAGE:
{
Size prefix_size = strlen(change->data.msg.prefix) + 1;
sz += prefix_size + change->data.msg.message_size +
sizeof(Size) + sizeof(Size);
break;
}
When calculating the change size of the message type, there are two
“sizeof(Size)” in the code. It is not clear why these two “sizeof(Size)” are
added and whether these two “sizeof(Size)” are redundant.
The data in change of message type is defined as:
struct
{
char *prefix;
Size message_size;
char *message;
} msg;
The size of msg seems to have been calculated at the beginning of
ReorderBufferChangeSize() with “sizeof(ReorderBufferChange)”.Is it only
necessary to add the specific data space occupied by *prefix and *message?
Not super familiar with this so please let me know if there's something I've
missed.
Thanks,
Xingwang xu
^ permalink raw reply [nested|flat] 4+ messages in thread
* Re: BUG #18486: Is there something wrong with the calculation in ReorderBufferChangeSize()?
@ 2024-06-04 05:28 Amit Kapila <amit.kapila16@gmail.com>
parent: PG Bug reporting form <noreply@postgresql.org>
0 siblings, 1 reply; 4+ messages in thread
From: Amit Kapila @ 2024-06-04 05:28 UTC (permalink / raw)
To: xu.xw2008@163.com; pgsql-bugs@lists.postgresql.org
On Thu, May 30, 2024 at 2:30 AM PG Bug reporting form
<noreply@postgresql.org> wrote:
>
> The following bug has been logged on the website:
>
> Bug reference: 18486
> Logged by: Xingwang Xu
> Email address: xu.xw2008@163.com
> PostgreSQL version: 17beta1
> Operating system: CentOS7.9
> Description:
>
> In the code related to logical replication, there is a function
> ReorderBufferChangeSize(), which is used to calculate the size of a change
> in memory.
>
> When looking at the ReorderBufferChangeSize() function, I saw the following
> code：
>
> case REORDER_BUFFER_CHANGE_MESSAGE:
> {
> Size prefix_size = strlen(change->data.msg.prefix) + 1;
>
> sz += prefix_size + change->data.msg.message_size +
> sizeof(Size) + sizeof(Size);
>
> break;
> }
>
> When calculating the change size of the message type, there are two
> “sizeof(Size)” in the code. It is not clear why these two “sizeof(Size)” are
> added and whether these two “sizeof(Size)” are redundant.
>
These two sizeof(Size) are added as while serializing or restoring
message change, we explicitly serialize/restore the size of the prefix
and the actual message. See
ReorderBufferSerializeChange()/ReorderBufferRestoreChange().
--
With Regards,
Amit Kapila.
^ permalink raw reply [nested|flat] 4+ messages in thread
* Re:Re: BUG #18486: Is there something wrong with the calculation in ReorderBufferChangeSize()?
@ 2024-06-06 10:05 Michael <xu.xw2008@163.com>
parent: Amit Kapila <amit.kapila16@gmail.com>
0 siblings, 1 reply; 4+ messages in thread
From: Michael @ 2024-06-06 10:05 UTC (permalink / raw)
To: Amit Kapila <amit.kapila16@gmail.com>; +Cc: pgsql-bugs@lists.postgresql.org
At 2024-06-04 13:28:52, "Amit Kapila" <amit.kapila16@gmail.com> wrote:
>On Thu, May 30, 2024 at 2:30 AM PG Bug reporting form
><noreply@postgresql.org> wrote:
>>
>> The following bug has been logged on the website:
>>
>> Bug reference: 18486
>> Logged by: Xingwang Xu
>> Email address: xu.xw2008@163.com
>> PostgreSQL version: 17beta1
>> Operating system: CentOS7.9
>> Description:
>>
>> In the code related to logical replication, there is a function
>> ReorderBufferChangeSize(), which is used to calculate the size of a change
>> in memory.
>>
>> When looking at the ReorderBufferChangeSize() function, I saw the following
>> code：
>>
>> case REORDER_BUFFER_CHANGE_MESSAGE:
>> {
>> Size prefix_size = strlen(change->data.msg.prefix) + 1;
>>
>> sz += prefix_size + change->data.msg.message_size +
>> sizeof(Size) + sizeof(Size);
>>
>> break;
>> }
>>
>> When calculating the change size of the message type, there are two
>> “sizeof(Size)” in the code. It is not clear why these two “sizeof(Size)” are
>> added and whether these two “sizeof(Size)” are redundant.
>>
>
>These two sizeof(Size) are added as while serializing or restoring
>message change, we explicitly serialize/restore the size of the prefix
>and the actual message. See
>ReorderBufferSerializeChange()/ReorderBufferRestoreChange().
>
Thanks for your reply.
I see in the code that the function ReorderBufferChangeSize() is used to calculate the space in memory, while ReorderBufferSerializeChange()/ReorderBufferRestoreChange() calculates the space on disk.
Am I right in understanding this?
With Regards,
Xingwang Xu
^ permalink raw reply [nested|flat] 4+ messages in thread
* Re: Re: BUG #18486: Is there something wrong with the calculation in ReorderBufferChangeSize()?
@ 2024-06-19 09:50 Amit Kapila <amit.kapila16@gmail.com>
parent: Michael <xu.xw2008@163.com>
0 siblings, 0 replies; 4+ messages in thread
From: Amit Kapila @ 2024-06-19 09:50 UTC (permalink / raw)
To: Michael <xu.xw2008@163.com>; +Cc: pgsql-bugs@lists.postgresql.org
On Thu, Jun 6, 2024 at 3:36 PM Michael <xu.xw2008@163.com> wrote:
>
> At 2024-06-04 13:28:52, "Amit Kapila" <amit.kapila16@gmail.com> wrote:
>
> >On Thu, May 30, 2024 at 2:30 AM PG Bug reporting form
> ><noreply@postgresql.org> wrote:
> >>
> >> The following bug has been logged on the website:
> >>
> >> Bug reference: 18486
> >> Logged by: Xingwang Xu
> >> Email address: xu.xw2008@163.com
> >> PostgreSQL version: 17beta1
> >> Operating system: CentOS7.9
> >> Description:
> >>
> >> In the code related to logical replication, there is a function
> >> ReorderBufferChangeSize(), which is used to calculate the size of a change
> >> in memory.
> >>
> >> When looking at the ReorderBufferChangeSize() function, I saw the following
> >> code：
> >>
> >> case REORDER_BUFFER_CHANGE_MESSAGE:
> >> {
> >> Size prefix_size = strlen(change->data.msg.prefix) + 1;
> >>
> >> sz += prefix_size + change->data.msg.message_size +
> >> sizeof(Size) + sizeof(Size);
> >>
> >> break;
> >> }
> >>
> >> When calculating the change size of the message type, there are two
> >> “sizeof(Size)” in the code. It is not clear why these two “sizeof(Size)” are
> >> added and whether these two “sizeof(Size)” are redundant.
> >>
> >
> >These two sizeof(Size) are added as while serializing or restoring
> >message change, we explicitly serialize/restore the size of the prefix
> >and the actual message. See
> >ReorderBufferSerializeChange()/ReorderBufferRestoreChange().
> >
>
> Thanks for your reply.
>
>
> I see in the code that the function ReorderBufferChangeSize() is used to calculate the space in memory, while ReorderBufferSerializeChange()/ReorderBufferRestoreChange() calculates the space on disk.
>
Shouldn't what we do in ReorderBufferRestoreChange() be the size of
the change in memory? Anyway, please see the following calculation in
ReorderBufferQueueMessage()
{
...
change->data.msg.prefix = pstrdup(prefix);
change->data.msg.message_size = message_size;
change->data.msg.message = palloc(message_size);
memcpy(change->data.msg.message, message, message_size);
...
As per this, the two sizeof(Size) seems to correspond to
'message_size' and 'message' in the above calculation. So, the
calculation corresponds to the size of the change we have in memory.
--
With Regards,
Amit Kapila.
^ permalink raw reply [nested|flat] 4+ messages in thread
end of thread, other threads:[~2024-06-19 09:50 UTC | newest]
Thread overview: 4+ messages (download: mbox mbox.gz follow: Atom feed)
-- links below jump to the message on this page --
2024-05-29 09:05 BUG #18486: Is there something wrong with the calculation in ReorderBufferChangeSize()? PG Bug reporting form <noreply@postgresql.org>
2024-06-04 05:28 ` Amit Kapila <amit.kapila16@gmail.com>
2024-06-06 10:05 ` Michael <xu.xw2008@163.com>
2024-06-19 09:50 ` Amit Kapila <amit.kapila16@gmail.com>
This inbox is served by DDX for PostgreSQL; see mirroring instructions
for how to clone and mirror all data and code used for this inbox