[
https://issues.apache.org/jira/browse/SOLR-14184?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
]
Chris M. Hostetter updated SOLR-14184:
--------------------------------------
Attachment: SOLR-14184.patch
Status: Open (was: Open)
Minimal patch for discussion that shows starting point for refactoring :
* the additions to TestInjection:
** {{public volatile static boolean skipIndexWriterCommitAndClose}}
** {{public static boolean injectSkipIndexWriterCommitAndClose(Object)}}
* the modifications to DUH2 to use this new injection method
** still supports {{commitOnClose}}
* modifications to a single existing test showing new usage
** TestTlogReplica
Note that unlike {{DUH2.commitOnClose}} this new approach doesn't _require_
tests to directly cleanup after themselves (even if they fail) because
{{SolrTestCaseJ4}} calls {{TestInjection.reset()}}
----
My initial thinking was to implement this along the lines of...
{code:java}
try {
// if this TestInjection triggers, we do some simple rollback()
// (which closes the underlying IndexWriter) and then return immediately
//
// DO NOT let the AssertionError propogate
assert TestInjection.injectSkipIndexWriterCommitAndClose(writer);
} catch (AssertionError ae) {
log.warn("Skipping commit for IndexWriter.close() due to TestInjection");
// TODO: do existing rollback
return;
}
{code}
...so that {{injectSkipIndexWriterCommitAndClose}} could be randomized using
the {{true:80}} pair syntax of other TestInjection methods (ie: 80% of the time
fail to due to commit) ... but in skimming some of the tests that currently set
{{DUH2.commitOnClose=false}} they seem to _depend_ on that ensuring there will
be no commit – meaning even if those tests used {{true:100}} they would still
fail if assertions were disabled.
So instead I went the simpler route of a direct boolean – but moved to
{{TestInjection}} to make it clear what it's for – while flipping it's
meaning/naming to reflect that "enabling" this {{TestInjection}} option to be
"true" will cause the {{IW.commit}} to be "skipped"
----
The next steps are the rote refactoring of {{DUH2.commitOnClose = false}} ->
{{TI.skipIndexWriterCommitOnClose = true}}
> replace DirectUpdateHandler2.commitOnClose with something in TestInjection
> --------------------------------------------------------------------------
>
> Key: SOLR-14184
> URL: https://issues.apache.org/jira/browse/SOLR-14184
> Project: Solr
> Issue Type: Test
> Security Level: Public(Default Security Level. Issues are Public)
> Reporter: Chris M. Hostetter
> Assignee: Chris M. Hostetter
> Priority: Major
> Attachments: SOLR-14184.patch
>
>
> {code:java}
> public static volatile boolean commitOnClose = true; // TODO: make this a
> real config option or move it to TestInjection
> {code}
> Lots of tests muck with this (to simulate unclean shutdown and force tlog
> replay on restart) but there's no garuntee that it is reset properly.
> It should be replaced by logic in {{TestInjection}} that is correctly cleaned
> up by {{TestInjection.reset()}}
--
This message was sent by Atlassian Jira
(v8.3.4#803005)
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscr...@lucene.apache.org
For additional commands, e-mail: issues-h...@lucene.apache.org
