Now that the issue with padding seems to no longer exists since the
patch works both with and without padding, I went through the code and
here are some comments I have (in no particular order).
In CheckPointReplicationIdentifier:
+ * FIXME: Add a CRC32 to the end.
The function already does it (I guess you forgot to remove the comment).
Using max_replication_slots as limit for replication_identifier states
does not really make sense to me as replication_identifiers track remote
info while and slots are local and in case of master-slave replication
you need replication identifiers but don't need slots.
In bootstrap.c:
#define MARKNOTNULL(att) \
((att)->attlen > 0 || \
(att)->atttypid == OIDVECTOROID || \
- (att)->atttypid == INT2VECTOROID)
+ (att)->atttypid == INT2VECTOROID || \
+ strcmp(NameStr((att)->attname), "riname") == 0 \
+ )
Huh? Can this be solved in a nicer way?
Since we call XLogFlush with local_lsn as parameter, shouldn't we check
that it's actually within valid range?
Currently we'll get errors like this if set to invalid value:
ERROR: xlog flush request 123/123 is not satisfied --- flushed only to
0/168FB18
In AdvanceReplicationIndentifier:
+ /*
+ * XXX: should we restore into a hashtable and dump into shmem only
after
+ * recovery finished?
+ */
Probably no given that the function is also callable via SQL interface.
As I wrote in another email, I would like to integrate the RepNodeId and
CommitTSNodeId into single thing.
There are no docs for the new sql interfaces.
The replication_identifier.c might deserve some intro/notes text.
--
Petr Jelinek http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Training & Services
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
