I'm starting to read through this and have a few questions. Kevin Grittner wrote:
> 4. Tests have been added. They are currently somewhat minimal, > since this is using a whole new technique for testing from any > existing committed tests -- I wanted to make sure that this > approach to testing was OK with everyone before expanding it. If > it is, I assume we will want to move some of the more generic > portions to a .pm file to make it available for other tests. I think your test module is a bit unsure about whether it's called tso or sto. I would place it inside src/test/modules, so that buildfarm runs it automatically; I'm not sure it will pick up things in src/test. (This is the whole point of src/test/modules, after all). I haven't written or reviewed our TestLib stuff so I can't comment on whether the test code is good or not. How long is the test supposed to last? It bothers me a bit to add #include rel.h to snapmgr.h because of the new macro definition. It seems out of place. I would instead move the macro closer to bufmgr headers or rel.h itself perhaps. Also, the definition isn't accompanied by an explanatory comment (other than why is it a macro instead of a function). So if I understand correctly, every call to BufferGetPage needs to have a TestForOldSnapshot immediately afterwards? It seems easy to later introduce places that fail to test for old snapshots. What happens if they do? Does vacuum remove tuples anyway and then the query returns wrong results? That seems pretty dangerous. Maybe the snapshot could be an argument of BufferGetPage? Please have the comments be clearer on what "align to minute boundaries" means. Is it floor or ceil? Also, in the OldSnapshotControlData big comment you talk about "length" and then the variable is called "used". Maybe that should be more consistent, for clarity. How large is the struct kept in shmem? I don't think the size is configurable, is it? I would have guessed that the size would depend on the GUC param ... -- Álvaro Herrera http://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Remote DBA, 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