Small review comment on pg_checksums
Hi,
While reading the pg_checksums code I found the following comment
"Check if cluster is running" is not placed at right place.
/* Check if cluster is running */
ControlFile = get_controlfile(DataDir, &crc_ok);
if (!crc_ok)
{
pg_log_error("pg_control CRC value is incorrect");
exit(1);
}
if (ControlFile->pg_control_version != PG_CONTROL_VERSION)
{
pg_log_error("cluster is not compatible with this version of
pg_checksums");
exit(1);
}
if (ControlFile->blcksz != BLCKSZ)
{
pg_log_error("database cluster is not compatible");
fprintf(stderr, _("The database cluster was initialized with
block size %u, but pg_checksums was compiled with block size %u.\n"),
ControlFile->blcksz, BLCKSZ);
exit(1);
}
if (ControlFile->state != DB_SHUTDOWNED &&
ControlFile->state != DB_SHUTDOWNED_IN_RECOVERY)
{
pg_log_error("cluster must be shut down");
exit(1);
}
So I'd like to propose a small fix for that; move the comment to the
right place and add another comment. Please find an attached small
patch.
Regards,
--
Masahiko Sawada
NIPPON TELEGRAPH AND TELEPHONE CORPORATION
NTT Open Source Software Center
Attachments:
fix_pg_checksums.patchapplication/octet-stream; name=fix_pg_checksums.patchDownload+2-1
On Thu, Jun 06, 2019 at 05:16:30PM +0900, Masahiko Sawada wrote:
So I'd like to propose a small fix for that; move the comment to the
right place and add another comment. Please find an attached small
patch.
No objections to that. Perhaps we should also mention that this does
not protect from someone starting the cluster concurrently and that
the reason why we require a clean shutdown is that we may get checksum
failures because of torn pages?
--
Michael
On Thu, Jun 6, 2019 at 10:21 PM Michael Paquier <michael@paquier.xyz> wrote:
On Thu, Jun 06, 2019 at 05:16:30PM +0900, Masahiko Sawada wrote:
So I'd like to propose a small fix for that; move the comment to the
right place and add another comment. Please find an attached small
patch.No objections to that. Perhaps we should also mention that this does
not protect from someone starting the cluster concurrently and that
the reason why we require a clean shutdown is that we may get checksum
failures because of torn pages?
Agreed. Please find an attached patch.
Regards,
--
Masahiko Sawada
NIPPON TELEGRAPH AND TELEPHONE CORPORATION
NTT Open Source Software Center
Attachments:
fix_pg_checksums_v2.patchapplication/octet-stream; name=fix_pg_checksums_v2.patchDownload+6-1
On Fri, Jun 07, 2019 at 03:30:35PM +0900, Masahiko Sawada wrote:
Agreed. Please find an attached patch.
Thanks, committed.
--
Michael
On Fri, Jun 7, 2019 at 8:52 PM Michael Paquier <michael@paquier.xyz> wrote:
On Fri, Jun 07, 2019 at 03:30:35PM +0900, Masahiko Sawada wrote:
Agreed. Please find an attached patch.
Thanks, committed.
Thank you!
Regards,
--
Masahiko Sawada
NIPPON TELEGRAPH AND TELEPHONE CORPORATION
NTT Open Source Software Center