+               /*
+                * Looks like an xlog file. Parse it's position.

s/it's/its/

+                */
+               if (sscanf(dirent->d_name, "%08X%08X%08X", &tli, &log, &seg) != 
3)
+               {
+                       fprintf(stderr, _("%s: could not parse xlog filename 
\"%s\"\n"),
+                                       progname, dirent->d_name);
+                       disconnect_and_exit(1);
+               }
+               log *= XLOG_SEG_SIZE;

That multiplication by XLOG_SEG_SIZE could overflow, if logid is very high. It seems completely unnecessary, anyway,

s/IDENFITY_SYSTEM/IDENTIFY_SYSTEM/ (two occurrences)

In pg_basebackup, it would be a good sanity check to check that the systemid returned by IDENTIFY_SYSTEM in the main connection and the WAL-streaming connection match. Just to be sure that some connection pooler didn't hijack one of the connections and point to a different server. And better check timelineid too while you're at it.

How does this interact with synchronous replication? If a base backup that streams WAL is in progress, and you have synchronous_standby_names set to '*', I believe the in-progress backup will count as a standby for that purpose. That might give a false sense of security. synchronous_standby_names='*' is prone to such confusion in general, but it seems that it's particularly surprising if a running pg_basebackup lets a commit in synchronous replication to proceed. Maybe we just need a warning in the docs. I think we should advise that synchronous_standby_names='*' is dangerous in general, and cite this as one reason for that.

--
  Heikki Linnakangas
  EnterpriseDB   http://www.enterprisedb.com

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Reply via email to