On 2018-05-10 09:50:03 +0800, Craig Ringer wrote: > while ((src = (RewriteMappingFile *) hash_seq_search(&seq_status)) != NULL) > { > if (FileSync(src->vfd, WAIT_EVENT_LOGICAL_REWRITE_SYNC) != 0) > - ereport(ERROR, > + ereport(PANIC, > (errcode_for_file_access(), > errmsg("could not fsync file \"%s\": %m", src->path)));
To me this (and the other callers) doesn't quite look right. First, I think we should probably be a bit more restrictive about when PANIC out. It seems like we should PANIC on ENOSPC and EIO, but possibly not others. Secondly, I think we should centralize the error handling. It seems likely that we'll acrue some platform specific workarounds, and I don't want to copy that knowledge everywhere.
Also, don't we need the same on close()?
Yes, we do, and that expands the scope a bit.
I agree with Robert that some sort of filter/macro is wise, though naming it clearly will be tricky.
I'll have a look.
On the queue for tomorrow.
Hi all.
I've revised the fsync patch with the cleanups discussed and gone through the close() calls.
AFAICS either socket closes, temp file closes, or (for WAL) already PANIC on close. It's mainly fd.c that needs amendment. Which I've done per the attached revised patch.