On Sun, 2008-10-05 at 14:51 -0400, Alvaro Herrera wrote:
> Simon Riggs wrote:
>
> > OK, spent long time testing various batching scenarios for this using a
> > custom test harness to simulate various spreads of xids in transaction
> > trees. All looks fine now.
>
> I had a look and was mostly rephrasing some comments and the README
> (hopefully I didn't make any of them worse than they were), when I
> noticed that the code to iterate thru pages could be refactored. I
> think the change makes the algorithm in TransactionIdSetTreeStatus
> easier to follow.
Yes, all fits on one screen when reading it now.
> I also noticed that TransactionIdSetPageStatus has a "subcommit" arg
> which is unexplained. I sort-of understand the point, but I think it's
> better that you fill in the explanation in the header comment (marked
> with XXX)
I've changed the logic slightly to remove the need for the subcommit
argument. So no need to explain now.
Added an Assert to check for what should be an impossible call.
Example provided in comments for a complex update.
> I hope I didn't break the code with the new function
> set_tree_status_by_pages -- please recheck that part.
Renamed to set_status_by_pages because we never use this on the whole
tree. Added comments to say that.
Overall, cleaner and more readable now. Thanks.
--
Simon Riggs www.2ndQuadrant.com
PostgreSQL Training, Services and Support