Thanks Shai, great feedback; I folded it all in.
This is not only about disk-full, but about any transient IOE (e.g. ran out of file handles temporarily) that occur. So maybe change the issue's description (and also the CHANGES entry text)?
I'll fix the issue description, and I added a CHANGES.txt entry.
In TestIndexWriterReader I see this:
Yes, this is intentional (it was helpful debugging to see exactly
where I broke r2.isCurrent()).
You think maybe we should move FakeIOE to MDW so that other tests can use it?
I moved it, and shared from TestIndexWriterReader.java and
In ReaderPool.release: why don't you call writer.checkpoint()?
Right, I needed to increment changeCount (so that the writer writes a
new segments file), plus checkpoint with IFD (so it knows about the
new .del file) but NOT increment SIS.version (because there was in
fact no "actual" change to the index contents, just moving state from
RAM to disk). I put a comment explaining this, and also factored out
a new method.
Separately, I think it will be a useful utility to have a method like Utils.throwEx(Throwable t)
Good idea! I factored that out.
I think you can move the newIWC call to inside the 'if (w == null)'.
OK I fixed that.
I implemented the TODO about sub-classing CMS. Perhaps you'll want to include that in your patch as well:
I folded this in too ... thanks!
Also, separately, I managed to re-attach the fang I previously removed
from testThreadInterruptDeadlock/testTwoThreadsInterruptDeadlock, by
fixing IW.ReaderPool.dropAll to throw any exceptions it hits when save
is true, rather than suppressing them and dropping all readers from
the pool. I think there was another bug lurking there, because the
pool would lose changes previously if an exc was hit writing deletes
on close, and the app then called close again, and it succeeded.
I also fixed a few tests to clean up their static fields ... they were
sometimes angry during distributed beasting!