On Tue, Jul 4, 2017 at 3:25 PM, Kuntal Ghosh <[email protected]> wrote: > On Mon, Jul 3, 2017 at 6:50 PM, Michael Paquier > <[email protected]> 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
rewind-test-case.sh
Description: Bourne shell script
-- Sent via pgsql-hackers mailing list ([email protected]) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
