Re: Streaming replication and WAL archive interactions

<div dir="ltr"><br /><br />On Thu, Apr 16, 2015 at 8:57 PM, Heikki Linnakangas wrote:<br />> Oh, hang on, that's not
necessarilytrue. On promotion, the standby archives<br />> the last, partial WAL segment from the old timeline.
That'sjust wrong<br />> (<a
href="http://www.postgresql.org/message-id/52FCD37C.3070806@vmware.com">http://www.postgresql.org/message-id/52FCD37C.3070806@vmware.com</a>),
andin<br />> fact I somehow thought I changed that already, but apparently not. So let's<br />> stop doing
that.<br/><br />Er. Are you planning to prevent the standby from archiving the last partial segment from the old
timelineat promotion? I thought from previous discussions that we should do it as master (be it crashed, burned,
burriedor dead) may not have the occasion to do it. By preventing its archiving you close the door to the case where
masterdid not have the occasion to archive it.<br /><br />+/* */<br />+static char primary_last_archived[MAX_XFN_CHARS
+1];<br />This is visibly missing a comment.<br /><br />As primary_last_archived is used only by
ProcessArchivalReport(),wouldn't it be better to pass it as argument to this function?<br /><br />+       /* Check that
thefilename the primary reported looks valid */<br />+       if (strlen(primary_last_archived) < 24 ||<br />+      
       strspn(primary_last_archived, "0123456789ABCDEF") != 24)<br />+               return;<br />Not related to this
patch,but we had better have a macro doing this job I think... It keeps spreading around.<br /><br />People may be
surprisedthat a base backup taken from a node that has archive_mode = on set (that's the case in a very large number of
cases)will not be able to work as-is as node startup will fail as follows:<br />FATAL:  archive_mode='on' cannot be
usedin archive recovery<br />HINT:  Use 'shared' or 'always' mode instead.<br />One idea would be to simply ignore the
factthat archive_mode = on on nodes in recovery instead of dropping an error. Note that I like the fact that it drops
anerror as that's clear, I just point the fact that people may be surprised that base backups are not working anymore
nowin this case.<br /><br />Are both WalSndArchivalReport() and WalSndArchivalReportIfNecessary() really necessary? I
thinkthat for simplicity you could merge them and use last_archival_report as a local variable.<br /><br />Creating a
dependencybetween the pgstat machinery and the WAL sender looks weak to me. For example with this patch a master cannot
stop,as it waits indefinitely:<br />LOG:  using stale statistics instead of current ones because stats collector is not
responding<br/>LOG:  sending archival report: <br />You could scan archive_status/ but that would be costly if there
aremany entries to scan and I think that walsender should be highly responsive. Or you could directly store the name of
thelastly archived WAL segment marked as .done in let's say archive_status/last_archived. An entry for that in the
controlfile does not seem the right place as a node may not have archive_mode enabled that's why I am not mentioning
it.<br/><br />Regards,<br />-- <br />Michael</div>