SOLR-2280: commitWithin ignored for a delete query

git-svn-id: https://svn.apache.org/repos/asf/lucene/dev/trunk@1235305 13f79535-47bb-0310-9956-ffa450edef68
This commit is contained in:
Jan Høydahl 2012-01-24 15:41:16 +00:00
parent 20d3c8335e
commit d0e4e731d9
17 changed files with 375 additions and 49 deletions

View File

@ -539,6 +539,8 @@ Bug Fixes
HyphenatedWordsFilter where they would create invalid offsets in
some situations, leading to problems in highlighting. (Robert Muir)
* SOLR-2280: commitWithin ignored for a delete query (Juan Grande via janhoy)
Other Changes
----------------------
* SOLR-2922: Upgrade commons-io and commons-lang to 2.1 and 2.6, respectively. (koji)

View File

@ -104,11 +104,8 @@ public class BinaryUpdateRequestHandler extends ContentStreamHandlerBase {
log.error("Exception while processing update request", e);
break;
}
if (update.getDeleteById() != null) {
delete(req, update.getDeleteById(), processor, true);
}
if (update.getDeleteQuery() != null) {
delete(req, update.getDeleteQuery(), processor, false);
if (update.getDeleteById() != null || update.getDeleteQuery() != null) {
delete(req, update, processor);
}
}
}
@ -121,18 +118,28 @@ public class BinaryUpdateRequestHandler extends ContentStreamHandlerBase {
return addCmd;
}
private void delete(SolrQueryRequest req, List<String> l, UpdateRequestProcessor processor, boolean isId) throws IOException {
for (String s : l) {
DeleteUpdateCommand delcmd = new DeleteUpdateCommand(req);
if (isId) {
private void delete(SolrQueryRequest req, UpdateRequest update, UpdateRequestProcessor processor) throws IOException {
SolrParams params = update.getParams();
DeleteUpdateCommand delcmd = new DeleteUpdateCommand(req);
if(params != null) {
delcmd.commitWithin = params.getInt(UpdateParams.COMMIT_WITHIN, -1);
}
if(update.getDeleteById() != null) {
for (String s : update.getDeleteById()) {
delcmd.id = s;
} else {
delcmd.query = s;
processor.processDelete(delcmd);
}
delcmd.id = null;
}
if(update.getDeleteQuery() != null) {
for (String s : update.getDeleteQuery()) {
delcmd.query = s;
processor.processDelete(delcmd);
}
processor.processDelete(delcmd);
}
}
@Override
public String getDescription() {
return "Add/Update multiple documents with javabin format";

View File

@ -155,6 +155,7 @@ class JsonLoader extends ContentStreamLoader {
assertNextEvent( JSONParser.OBJECT_START );
DeleteUpdateCommand cmd = new DeleteUpdateCommand(req);
cmd.commitWithin = commitWithin;
while( true ) {
int ev = parser.nextEvent();
@ -167,7 +168,9 @@ class JsonLoader extends ContentStreamLoader {
else if( "query".equals(key) ) {
cmd.query = parser.getString();
}
else {
else if( "commitWithin".equals(key) ) {
cmd.commitWithin = Integer.parseInt(parser.getString());
} else {
throw new SolrException(SolrException.ErrorCode.BAD_REQUEST, "Unknown key: "+key+" ["+parser.getPosition()+"]" );
}
}

View File

@ -181,6 +181,10 @@ class XMLLoader extends ContentStreamLoader {
// Parse the command
DeleteUpdateCommand deleteCmd = new DeleteUpdateCommand(req);
// First look for commitWithin parameter on the request, will be overwritten for individual <delete>'s
SolrParams params = req.getParams();
deleteCmd.commitWithin = params.getInt(UpdateParams.COMMIT_WITHIN, -1);
for (int i = 0; i < parser.getAttributeCount(); i++) {
String attrName = parser.getAttributeLocalName(i);
String attrVal = parser.getAttributeValue(i);
@ -188,6 +192,8 @@ class XMLLoader extends ContentStreamLoader {
// deprecated
} else if ("fromCommitted".equals(attrName)) {
// deprecated
} else if (XmlUpdateRequestHandler.COMMIT_WITHIN.equals(attrName)) {
deleteCmd.commitWithin = Integer.parseInt(attrVal);
} else {
XmlUpdateRequestHandler.log.warn("unexpected attribute delete/@" + attrName);
}

View File

@ -47,7 +47,6 @@ public class XmlUpdateRequestHandler extends ContentStreamHandlerBase {
// NOTE: This constant is for use with the <add> XML tag, not the HTTP param with same name
public static final String COMMIT_WITHIN = "commitWithin";
XMLInputFactory inputFactory;

View File

@ -91,6 +91,14 @@ final class CommitTracker implements Runnable {
public void scheduleCommitWithin(long commitMaxTime) {
_scheduleCommitWithin(commitMaxTime);
}
private void _scheduleCommitWithinIfNeeded(long commitWithin) {
long ctime = (commitWithin > 0) ? commitWithin : timeUpperBound;
if (ctime > 0) {
_scheduleCommitWithin(ctime);
}
}
private void _scheduleCommitWithin(long commitMaxTime) {
if (commitMaxTime <= 0) return;
@ -139,11 +147,14 @@ final class CommitTracker implements Runnable {
}
// maxTime-triggered autoCommit
long ctime = (commitWithin > 0) ? commitWithin : timeUpperBound;
if (ctime > 0) {
_scheduleCommitWithin(ctime);
}
_scheduleCommitWithinIfNeeded(commitWithin);
}
/**
* Indicate that documents have been deleted
*/
public void deletedDocument( int commitWithin ) {
_scheduleCommitWithinIfNeeded(commitWithin);
}
/** Inform tracker that a commit has occurred */

View File

@ -18,7 +18,6 @@
package org.apache.solr.update;
import org.apache.lucene.util.BytesRef;
import org.apache.solr.common.SolrInputField;
import org.apache.solr.request.SolrQueryRequest;
import org.apache.solr.schema.IndexSchema;
import org.apache.solr.schema.SchemaField;
@ -30,6 +29,7 @@ public class DeleteUpdateCommand extends UpdateCommand {
public String id; // external (printable) id, for delete-by-id
public String query; // query string for delete-by-query
private BytesRef indexedId;
public int commitWithin = -1;
public DeleteUpdateCommand(SolrQueryRequest req) {
@ -62,6 +62,7 @@ public class DeleteUpdateCommand extends UpdateCommand {
sb.append(':');
if (id!=null) sb.append("id=").append(id);
else sb.append("query=`").append(query).append('`');
sb.append(",commitWithin=").append(commitWithin);
return sb.toString();
}
}

View File

@ -203,6 +203,7 @@ public class DirectUpdateHandler2 extends UpdateHandler {
Term deleteTerm = new Term(idField.getName(), cmd.getIndexedId());
// SolrCore.verbose("deleteDocuments",deleteTerm,writer);
commitTracker.deletedDocument( cmd.commitWithin );
writer.deleteDocuments(deleteTerm);
// SolrCore.verbose("deleteDocuments",deleteTerm,"DONE");
@ -234,6 +235,8 @@ public class DirectUpdateHandler2 extends UpdateHandler {
boolean delAll = MatchAllDocsQuery.class == q.getClass();
commitTracker.deletedDocument(cmd.commitWithin);
if (delAll) {
deleteAll();
} else {

View File

@ -68,7 +68,9 @@ public class JsonLoaderTest extends SolrTestCaseJ4 {
"'optimize': { 'waitSearcher':false },\n" +
"\n" +
"'delete': { 'id':'ID' },\n" +
"'delete': { 'id':'ID', 'commitWithin':'500' },\n" +
"'delete': { 'query':'QUERY' },\n" +
"'delete': { 'query':'QUERY', 'commitWithin':'500' },\n" +
"'rollback': {}\n" +
"\n" +
"}\n" +
@ -113,14 +115,26 @@ public class JsonLoaderTest extends SolrTestCaseJ4 {
// DELETE COMMANDS
assertEquals( 2, p.deleteCommands.size() );
assertEquals( 4, p.deleteCommands.size() );
DeleteUpdateCommand delete = p.deleteCommands.get( 0 );
assertEquals( delete.id, "ID" );
assertEquals( delete.query, null );
assertEquals( delete.commitWithin, -1);
delete = p.deleteCommands.get( 1 );
assertEquals( delete.id, "ID" );
assertEquals( delete.query, null );
assertEquals( delete.commitWithin, 500);
delete = p.deleteCommands.get( 2 );
assertEquals( delete.id, null );
assertEquals( delete.query, "QUERY" );
assertEquals( delete.commitWithin, -1);
delete = p.deleteCommands.get( 3 );
assertEquals( delete.id, null );
assertEquals( delete.query, "QUERY" );
assertEquals( delete.commitWithin, 500);
// ROLLBACK COMMANDS
assertEquals( 1, p.rollbackCommands.size() );

View File

@ -17,21 +17,27 @@
package org.apache.solr.handler;
import org.apache.solr.SolrTestCaseJ4;
import java.io.StringReader;
import java.util.Collection;
import javax.xml.stream.XMLInputFactory;
import javax.xml.stream.XMLStreamReader;
import org.apache.commons.lang.ObjectUtils;
import org.apache.solr.common.SolrInputDocument;
import org.apache.solr.common.util.ContentStreamBase;
import org.apache.solr.request.SolrQueryRequest;
import org.apache.solr.response.SolrQueryResponse;
import org.apache.solr.update.AddUpdateCommand;
import org.apache.solr.update.DeleteUpdateCommand;
import org.apache.solr.update.processor.BufferingRequestProcessor;
import org.apache.solr.update.processor.UpdateRequestProcessor;
import org.junit.BeforeClass;
import org.junit.Test;
import java.io.IOException;
import java.io.StringReader;
import java.util.Collection;
import java.util.LinkedList;
import java.util.Queue;
import javax.xml.stream.XMLInputFactory;
import javax.xml.stream.XMLStreamReader;
public class XmlUpdateRequestHandlerTest extends SolrTestCaseJ4 {
private static XMLInputFactory inputFactory = XMLInputFactory.newInstance();
protected static XmlUpdateRequestHandler handler;
@ -102,5 +108,68 @@ public class XmlUpdateRequestHandlerTest extends SolrTestCaseJ4 {
assertEquals(false, add.overwrite);
req.close();
}
@Test
public void testReadDelete() throws Exception {
String xml =
"<update>" +
" <delete>" +
" <query>id:150</query>" +
" <id>150</id>" +
" <id>200</id>" +
" <query>id:200</query>" +
" </delete>" +
" <delete commitWithin=\"500\">" +
" <query>id:150</query>" +
" </delete>" +
" <delete>" +
" <id>150</id>" +
" </delete>" +
"</update>";
MockUpdateRequestProcessor p = new MockUpdateRequestProcessor(null);
p.expectDelete(null, "id:150", -1);
p.expectDelete("150", null, -1);
p.expectDelete("200", null, -1);
p.expectDelete(null, "id:200", -1);
p.expectDelete(null, "id:150", 500);
p.expectDelete("150", null, -1);
XMLLoader loader = new XMLLoader(p, inputFactory);
loader.load(req(), new SolrQueryResponse(), new ContentStreamBase.StringStream(xml));
p.assertNoCommandsPending();
}
private class MockUpdateRequestProcessor extends UpdateRequestProcessor {
private Queue<DeleteUpdateCommand> deleteCommands = new LinkedList<DeleteUpdateCommand>();
public MockUpdateRequestProcessor(UpdateRequestProcessor next) {
super(next);
}
public void expectDelete(String id, String query, int commitWithin) {
DeleteUpdateCommand cmd = new DeleteUpdateCommand(null);
cmd.id = id;
cmd.query = query;
cmd.commitWithin = commitWithin;
deleteCommands.add(cmd);
}
public void assertNoCommandsPending() {
assertTrue(deleteCommands.isEmpty());
}
@Override
public void processDelete(DeleteUpdateCommand cmd) throws IOException {
DeleteUpdateCommand expected = deleteCommands.poll();
assertNotNull("Unexpected delete command: [" + cmd + "]", expected);
assertTrue("Expected [" + expected + "] but found [" + cmd + "]",
ObjectUtils.equals(expected.id, cmd.id) &&
ObjectUtils.equals(expected.query, cmd.query) &&
expected.commitWithin==cmd.commitWithin);
}
}
}

View File

@ -258,5 +258,85 @@ public class AutoCommitTest extends AbstractSolrTestCase {
assertQ("now it should", req("id:500") ,"//result[@numFound=1]" );
}
public void testCommitWithin() throws Exception {
SolrCore core = h.getCore();
NewSearcherListener trigger = new NewSearcherListener();
core.registerNewSearcherListener(trigger);
DirectUpdateHandler2 updater = (DirectUpdateHandler2) core.getUpdateHandler();
CommitTracker tracker = updater.commitTracker;
tracker.setTimeUpperBound(0);
tracker.setDocsUpperBound(-1);
XmlUpdateRequestHandler handler = new XmlUpdateRequestHandler();
handler.init( null );
MapSolrParams params = new MapSolrParams( new HashMap<String, String>() );
// Add a single document with commitWithin == 1 second
SolrQueryResponse rsp = new SolrQueryResponse();
SolrQueryRequestBase req = new SolrQueryRequestBase( core, params ) {};
req.setContentStreams( toContentStreams(
adoc(1000, "id", "529", "field_t", "what's inside?", "subject", "info"), null ) );
trigger.reset();
handler.handleRequest( req, rsp );
// Check it isn't in the index
assertQ("shouldn't find any", req("id:529") ,"//result[@numFound=0]" );
// Wait longer than the commitWithin time
assertTrue("commitWithin failed to commit", trigger.waitForNewSearcher(30000));
// Add one document without commitWithin
req.setContentStreams( toContentStreams(
adoc("id", "530", "field_t", "what's inside?", "subject", "info"), null ) );
trigger.reset();
handler.handleRequest( req, rsp );
// Check it isn't in the index
assertQ("shouldn't find any", req("id:530") ,"//result[@numFound=0]" );
// Delete one document with commitWithin
req.setContentStreams( toContentStreams(
delI("529", "commitWithin", "1000"), null ) );
trigger.reset();
handler.handleRequest( req, rsp );
// Now make sure we can find it
assertQ("should find one", req("id:529") ,"//result[@numFound=1]" );
// Wait for the commit to happen
assertTrue("commitWithin failed to commit", trigger.waitForNewSearcher(30000));
// Now we shouldn't find it
assertQ("should find none", req("id:529") ,"//result[@numFound=0]" );
// ... but we should find the new one
assertQ("should find one", req("id:530") ,"//result[@numFound=1]" );
trigger.reset();
// now make the call 10 times really fast and make sure it
// only commits once
req.setContentStreams( toContentStreams(
adoc(1000, "id", "500" ), null ) );
for( int i=0;i<10; i++ ) {
handler.handleRequest( req, rsp );
}
assertQ("should not be there yet", req("id:500") ,"//result[@numFound=0]" );
// the same for the delete
req.setContentStreams( toContentStreams(
delI("530", "commitWithin", "1000"), null ) );
for( int i=0;i<10; i++ ) {
handler.handleRequest( req, rsp );
}
assertQ("should be there", req("id:530") ,"//result[@numFound=1]" );
assertTrue("commitWithin failed to commit", trigger.waitForNewSearcher(30000));
assertQ("should be there", req("id:500") ,"//result[@numFound=1]" );
assertQ("should not be there", req("id:530") ,"//result[@numFound=0]" );
assertEquals(3, tracker.getCommitCount());
}
}

View File

@ -226,7 +226,22 @@ public abstract class SolrServer implements Serializable
* @throws IOException
*/
public UpdateResponse deleteById(String id) throws SolrServerException, IOException {
return new UpdateRequest().deleteById( id ).process( this );
return deleteById(id, -1);
}
/**
* Deletes a single document by unique ID, specifying max time before commit
* @param id the ID of the document to delete
* @param commitWithinMs max time (in ms) before a commit will happen
* @throws SolrServerException
* @throws IOException
* @since 3.6
*/
public UpdateResponse deleteById(String id, int commitWithinMs) throws SolrServerException, IOException {
UpdateRequest req = new UpdateRequest();
req.deleteById(id);
req.setCommitWithin(commitWithinMs);
return req.process(this);
}
/**
@ -236,7 +251,22 @@ public abstract class SolrServer implements Serializable
* @throws IOException
*/
public UpdateResponse deleteById(List<String> ids) throws SolrServerException, IOException {
return new UpdateRequest().deleteById( ids ).process( this );
return deleteById(ids, -1);
}
/**
* Deletes a list of documents by unique ID, specifying max time before commit
* @param ids the list of document IDs to delete
* @param commitWithinMs max time (in ms) before a commit will happen
* @throws SolrServerException
* @throws IOException
* @since 3.6
*/
public UpdateResponse deleteById(List<String> ids, int commitWithinMs) throws SolrServerException, IOException {
UpdateRequest req = new UpdateRequest();
req.deleteById(ids);
req.setCommitWithin(commitWithinMs);
return req.process(this);
}
/**
@ -246,7 +276,22 @@ public abstract class SolrServer implements Serializable
* @throws IOException
*/
public UpdateResponse deleteByQuery(String query) throws SolrServerException, IOException {
return new UpdateRequest().deleteByQuery( query ).process( this );
return deleteByQuery(query, -1);
}
/**
* Deletes documents from the index based on a query, specifying max time before commit
* @param query the query expressing what documents to delete
* @param commitWithinMs max time (in ms) before a commit will happen
* @throws SolrServerException
* @throws IOException
* @since 3.6
*/
public UpdateResponse deleteByQuery(String query, int commitWithinMs) throws SolrServerException, IOException {
UpdateRequest req = new UpdateRequest();
req.deleteByQuery(query);
req.setCommitWithin(commitWithinMs);
return req.process(this);
}
/**

View File

@ -145,6 +145,15 @@ public class JavaBinUpdateRequestCodec {
codec.unmarshal(is);
// NOTE: if the update request contains only delete commands the params
// must be loaded now
if(updateRequest.getParams()==null) {
NamedList params = (NamedList) namedList[0].get("params");
if(params!=null) {
updateRequest.setParams(new ModifiableSolrParams(SolrParams.toSolrParams(params)));
}
}
delById = (List<String>) namedList[0].get("delById");
delByQ = (List<String>) namedList[0].get("delByQ");
doclist = (List) namedList[0].get("docs");

View File

@ -228,7 +228,11 @@ public class UpdateRequest extends AbstractUpdateRequest {
boolean deleteI = deleteById != null && deleteById.size() > 0;
boolean deleteQ = deleteQuery != null && deleteQuery.size() > 0;
if( deleteI || deleteQ ) {
writer.append( "<delete>" );
if(commitWithin>0) {
writer.append( "<delete commitWithin=\"" + commitWithin + "\">" );
} else {
writer.append( "<delete>" );
}
if( deleteI ) {
for( String id : deleteById ) {
writer.append( "<id>" );

View File

@ -343,7 +343,7 @@ abstract public class SolrExampleTests extends SolrJettyTestBase
* query the example
*/
@Test
public void testCommitWithin() throws Exception
public void testCommitWithinOnAdd() throws Exception
{
// make sure it is empty...
SolrServer server = getSolrServer();
@ -388,7 +388,6 @@ abstract public class SolrExampleTests extends SolrJettyTestBase
Assert.assertEquals( 1, rsp.getResults().getNumFound() );
// Now test the new convenience parameter on the add() for commitWithin
SolrInputDocument doc4 = new SolrInputDocument();
doc4.addField( "id", "id4", 1.0f );
@ -416,7 +415,52 @@ abstract public class SolrExampleTests extends SolrJettyTestBase
}
Assert.assertEquals( 1, rsp.getResults().getNumFound() );
}
@Test
public void testCommitWithinOnDelete() throws Exception
{
// make sure it is empty...
SolrServer server = getSolrServer();
server.deleteByQuery( "*:*" );// delete everything!
server.commit();
QueryResponse rsp = server.query( new SolrQuery( "*:*") );
Assert.assertEquals( 0, rsp.getResults().getNumFound() );
// Now add one document...
SolrInputDocument doc3 = new SolrInputDocument();
doc3.addField( "id", "id3", 1.0f );
doc3.addField( "name", "doc3", 1.0f );
doc3.addField( "price", 10 );
server.add(doc3);
server.commit();
// now check that it comes out...
rsp = server.query( new SolrQuery( "id:id3") );
Assert.assertEquals( 1, rsp.getResults().getNumFound() );
// now test commitWithin on a delete
UpdateRequest up = new UpdateRequest();
up.setCommitWithin(1000);
up.deleteById("id3");
up.process( server );
// the document should still be there
rsp = server.query( new SolrQuery( "id:id3") );
Assert.assertEquals( 1, rsp.getResults().getNumFound() );
// check if the doc has been deleted every 250 ms for 30 seconds
long timeout = System.currentTimeMillis() + 30000;
do {
Thread.sleep( 250 ); // wait 250 ms
rsp = server.query( new SolrQuery( "id:id3") );
if(rsp.getResults().getNumFound()==0) {
return;
}
} while(System.currentTimeMillis()<timeout);
Assert.fail("commitWithin failed to commit");
}

View File

@ -316,6 +316,20 @@ public abstract class AbstractSolrTestCase extends LuceneTestCase {
Doc d = doc(fieldsAndValues);
return add(d);
}
/**
* Generates a simple &lt;add&gt;&lt;doc&gt;... XML String with the
* commitWithin attribute.
*
* @param commitWithin the value of the commitWithin attribute
* @param fieldsAndValues 0th and Even numbered args are fields names odds are field values.
* @see #add
* @see #doc
*/
public String adoc(int commitWithin, String... fieldsAndValues) {
Doc d = doc(fieldsAndValues);
return add(d, "commitWithin", String.valueOf(commitWithin));
}
/**
* Generates a simple &lt;add&gt;&lt;doc&gt;... XML String with no options
@ -366,16 +380,17 @@ public abstract class AbstractSolrTestCase extends LuceneTestCase {
*
* @see TestHarness#deleteById
*/
public String delI(String id) {
return h.deleteById(id);
public String delI(String id, String... args) {
return h.deleteById(id, args);
}
/**
* Generates a &lt;delete&gt;... XML string for an query
*
* @see TestHarness#deleteByQuery
*/
public String delQ(String q) {
return h.deleteByQuery(q);
public String delQ(String q, String... args) {
return h.deleteByQuery(q, args);
}
/**

View File

@ -453,32 +453,46 @@ public class TestHarness {
/**
* Generates a delete by query xml string
* @param q Query that has not already been xml escaped
* @param args The attributes of the delete tag
*/
public static String deleteByQuery(String q) {
return delete("query", q);
public static String deleteByQuery(String q, String... args) {
try {
StringWriter r = new StringWriter();
XML.writeXML(r, "query", q);
return delete(r.getBuffer().toString(), args);
} catch(IOException e) {
throw new RuntimeException
("this should never happen with a StringWriter", e);
}
}
/**
* Generates a delete by id xml string
* @param id ID that has not already been xml escaped
* @param args The attributes of the delete tag
*/
public static String deleteById(String id) {
return delete("id", id);
public static String deleteById(String id, String... args) {
try {
StringWriter r = new StringWriter();
XML.writeXML(r, "id", id);
return delete(r.getBuffer().toString(), args);
} catch(IOException e) {
throw new RuntimeException
("this should never happen with a StringWriter", e);
}
}
/**
* Generates a delete xml string
* @param val text that has not already been xml escaped
* @param args 0 and Even numbered args are params, Odd numbered args are XML escaped values.
*/
private static String delete(String deltype, String val) {
private static String delete(String val, String... args) {
try {
StringWriter r = new StringWriter();
r.write("<delete>");
XML.writeXML(r, deltype, val);
r.write("</delete>");
XML.writeUnescapedXML(r, "delete", val, (Object[])args);
return r.getBuffer().toString();
} catch (IOException e) {
} catch(IOException e) {
throw new RuntimeException
("this should never happen with a StringWriter", e);
}