Re: snapshot too old, configured by time

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