A word of caution: relatedly to the issue demonstrated by the added
tests, an infinite loop can also occur, as it already could in
TestReindex_LevelDB. As it is, after all, a consequence of a race, I
haven't been able to determine what exactly makes the loop occur. But
what I observed is:
1) It seems to be occuring much more easily with LevelDB, which is why I
ended up just disabling TestReindex_LevelDB.
2) I've never seen it happen in TestReindex_Kvfile, but who knows.
3) I've seen it rarely happen with TestShowReindexRace_Kvfile, but it
seems that adding in TestShowReindexRace_Kvfile the kind of timed kill
that I had added TestReindex_LevelDB, actually makes the loop happen
much more often. And it ends up eclipsing the original issue that we
want to demonstrate, which is why I decided against it.
TL;DR: if you use -show_reindex_race=true , be prepared to maybe
have to kill(1) the test manually.
Change-Id: I47fd3c55363c8d0dda17ad19665115cb96f3d58f
Follow-up from the findings of
https://camlistore-review.googlesource.com/6227 which hinted that the
iter "err" field was not needed.
-Added Error check on iterator Closing.
-Removed Error call in Next, because it.it.Next already does it. See
func (i *dbIter) Next(), that checks i.err, which is the same as calling
Error(). (in github.com/syndtr/goleveldb/leveldb/db_iter.go)
-the closed field, and related check in Next are not strictly necessary,
because that's part of what Release does, in conjunction with func (i
*dbIter) Next() which is checking if we already released (look for i.dir
== dirReleased). But we're keeping it and the panic for the benefit of
detecting programmer's errors.
Also added the missing leveldb tests in index pkg.
Because of a potential infinite loop error (likely related to issue #756,
and not introduced by this CL), I've added a timer triggered panic to
break that loop when it happens.
Change-Id: I26e0815f1d85279f0ead7bf90daae2ae03f1af63
mpl