From 5cd69d47e1613fde7dfd5dbb8309370a52297ee0 Mon Sep 17 00:00:00 2001 From: Erik Hatcher Date: Fri, 25 Jul 2014 12:19:48 +0000 Subject: [PATCH] SOLR-3622, SOLR-5847, SOLR-6194, SOLR-6269: Several DIH fixes/improvements git-svn-id: https://svn.apache.org/repos/asf/lucene/dev/trunk@1613406 13f79535-47bb-0310-9956-ffa450edef68 --- solr/CHANGES.txt | 11 ++++++ .../solr/handler/dataimport/ContextImpl.java | 2 + .../solr/handler/dataimport/DataImporter.java | 30 +++++++-------- .../solr/handler/dataimport/DocBuilder.java | 37 ++++++++++++------- .../solr/handler/dataimport/SolrWriter.java | 2 +- .../dataimport/config/DIHConfiguration.java | 8 ++-- .../handler/dataimport/TestDocBuilder2.java | 19 ++++++---- solr/webapp/web/js/scripts/dataimport.js | 2 +- 8 files changed, 69 insertions(+), 42 deletions(-) diff --git a/solr/CHANGES.txt b/solr/CHANGES.txt index 1327af16194..775833ab28e 100644 --- a/solr/CHANGES.txt +++ b/solr/CHANGES.txt @@ -222,6 +222,8 @@ Bug Fixes datatypes of options people can specify, additional error handling of duplicated/unidentified options has also been added. (Maciej Zasada, hossman) +* SOLR-5847: Fixed data import abort button in admin UI. (ehatcher) + Optimizations --------------------- @@ -289,6 +291,15 @@ Other Changes * SOLR-6274: UpdateShardHandler should log the params used to configure it's HttpClient. (Ramkumar Aiyengar via Mark Miller) +* SOLR-6194: Opened up "public" access to DataSource, DocBuilder, and EntityProcessorWrapper + in DIH. (Aaron LaBella via ehatcher) + +* SOLR-6269: Renamed "rollback" to "error" in DIH internals, including renaming onRollback + to onError introduced in SOLR-6258. (ehatcher) + +* SOLR-3622: When using DIH in SolrCloud-mode, rollback will no longer be called when + an error occurs. (ehatcher) + ================== 4.9.0 ================== Versions of Major Components diff --git a/solr/contrib/dataimporthandler/src/java/org/apache/solr/handler/dataimport/ContextImpl.java b/solr/contrib/dataimporthandler/src/java/org/apache/solr/handler/dataimport/ContextImpl.java index b233b7b12c1..051da0b0625 100644 --- a/solr/contrib/dataimporthandler/src/java/org/apache/solr/handler/dataimport/ContextImpl.java +++ b/solr/contrib/dataimporthandler/src/java/org/apache/solr/handler/dataimport/ContextImpl.java @@ -54,6 +54,8 @@ public class ContextImpl extends Context { DocBuilder docBuilder; + Exception lastException = null; + public ContextImpl(EntityProcessorWrapper epw, VariableResolver resolver, DataSource ds, String currProcess, diff --git a/solr/contrib/dataimporthandler/src/java/org/apache/solr/handler/dataimport/DataImporter.java b/solr/contrib/dataimporthandler/src/java/org/apache/solr/handler/dataimport/DataImporter.java index 31a6cf2802a..370ec5ee3a0 100644 --- a/solr/contrib/dataimporthandler/src/java/org/apache/solr/handler/dataimport/DataImporter.java +++ b/solr/contrib/dataimporthandler/src/java/org/apache/solr/handler/dataimport/DataImporter.java @@ -179,7 +179,11 @@ public class DataImporter { * Used by tests */ public void loadAndInit(String configStr) { - config = loadDataConfig(new InputSource(new StringReader(configStr))); + config = loadDataConfig(new InputSource(new StringReader(configStr))); + } + + public void loadAndInit(InputSource configFile) { + config = loadDataConfig(configFile); } public DIHConfiguration loadDataConfig(InputSource configFile) { @@ -349,7 +353,7 @@ public class DataImporter { return store.get(key); } - DataSource getDataSourceInstance(Entity key, String name, Context ctx) { + public DataSource getDataSourceInstance(Entity key, String name, Context ctx) { Map p = requestLevelDataSourceProps.get(name); if (p == null) p = config.getDataSources().get(name); @@ -404,7 +408,6 @@ public class DataImporter { public void doFullImport(DIHWriter writer, RequestInfo requestParams) { LOG.info("Starting Full Import"); setStatus(Status.RUNNING_FULL_DUMP); - boolean success = false; try { DIHProperties dihPropWriter = createPropertyWriter(); setIndexStartTime(dihPropWriter.getCurrentTimestamp()); @@ -413,14 +416,10 @@ public class DataImporter { docBuilder.execute(); if (!requestParams.isDebug()) cumulativeStatistics.add(docBuilder.importStatistics); - success = true; } catch (Exception e) { SolrException.log(LOG, "Full Import failed", e); + docBuilder.handleError("Full Import failed", e); } finally { - if (!success) { - docBuilder.rollback(); - } - setStatus(Status.IDLE); DocBuilder.INSTANCE.set(null); } @@ -437,7 +436,6 @@ public class DataImporter { public void doDeltaImport(DIHWriter writer, RequestInfo requestParams) { LOG.info("Starting Delta Import"); setStatus(Status.RUNNING_DELTA_DUMP); - boolean success = false; try { DIHProperties dihPropWriter = createPropertyWriter(); setIndexStartTime(dihPropWriter.getCurrentTimestamp()); @@ -446,13 +444,10 @@ public class DataImporter { docBuilder.execute(); if (!requestParams.isDebug()) cumulativeStatistics.add(docBuilder.importStatistics); - success = true; } catch (Exception e) { LOG.error("Delta Import Failed", e); + docBuilder.handleError("Delta Import Failed", e); } finally { - if (!success) { - docBuilder.rollback(); - } setStatus(Status.IDLE); DocBuilder.INSTANCE.set(null); } @@ -510,10 +505,15 @@ public class DataImporter { } - DocBuilder getDocBuilder() { + public DocBuilder getDocBuilder() { return docBuilder; } - + + public DocBuilder getDocBuilder(DIHWriter writer, RequestInfo requestParams) { + DIHProperties dihPropWriter = createPropertyWriter(); + return new DocBuilder(this, writer, dihPropWriter, requestParams); + } + Map getEvaluators() { return getEvaluators(config.getFunctions()); } diff --git a/solr/contrib/dataimporthandler/src/java/org/apache/solr/handler/dataimport/DocBuilder.java b/solr/contrib/dataimporthandler/src/java/org/apache/solr/handler/dataimport/DocBuilder.java index 4a5276fa998..0ff9982488a 100644 --- a/solr/contrib/dataimporthandler/src/java/org/apache/solr/handler/dataimport/DocBuilder.java +++ b/solr/contrib/dataimporthandler/src/java/org/apache/solr/handler/dataimport/DocBuilder.java @@ -94,7 +94,9 @@ public class DocBuilder { writer = solrWriter; ContextImpl ctx = new ContextImpl(null, null, null, null, reqParams.getRawParams(), null, this); - writer.init(ctx); + if (writer != null) { + writer.init(ctx); + } } @@ -146,25 +148,31 @@ public class DocBuilder { return null; } } - private void invokeEventListener(String className) { + invokeEventListener(className, null); + } + + + private void invokeEventListener(String className, Exception lastException) { try { EventListener listener = (EventListener) loadClass(className, dataImporter.getCore()).newInstance(); - notifyListener(listener); + notifyListener(listener, lastException); } catch (Exception e) { wrapAndThrow(SEVERE, e, "Unable to load class : " + className); } } - private void notifyListener(EventListener listener) { + private void notifyListener(EventListener listener, Exception lastException) { String currentProcess; if (dataImporter.getStatus() == DataImporter.Status.RUNNING_DELTA_DUMP) { currentProcess = Context.DELTA_DUMP; } else { currentProcess = Context.FULL_DUMP; } - listener.onEvent(new ContextImpl(null, getVariableResolver(), null, currentProcess, session, null, this)); + ContextImpl ctx = new ContextImpl(null, getVariableResolver(), null, currentProcess, session, null, this); + ctx.lastException = lastException; + listener.onEvent(ctx); } @SuppressWarnings("unchecked") @@ -234,7 +242,7 @@ public class DocBuilder { if (stop.get()) { // Dont commit if aborted using command=abort statusMessages.put("Aborted", new SimpleDateFormat("yyyy-MM-dd HH:mm:ss", Locale.ROOT).format(new Date())); - rollback(); + handleError("Aborted", null); } else { // Do not commit unnecessarily if this is a delta-import and no documents were created or deleted if (!reqParams.isClean()) { @@ -305,12 +313,15 @@ public class DocBuilder { } } - void rollback() { - writer.rollback(); - statusMessages.put("", "Indexing failed. Rolled back all changes."); - addStatusMessage("Rolledback"); - if ((config != null) && (config.getOnRollback() != null)) { - invokeEventListener(config.getOnRollback()); + void handleError(String message, Exception e) { + if (!dataImporter.getCore().getCoreDescriptor().getCoreContainer().isZooKeeperAware()) { + writer.rollback(); + } + + statusMessages.put(message, "Indexing error"); + addStatusMessage(message); + if ((config != null) && (config.getOnError() != null)) { + invokeEventListener(config.getOnError(), e); } } @@ -688,7 +699,7 @@ public class DocBuilder { } } - private EntityProcessorWrapper getEntityProcessorWrapper(Entity entity) { + public EntityProcessorWrapper getEntityProcessorWrapper(Entity entity) { EntityProcessor entityProcessor = null; if (entity.getProcessorName() == null) { entityProcessor = new SqlEntityProcessor(); diff --git a/solr/contrib/dataimporthandler/src/java/org/apache/solr/handler/dataimport/SolrWriter.java b/solr/contrib/dataimporthandler/src/java/org/apache/solr/handler/dataimport/SolrWriter.java index 61cbaa3e20b..4a5dbb016c9 100644 --- a/solr/contrib/dataimporthandler/src/java/org/apache/solr/handler/dataimport/SolrWriter.java +++ b/solr/contrib/dataimporthandler/src/java/org/apache/solr/handler/dataimport/SolrWriter.java @@ -117,7 +117,7 @@ public class SolrWriter extends DIHWriterBase implements DIHWriter { RollbackUpdateCommand rollback = new RollbackUpdateCommand(req); processor.processRollback(rollback); } catch (Exception e) { - log.error("Exception while solr rollback.", e); + log.error("Exception during rollback command.", e); } } diff --git a/solr/contrib/dataimporthandler/src/java/org/apache/solr/handler/dataimport/config/DIHConfiguration.java b/solr/contrib/dataimporthandler/src/java/org/apache/solr/handler/dataimport/config/DIHConfiguration.java index a623d90a51f..ee668bca43d 100644 --- a/solr/contrib/dataimporthandler/src/java/org/apache/solr/handler/dataimport/config/DIHConfiguration.java +++ b/solr/contrib/dataimporthandler/src/java/org/apache/solr/handler/dataimport/config/DIHConfiguration.java @@ -56,7 +56,7 @@ public class DIHConfiguration { private final List entities; private final String onImportStart; private final String onImportEnd; - private final String onRollback; + private final String onError; private final List> functions; private final Script script; private final Map> dataSources; @@ -72,7 +72,7 @@ public class DIHConfiguration { this.deleteQuery = ConfigParseUtil.getStringAttribute(element, "deleteQuery", null); this.onImportStart = ConfigParseUtil.getStringAttribute(element, "onImportStart", null); this.onImportEnd = ConfigParseUtil.getStringAttribute(element, "onImportEnd", null); - this.onRollback = ConfigParseUtil.getStringAttribute(element, "onRollback", null); + this.onError = ConfigParseUtil.getStringAttribute(element, "onError", null); List modEntities = new ArrayList<>(); List l = ConfigParseUtil.getChildNodes(element, "entity"); boolean docRootFound = false; @@ -165,8 +165,8 @@ public class DIHConfiguration { public String getOnImportEnd() { return onImportEnd; } - public String getOnRollback() { - return onRollback; + public String getOnError() { + return onError; } public List> getFunctions() { return functions; diff --git a/solr/contrib/dataimporthandler/src/test/org/apache/solr/handler/dataimport/TestDocBuilder2.java b/solr/contrib/dataimporthandler/src/test/org/apache/solr/handler/dataimport/TestDocBuilder2.java index 59a6e35b997..2efc90c664a 100644 --- a/solr/contrib/dataimporthandler/src/test/org/apache/solr/handler/dataimport/TestDocBuilder2.java +++ b/solr/contrib/dataimporthandler/src/test/org/apache/solr/handler/dataimport/TestDocBuilder2.java @@ -76,14 +76,15 @@ public class TestDocBuilder2 extends AbstractDataImportHandlerTestCase { } @Test - public void testRollbackHandler() throws Exception { + public void testErrorHandler() throws Exception { List rows = new ArrayList(); - rows.add(createMap("id", "1", "FORCE_ROLLBACK", "true")); + rows.add(createMap("id", "1", "FORCE_ERROR", "true")); MockDataSource.setIterator("select * from x", rows.iterator()); - runFullImport(dataConfigWithRollbackHandler); + runFullImport(dataConfigWithErrorHandler); - assertTrue("Rollback event listener was not called", RollbackEventListener.executed); + assertTrue("Error event listener was not called", ErrorEventListener.executed); + assertTrue(ErrorEventListener.lastException.getMessage().contains("ForcedException")); } @Test @@ -316,12 +317,14 @@ public class TestDocBuilder2 extends AbstractDataImportHandlerTestCase { } } - public static class RollbackEventListener implements EventListener { + public static class ErrorEventListener implements EventListener { public static boolean executed = false; + public static Exception lastException = null; @Override public void onEvent(Context ctx) { executed = true; + lastException = ((ContextImpl) ctx).lastException; } } @@ -377,11 +380,11 @@ public class TestDocBuilder2 extends AbstractDataImportHandlerTestCase { " \n" + ""; - private final String dataConfigWithRollbackHandler = " \n" + - " \n" + + private final String dataConfigWithErrorHandler = " \n" + + " \n" + " \n" + " \n" + - " \n" + + " \n" + " \n" + " \n" + ""; diff --git a/solr/webapp/web/js/scripts/dataimport.js b/solr/webapp/web/js/scripts/dataimport.js index 28beb0aaac7..10244f32114 100644 --- a/solr/webapp/web/js/scripts/dataimport.js +++ b/solr/webapp/web/js/scripts/dataimport.js @@ -350,7 +350,7 @@ sammy.get { url : handler_url + '?command=abort&wt=json', dataType : 'json', - type: 'POST', + type: 'GET', context: $( this ), beforeSend : function( xhr, settings ) {