On Tue, Jul 4, 2017 at 3:25 PM, Kuntal Ghosh <kuntalghosh.2007@gmail.com> wrote:
> On Mon, Jul 3, 2017 at 6:50 PM, Michael Paquier
> <michael.paquier@gmail.com> wrote:
>> pg_basebackup/ with fe_recvint64() has its own way to do things, as
>> does the large object things in libpq. I would think that at least on
>> HEAD things could be gathered with a small set of routines. It is
>> annoying to have a third copy of the same thing. OK that's not
>> invasive but src/common/ would be a nice place to put things.
>>
>> - if (PQgetlength(res, 0, 1) != sizeof(int32))
>> + if (PQgetlength(res, 0, 1) != sizeof(long long int))
>> This had better be int64.
>
> Thank you. I'll do the changes and submit the revised patch. I've
> added an entry in commitfest for the same.
Yeah... I was halfway into writing a patch myself. As it seems that
you are planning to submit a new version, and because it won't be nice
to step on your toes, I'll wait for your updated version.
I am also attaching an updated version of your script which triggers
the error if you use for a larger size for --with-segsize. I have
mainly played with a segment size of 16GB to check for all potential
overflows and with more than 4GB of page offsets. I have spent a
couple of hours into looking at those overflow issues, and only
fetch_file_range is at risk as the data inserted into the page map
itself is a block number, which is fine as a 32-bit integer.
Here are some notes on what the patch should do.
1) In fetch_file_range, use uint64 for begin and end. Using int4 for
the length calculation is fine as this cannot go past CHUNKSIZE. I
would love to be able to add a static assertion on frontend tools
though. But this meritates a comment.
2) Using int8 for the definition of fetchchunks would be more
consistent with the existing code.
3) About the logs:
+ pg_log(PG_DEBUG, "received chunk for file \"%s\", offset %lld,
size %d\n",
filename, chunkoff, chunksize);
This should not use %lld but UINT64_FORMAT.
4) long long int needs to be replaced by uint64.
5) Using ntohll as name results in a compilation failure on MacOS.
Let's avid any fancy refactoring for now, and just use a static
routine with a name similar to what is in receivelog.c. You could as
well just use the routine of this file.
--
Michael
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers