Transfer encryption metadata in the commit when DirectUpdateHandler2.closeWriter is called.

[bruno-roustant]

The problem is described in the corresponding issue.
It happens when documents are inserted but without commit, and the IndexWriter is closed, which triggers an auto-commit from DirectUpdateHandler2.closeWriter. Unfortunately this auto-commit does not really call the DirectUpdateHandler2.commit code but rather copies it, and we miss a call to DirectUpdateHandler2.shouldCommit to manage the encryption commit metadata.

Actually this requires to change DirectUpdateHandler2.closeWriter in the Solr project.
This PR shows the small required change (see comment in the DirectUpdateHandler2 copy), and verifies the fix with a test.
But I don't think I'll merge this PR. I'll rather change upstream and wait for the next release to upgrade here.

[bruno-roustant]

This is the small change required for encryption. I'll do the change upstream.
Actually, from the comment line 959 above, ideally we should call commit(cmd) here instead of copying the code from the commit() method. But apparently many tests failed. So, given this code is a copy of commit(), we should also copy the shouldCommit() call part (which fixes our issue here with encryption).

[bruno-roustant]

@dsmiley why did you close this PR?
It seems you wrote some comments, but I don't see them anymore.

[dsmiley]

:embarrassed: totally an accident; sorry! I saw it was closed (didn't notice it was me!), so I deleted my comment to shift it to #109

I suppose IndexWriter.commit() is risky for index encryption... we must always guarantee there is commit metadata? This seems fragile; hard to guarantee, and with bad consequences.

I did find-usages and I see two other callers, and would like your opinion:

• CoreContainer.reload
• SolrIndexSplitter.doSplit

But finding all callers and adding protections, again, feels fragile. Like if we do index encryption, maybe we need a stronger lower level guarantee that the metadata is there. I haven't thought about this too much other than observing the fragility.

[bruno-roustant]

For the SolrIndexSplitter.doSplit, it is already handled in EncryptionUpdateHandler which extends DirectUpdateHandler2.split(). This method creates the SolrIndexSplitter and gives the Command which contains the metadata.

For CoreContainer.reload, I missed it. Good point.
I don't think there are other callers currently. But I agree this is fragile while Solr evolves. I hope that when this encryption module comes into Solr, we can encapsulate the Lucene IndexWriter in a way that it does the commit metadata transfer systematically at nearly the Lucene level.

[bruno-roustant]

I created a PR upstream to fix this.