Re: Minor race-condition problem during database startup

Zdenek Kotala napsal(a):
> Tom Lane napsal(a):
> 
>> What seems to have happened is that the bgwriter didn't get as far as
>> the first line of BackgroundWriterMain before the client backend tried
>> to issue a checkpoint request.
>>
>> This is obviously a pretty minor issue, but it still seems worth fixing.
>> We could either try to make sure that BgWriterShmem->bgwriter_pid gets
>> set before the postmaster "opens its doors" for clients, or allow
>> RequestCheckpoint() to wait a little bit if needed for the bgwriter
>> to come ready.  The latter seems like a more localized change.
> 
> I think, postmaster should wait until bgwriter is not up.
> 
> Another strange thing in RequestCheckpoint() is following code:
> 
> 00926         else if (kill(BgWriterShmem->bgwriter_pid, SIGINT) != 0)
> 00927         {
> 00928             if (ntries >= 20)       /* max wait 2.0 sec */
> 00929             {
> 00930                 elog((flags & CHECKPOINT_WAIT) ? ERROR : LOG,
> 00931                      "could not signal for checkpoint: %m");
> 00932                 break;
> 00933             }
> 00934         }
> 
> By my opinion there is not reason to retry kill call, because it fails 
> only in situation if process does not exist or caller does not have 
> permission to send a signal. If one of these situation happens it means 
> that bgwriter is dead or memory is corrupted. Maybe it is time for panic 
> (or fatal)?

If I think more about it, that code is here because it probably tries to bypass 
time when bgwriter is restarted. But It could invoke another race condition in 
situation when old bgwriter pid is recycled to another backend (or another 
postgresql server) in mean time. Should postmaster/bgwriter clean 
BgWriterShmem->bgwriter_pid at the end? I think it should be zeroed in places 
where BgWriterPID is set to zero (in postmaster.c).
    Zdenek