Calling pgstat_report_wait_end() before ereport(ERROR) - Mailing list pgsql-hackers

From Masahiko Sawada
Subject Calling pgstat_report_wait_end() before ereport(ERROR)
Date
Msg-id CAD21AoDhHYVq5KkXfkaHhmjA-zJYj-e4teiRAJefvXuKJz1tKQ@mail.gmail.com
Whole thread Raw
Responses Re: Calling pgstat_report_wait_end() before ereport(ERROR)
Re: Calling pgstat_report_wait_end() before ereport(ERROR)
List 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



pgsql-hackers by date:

Previous
From: Masahiko Sawada
Date:
Subject: Re: Transparent data encryption support as an extension
Next
From: ilmari@ilmari.org (Dagfinn Ilmari Mannsåker)
Date:
Subject: Re: REINDEX CONCURRENTLY 2.0