Code review comment for lp:~pbeaman/akiban-persistit/transaction-tree-management

Very nice! The meat of the changes really aren't too complicated and everything seems to fit nicely.

The only code change I would suggest, ask about really, is the live-ness determination in TimelyResource#prune(). I don't suspect it to be wrong but it is performing the same thing as MVV#prune(). They are operating on structures so different that I don't see a simple way to merge, but I had to convince myself they did the same thing. Can you think of anything to share the logic between them?

I would also like to see a handful of tests around what is in the directory tree, shutdown and recovery where multiple versions were present, and the (lack of) MVVs in writes to trees created in the same transaction.

The rest is a smattering of feedback, ranging from small to nits.

TimelyResource
- JavaDoc has a number of invalid links. Lots of other docs like that too, but this is all new.
- Should getVersionCount() should be public? Fairly harmless, but I don't think we leak information like that from other places (Exchange, etc)
- Should delete() check for _first != null? Seems like deleting a resource with no versions should be exception or assert worthy.

Exchange
- Why was the assertCorrecThread() removed from init(), hasChildren(), and isDirectoryExchange??
- The semantic change on checkThread (null-ing out of !set) is subtle. Perhaps renaming the method to checkAndSetThread or checkSetOrClearThread or something would help.
- A couple places call throttle() and now have multiple conditions. A helper may be in order.
- The addition of 'spareValue.isDefined() || !_tree.isTransationPrivate' in store() isn't clear to me. When would doMVCC be be true and the value be empty?

Tree
- End of the new paragraph of JavaDoc has two open <code> tags instead of open and close.
- version() has a // TODO about throwing a RuntimeException
- There are now a number of places where isValid() and isDeleted() must be called together. Maybe a new helper? A more drastic change, but perhaps safer, would be to make it protected in SharedResource and not promote it to public in Tree.

VolumeStructure
- remoteTree() now only returns true (or throws)

TimelyResourceTest
- testAndPruneResources(boolean) might want to append withTransactions to the assert messages. Does the CleanupManager need disabled to avoid spurious failures?
- doConcurrentTransaction() is called from threads so using fail() won't cause the test to fail but only generate console output. This needs to get propagated back to the main thread somewhere. There are lightly used helpers in ConcurrentUtil for this.
- deleteResources() uses a raw assert instead of jUnits.

TreeTransactionalLifetimeTest
- simplePruning() has a 5s sleep and a commented out manual cleanup call. The latter seems cleaner and more deterministic, does it not work?
- I'd like to see some tests around the behavior regarding steps. I believe having trees obey it just like data makes the most sense, which is what it does now, but confirming it would be good.
- The TExec helper is duplicating part of the functionality in ConcurrentUtil and doesn't handle errors in a way that will fail the test.