acid-state/acid-state — pull request #38 by edsko (2014-06-10 17:55:46+00:00)
Title: Add two failing test cases, fix one.

The test suite `examples/errors/Exceptions` contains two different kinds of exceptions:

1. The transaction contains an explicit `fail` (or `error`) _but can still be serialized_ (`failEvent`, `errorEvent`)

2. The transaction returns an undefined state (`stateError`).

This pull request introduced two further classes of errors, and fixes one of them:

3. The transaction returns a state which is partially defined: defined in the spine but containing nested errors (`stateNestedError1`). This is the class of error which this pull request does _not_ fix (see below).

4. The transaction itself cannot be serialized (`stateNestedError2` with an `error` value as argument).

For the latter error, `scheduleLocalUpdate` calls `pushEntry` on the encoding of the transaction to write the transaction to the log, but this encoding happens lazily. This means that the exception will happen in the file writer thread spawned by `openFileLog`; in particular, it will happen in `fileWriter` on the call to `writeToDisk`, which means the subsequent call to `sequence_ action` never happens. This means that the call to `putMVar` in `scheduleLocalUpdate` never happens and hence the call to `takeMVar` in `update` will be forever blocked.

We can fix this by reenabling the call to `evaluate` that was commented out in `scheduleLocalUpdate`, which is what this pull request does. This will ensure that the exception happen in _this_ thread, and the problem is avoided.

The third problem mentioned above (exemplified by test `stateNestedError1`), however, is trickier to avoid. In this case any update will go through fine, but we will have a similar problem when we try to write a checkpoint (`createLocalCheckpoint` as well as `createCheckpointAndClose`). In this case, the state contains a nested error, so when the file writer thread attempts to write out the entire state as part of the `Checkpoint` it will again throw an exception, and again the `putMVar` never happens. A similar fix to the one for `scheduleLocalUpdate` would be possible, of course, simply by calling evaluate on the encoded state. However, unlike individual transactions, insisting that the full bytestring for the entire database state is loaded into memory before it is written to disk might be a bad idea.

Instead, it is probably necessary to adapt `fileWriter` to catch exceptions. This probably means it needs to get _two_ lists of actions to execute: one on successful writing, one for exception cases. However, this is a bigger change so I didn't want to make it without discussion. (Once such a change is made, the call to `evaluate` in `scheduleLocalUpdate` will be obsolete).

(As an aside, the call to `pushEntry` in `scheduleLocalColdUpdate` may be suspect, too.)