Re: Non-blocking archiver process - Mailing list pgsql-hackers
Hello Patrick
I did a review of your patch.
Initial Run
===========
===========
The patch applies cleanly to HEAD (196063d6761). All tests successfully pass on MacOS 15.7.
Comments
===========
1) Instead of `malloc` and `free` it should be `palloc` and `pfree`.FILE *archiveFd = NULL;
int archiveFileno;
3) Variable name `bytesRead` is rare in PG code base. It is used only two times, while `readBytes` is used four times. Other variants, like `nbytes` are more common. Let's pick some popular name.
4) Variable name `dwRc` is unique for the PG codebase and not really clear as for me. How about name it just `exitcode`?
5) `return` is redundant here as it will never be reached.
ereport(FATAL,
(errmsg_internal("Failed to malloc win32cmd %m")));
return false;
6) Same here, `free` and `return` are unreachable due ereport with fatal level.
ereport(FATAL,
(errmsg("CreateProcess() call failed: %m (error code %lu)",
GetLastError())));
free(win32cmd);
return false;
7) This loop can be stuck forever as `WaitForSingleObject` may return `WAIT_FAILED` and it's not always retriable.
while (true)--
{
CHECK_FOR_INTERRUPTS();
if (WaitForSingleObject(pi.hProcess, POLL_TIMEOUT_MSEC) == WAIT_OBJECT_0)
break;
}
pgsql-hackers by date: