Calling pgstat_report_wait_end() before ereport(ERROR)

From: Masahiko Sawada <sawada(dot)mshk(at)gmail(dot)com>
To: PostgreSQL-development <pgsql-hackers(at)postgresql(dot)org>
Subject: Calling pgstat_report_wait_end() before ereport(ERROR)
Date: 2019-04-12 10:27:44
Message-ID: CAD21AoDhHYVq5KkXfkaHhmjA-zJYj-e4teiRAJefvXuKJz1tKQ@mail.gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

Hi,

There are something like the following code in many places in PostgreSQL code.

pgstat_report_wait_start(WAIT_EVENT_xxx);
if (write(...) != len)
{
ereport(ERROR, ...);
}
pgstat_report_wait_end();

Almost of these places don't call pgstat_report_wait_end() before
ereport(ERROR) but some places. Especially in RecreateTwoPhaseFile()
we have,

/* Write content and CRC */
errno = 0;
pgstat_report_wait_start(WAIT_EVENT_TWOPHASE_FILE_WRITE);
if (write(fd, content, len) != len)
{
int save_errno = errno;

pgstat_report_wait_end();
CloseTransientFile(fd);

/* if write didn't set errno, assume problem is no disk space */
errno = save_errno ? save_errno : ENOSPC;
ereport(ERROR,
(errcode_for_file_access(),
errmsg("could not write file \"%s\": %m", path)));
}
if (write(fd, &statefile_crc, sizeof(pg_crc32c)) != sizeof(pg_crc32c))
{
int save_errno = errno;

pgstat_report_wait_end();
CloseTransientFile(fd);

/* if write didn't set errno, assume problem is no disk space */
errno = save_errno ? save_errno : ENOSPC;
ereport(ERROR,
(errcode_for_file_access(),
errmsg("could not write file \"%s\": %m", path)));
}
pgstat_report_wait_end();

/*
* We must fsync the file because the end-of-replay checkpoint will not do
* so, there being no GXACT in shared memory yet to tell it to.
*/
pgstat_report_wait_start(WAIT_EVENT_TWOPHASE_FILE_SYNC);
if (pg_fsync(fd) != 0)
{
int save_errno = errno;

CloseTransientFile(fd);
errno = save_errno;
ereport(ERROR,
(errcode_for_file_access(),
errmsg("could not fsync file \"%s\": %m", path)));
}
pgstat_report_wait_end();

First two call pgstat_report_wait_end() but third one doesn't.

As far as I know there are three places where call
pgstat_report_wait_end before ereport(ERROR): two in twophase.c
andanother in copydir.c(at L199). Since we eventually call
pgstat_report_wait_end() in AbortTransaction(). I think that we don't
need to call pgstat_report_wait_end() if we're going to raise an error
just after that. Is that right?

Regards,

--
Masahiko Sawada
NIPPON TELEGRAPH AND TELEPHONE CORPORATION
NTT Open Source Software Center

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Dagfinn Ilmari Mannsåker 2019-04-12 11:11:12 Re: REINDEX CONCURRENTLY 2.0
Previous Message Masahiko Sawada 2019-04-12 10:04:16 Re: Transparent data encryption support as an extension