Hi,
Thanks for this patch. I have a few comments.
> Hmm. Makes sense to me (perhaps you have played with an LLM to find
> out fread() and fwrite() patterns that would need an event?), but I
> don't think that the part about COPY TO is done right. The wait event
> should be set during the fwrite() call only, similarly to the COPY
> FROM case, by saving the result returned by fwrite() in a separate
> variable.
I see other cases such as inside copy_file() in which the read() is wrapped
by wait_start and wait_end, but in the write() case, the wait_end() is after
the error handling ( as the attached v1 ).
```
pgstat_report_wait_start(WAIT_EVENT_COPY_FILE_READ);
nbytes = read(srcfd, buffer, COPY_BUF_SIZE);
pgstat_report_wait_end();
.....
..........
pgstat_report_wait_start(WAIT_EVENT_COPY_FILE_WRITE);
if ((int) write(dstfd, buffer, nbytes) != nbytes)
{
/* if write didn't set errno, assume problem is no disk space */
if (errno == 0)
errno = ENOSPC;
ereport(ERROR,
(errcode_for_file_access(),
errmsg("could not write to file \"%s\": %m", tofile)));
}
pgstat_report_wait_end();
```
another example is write_relmap_file():
```
/* Write new data to the file. */
pgstat_report_wait_start(WAIT_EVENT_RELATION_MAP_WRITE);
if (write(fd, newmap, sizeof(RelMapFile)) != sizeof(RelMapFile))
{
/* if write didn't set errno, assume problem is no disk space */
if (errno == 0)
errno = ENOSPC;
ereport(ERROR,
(errcode_for_file_access(),
errmsg("could not write file \"%s\": %m",
maptempfilename)));
}
pgstat_report_wait_end();
```
I don't see any issue with this approach for fwrite().
Another comment. Wouldn't be better to use "COPY FROM" and "COPY TO" in the
names to make it more obvious they are related to the COPY command?
so instead of:
+COPY_DATA_READ "Waiting for a read from a file or program during COPY FROM."
+COPY_DATA_WRITE "Waiting for a write to a file or program during COPY TO."
this:
COPY_FROM_READ: "Waiting to read data from a file or program during COPY FROM."
COPY_TO_WRITE: "Waiting to write data to a file or program during COPY TO."
--
Sami Imseih
Amazon Web Services (AWS)
