diff --git a/solr/core/src/java/org/apache/solr/update/DirectUpdateHandler2.java b/solr/core/src/java/org/apache/solr/update/DirectUpdateHandler2.java index d591ffc2017..e5f87271043 100644 --- a/solr/core/src/java/org/apache/solr/update/DirectUpdateHandler2.java +++ b/solr/core/src/java/org/apache/solr/update/DirectUpdateHandler2.java @@ -137,8 +137,15 @@ public class DirectUpdateHandler2 extends UpdateHandler { try { - commitTracker.addedDocument( cmd.commitWithin ); - softCommitTracker.addedDocument( cmd.commitWithin ); + boolean triggered = commitTracker.addedDocument( cmd.commitWithin ); + + if (!triggered) { + // if we hard commit, don't soft commit + softCommitTracker.addedDocument( cmd.commitWithin ); + } else { + // still inc softCommit + softCommitTracker.docsSinceCommit++; + } if (cmd.overwrite) { Term updateTerm; diff --git a/solr/core/src/test/org/apache/solr/update/AutoCommitTest.java b/solr/core/src/test/org/apache/solr/update/AutoCommitTest.java index 081e65a6269..c4b65287780 100644 --- a/solr/core/src/test/org/apache/solr/update/AutoCommitTest.java +++ b/solr/core/src/test/org/apache/solr/update/AutoCommitTest.java @@ -282,7 +282,7 @@ public class AutoCommitTest extends AbstractSolrTestCase { assertTrue(trigger.waitForNewSearcher(10000)); assertQ("should find 10", req("*:*") ,"//result[@numFound=10]" ); - assertEquals( 2, softTracker.getCommitCount()); + assertEquals( 1, softTracker.getCommitCount()); assertEquals( 1, tracker.getCommitCount()); } @@ -362,87 +362,4 @@ public class AutoCommitTest extends AbstractSolrTestCase { assertQ("now it should", req("id:500") ,"//result[@numFound=1]" ); assertQ("but not this", req("id:531") ,"//result[@numFound=0]" ); } - - public void testSoftAndHardCommitMaxTime() throws Exception { - SolrCore core = h.getCore(); - NewSearcherListener trigger = new NewSearcherListener(); - core.registerNewSearcherListener(trigger); - DirectUpdateHandler2 updater = (DirectUpdateHandler2) core.getUpdateHandler(); - CommitTracker hardTracker = updater.commitTracker; - CommitTracker softTracker = updater.softCommitTracker; - - // too low of a number can cause a slow host to commit before the test code checks that it - // isn't there... causing a failure at "shouldn't find any" - softTracker.timeUpperBound = 1200; - softTracker.docsUpperBound = -1; - hardTracker.timeUpperBound = 3000; - hardTracker.docsUpperBound = -1; - // updater.commitCallbacks.add(trigger); - - XmlUpdateRequestHandler handler = new XmlUpdateRequestHandler(); - handler.init( null ); - - MapSolrParams params = new MapSolrParams( new HashMap() ); - - // Add a single document - SolrQueryResponse rsp = new SolrQueryResponse(); - SolrQueryRequestBase req = new SolrQueryRequestBase( core, params ) {}; - req.setContentStreams( toContentStreams( - adoc("id", "529", "field_t", "what's inside?", "subject", "info"), null ) ); - trigger.reset(); - handler.handleRequest( req, rsp ); - - // Check it it is in the index - assertQ("shouldn't find any", req("id:529") ,"//result[@numFound=0]" ); - - // Wait longer than the autocommit time - assertTrue(trigger.waitForNewSearcher(30000)); - trigger.reset(); - req.setContentStreams( toContentStreams( - adoc("id", "530", "field_t", "what's inside?", "subject", "info"), null ) ); - handler.handleRequest( req, rsp ); - - // Now make sure we can find it - assertQ("should find one", req("id:529") ,"//result[@numFound=1]" ); - // But not this one - assertQ("should find none", req("id:530") ,"//result[@numFound=0]" ); - - // Delete the document - assertU( delI("529") ); - assertQ("deleted, but should still be there", req("id:529") ,"//result[@numFound=1]" ); - // Wait longer than the autocommit time - assertTrue(trigger.waitForNewSearcher(15000)); - trigger.reset(); - req.setContentStreams( toContentStreams( - adoc("id", "550", "field_t", "what's inside?", "subject", "info"), null ) ); - handler.handleRequest( req, rsp ); - assertEquals( 2, softTracker.getCommitCount() ); - assertQ("deleted and time has passed", req("id:529") ,"//result[@numFound=0]" ); - - // now make the call 5 times really fast and make sure it - // only commits once - req.setContentStreams( toContentStreams( - adoc("id", "500" ), null ) ); - for( int i=0;i<5; i++ ) { - handler.handleRequest( req, rsp ); - } - assertQ("should not be there yet", req("id:500") ,"//result[@numFound=0]" ); - - // Wait longer than the autocommit time - assertTrue(trigger.waitForNewSearcher(15000)); - trigger.reset(); - - req.setContentStreams( toContentStreams( - adoc("id", "531", "field_t", "what's inside?", "subject", "info"), null ) ); - handler.handleRequest( req, rsp ); - - // depending on timing, you might see 2 or 3 soft commits - int softCommitCnt = softTracker.getCommitCount(); - assertTrue("commit cnt:" + softCommitCnt, softCommitCnt == 2 - || softCommitCnt == 3); - assertEquals(1, hardTracker.getCommitCount()); - - assertQ("now it should", req("id:500") ,"//result[@numFound=1]" ); - assertQ("but not this", req("id:531") ,"//result[@numFound=0]" ); - } }