Rajitha, Seeing review request and commit at the same time. Was this by mistake?
Thanks On Tue, Jul 10, 2018 at 12:55 PM, <[email protected]> wrote: > Repository: lens > Updated Branches: > refs/heads/master 278a02473 -> 4a24fb94f > > > LENS-1371 : Add a substring based retry policy for failures > > > Project: http://git-wip-us.apache.org/repos/asf/lens/repo > Commit: http://git-wip-us.apache.org/repos/asf/lens/commit/4a24fb94 > Tree: http://git-wip-us.apache.org/repos/asf/lens/tree/4a24fb94 > Diff: http://git-wip-us.apache.org/repos/asf/lens/diff/4a24fb94 > > Branch: refs/heads/master > Commit: 4a24fb94f81554f6de2eec5f0cc86fe4e80c1ac9 > Parents: 278a024 > Author: Rajitha R <[email protected]> > Authored: Tue Jul 10 12:55:20 2018 +0530 > Committer: Rajitha.R <[email protected]> > Committed: Tue Jul 10 12:55:20 2018 +0530 > > ---------------------------------------------------------------------- > .../apache/lens/cube/parse/TestVirtualFactQueries.java | 8 ++++---- > .../src/main/resources/hivedriver-default.xml | 12 ++++++++++++ > .../java/org/apache/lens/driver/jdbc/JDBCDriver.java | 9 +++++++-- > .../src/main/resources/jdbcdriver-default.xml | 12 ++++++++++++ > .../org/apache/lens/driver/jdbc/TestJdbcDriver.java | 8 -------- > .../org/apache/lens/server/api/LensConfConstants.java | 5 +++++ > .../lens/server/api/retry/BackOffRetryHandler.java | 5 ++++- > .../retry/FibonacciExponentialBackOffRetryHandler.java | 4 ++++ > .../lens/server/api/retry/ImmediateRetryHandler.java | 6 +++++- > .../apache/lens/server/api/retry/NoRetryHandler.java | 5 +++++ > .../lens/server/query/QueryExecutionServiceImpl.java | 11 +++++++++-- > src/site/apt/admin/hivedriver-config.apt | 4 ++++ > src/site/apt/admin/jdbcdriver-config.apt | 4 ++++ > 13 files changed, 75 insertions(+), 18 deletions(-) > ---------------------------------------------------------------------- > > > http://git-wip-us.apache.org/repos/asf/lens/blob/4a24fb94/ > lens-cube/src/test/java/org/apache/lens/cube/parse/ > TestVirtualFactQueries.java > ---------------------------------------------------------------------- > diff --git > a/lens-cube/src/test/java/org/apache/lens/cube/parse/TestVirtualFactQueries.java > b/lens-cube/src/test/java/org/apache/lens/cube/parse/ > TestVirtualFactQueries.java > index e2f1074..81a8972 100644 > --- a/lens-cube/src/test/java/org/apache/lens/cube/parse/ > TestVirtualFactQueries.java > +++ b/lens-cube/src/test/java/org/apache/lens/cube/parse/ > TestVirtualFactQueries.java > @@ -71,10 +71,10 @@ public class TestVirtualFactQueries extends > TestQueryRewrite { > @Test > public void testVirtualFactColStartTimeQuery() { > try { > - rewriteCtx("select dim1,SUM(msr1) from virtualcube where " + > TWO_DAYS_RANGE, getConfWithStorages("C1")); > - Assert.fail("Should not come here..Column Start time feature is > failing.."); > + rewriteCtx("select dim1,SUM(msr1) from virtualcube where " + > TWO_DAYS_RANGE, getConfWithStorages("C1")); > + Assert.fail("Should not come here..Column Start time feature is > failing.."); > }catch (LensException e) { > - if(e.getErrorCode() == 3024) { > + if (e.getErrorCode() == 3024) { > System.out.println("Expected flow :" + e.getMessage()); > }else { > Assert.fail("Exception not as expected"); > @@ -88,7 +88,7 @@ public class TestVirtualFactQueries extends > TestQueryRewrite { > rewriteCtx("select dim2,SUM(msr1) from virtualcube where " + > TWO_DAYS_RANGE, getConfWithStorages("C1")); > Assert.fail("Should not come here..Column End time feature is > failing.."); > }catch (LensException e) { > - if(e.getErrorCode() == 3024) { > + if (e.getErrorCode() == 3024) { > System.out.println("Expected flow :" + e.getMessage()); > }else { > Assert.fail("Exception not as expected"); > > http://git-wip-us.apache.org/repos/asf/lens/blob/4a24fb94/ > lens-driver-hive/src/main/resources/hivedriver-default.xml > ---------------------------------------------------------------------- > diff --git a/lens-driver-hive/src/main/resources/hivedriver-default.xml > b/lens-driver-hive/src/main/resources/hivedriver-default.xml > index d41e36a..2776123 100644 > --- a/lens-driver-hive/src/main/resources/hivedriver-default.xml > +++ b/lens-driver-hive/src/main/resources/hivedriver-default.xml > @@ -185,4 +185,16 @@ > <description>Cost based Query type mapping</description> > </property> > > + <property> > + <name>retry.messages.contains.map</name> > + <value>Session handle not found=org.apache.lens.server.api.retry. > ImmediateRetryHandler(2)</value> > + <description>Comma separated error messages and retry > policy</description> > + </property> > + > + <property> > + <name>query.retry.policy.classes</name> > + <value>org.apache.lens.server.api.retry. > SubstringMessagePolicyDecider</value> > + <description>List of policy decider classes</description> > + </property> > + > </configuration> > > http://git-wip-us.apache.org/repos/asf/lens/blob/4a24fb94/ > lens-driver-jdbc/src/main/java/org/apache/lens/driver/jdbc/JDBCDriver.java > ---------------------------------------------------------------------- > diff --git > a/lens-driver-jdbc/src/main/java/org/apache/lens/driver/jdbc/JDBCDriver.java > b/lens-driver-jdbc/src/main/java/org/apache/lens/driver/ > jdbc/JDBCDriver.java > index e810e7c..8dd299a 100644 > --- a/lens-driver-jdbc/src/main/java/org/apache/lens/driver/ > jdbc/JDBCDriver.java > +++ b/lens-driver-jdbc/src/main/java/org/apache/lens/driver/ > jdbc/JDBCDriver.java > @@ -278,6 +278,7 @@ public class JDBCDriver extends AbstractLensDriver { > try { > stmt = createStatement(conn); > result.stmt = stmt; > + > Boolean isResultAvailable = stmt.execute(queryContext. > getRewrittenQuery()); > if (queryContext.getLensContext() > .getDriverStatus().isCanceled()) { > return result; > @@ -992,8 +993,12 @@ public class JDBCDriver extends AbstractLensDriver { > checkConfigured(); > try { > JdbcQueryContext ctx = getQueryContext(handle); > - ctx.getResultFuture().cancel(true); > - ctx.closeResult(); > + if (ctx != null) { > + ctx.getResultFuture().cancel(true); > + ctx.closeResult(); > + } > + } catch (LensException exc) { > + log.error("{} Failed to close query {}", getFullyQualifiedName(), > handle.getHandleId()); > } finally { > queryContextMap.remove(handle); > } > > http://git-wip-us.apache.org/repos/asf/lens/blob/4a24fb94/ > lens-driver-jdbc/src/main/resources/jdbcdriver-default.xml > ---------------------------------------------------------------------- > diff --git a/lens-driver-jdbc/src/main/resources/jdbcdriver-default.xml > b/lens-driver-jdbc/src/main/resources/jdbcdriver-default.xml > index 511fe71..2c69eef 100644 > --- a/lens-driver-jdbc/src/main/resources/jdbcdriver-default.xml > +++ b/lens-driver-jdbc/src/main/resources/jdbcdriver-default.xml > @@ -294,4 +294,16 @@ > <description>Cost based Query type mapping</description> > </property> > > + <property> > + <name>retry.messages.contains.map</name> > + <value>Query not found=org.apache.lens.server.api.retry. > ImmediateRetryHandler(2)</value> > + <description>Comma separated error messages and retry > policy</description> > + </property> > + > + <property> > + <name>query.retry.policy.classes</name> > + <value>org.apache.lens.server.api.retry. > SubstringMessagePolicyDecider</value> > + <description>List of classes to decide policies</description> > + </property> > + > </configuration> > > http://git-wip-us.apache.org/repos/asf/lens/blob/4a24fb94/ > lens-driver-jdbc/src/test/java/org/apache/lens/driver/ > jdbc/TestJdbcDriver.java > ---------------------------------------------------------------------- > diff --git > a/lens-driver-jdbc/src/test/java/org/apache/lens/driver/jdbc/TestJdbcDriver.java > b/lens-driver-jdbc/src/test/java/org/apache/lens/driver/ > jdbc/TestJdbcDriver.java > index 506935b..c08b04a 100644 > --- a/lens-driver-jdbc/src/test/java/org/apache/lens/driver/ > jdbc/TestJdbcDriver.java > +++ b/lens-driver-jdbc/src/test/java/org/apache/lens/driver/ > jdbc/TestJdbcDriver.java > @@ -768,14 +768,6 @@ public class TestJdbcDriver { > } > > driver.closeQuery(handle); > - // Close again, should get not found > - try { > - driver.closeQuery(handle); > - fail("Close again should have thrown exception"); > - } catch (LensException ex) { > - assertTrue(ex.getMessage().contains("not found") && > ex.getMessage().contains(handle.getHandleId().toString())); > - System.out.println("Matched exception"); > - } > } else { > fail("Only in memory result set is supported as of now"); > } > > http://git-wip-us.apache.org/repos/asf/lens/blob/4a24fb94/ > lens-server-api/src/main/java/org/apache/lens/server/api/ > LensConfConstants.java > ---------------------------------------------------------------------- > diff --git > a/lens-server-api/src/main/java/org/apache/lens/server/api/LensConfConstants.java > b/lens-server-api/src/main/java/org/apache/lens/server/ > api/LensConfConstants.java > index 2147f08..58a235a 100644 > --- a/lens-server-api/src/main/java/org/apache/lens/server/ > api/LensConfConstants.java > +++ b/lens-server-api/src/main/java/org/apache/lens/server/ > api/LensConfConstants.java > @@ -1341,4 +1341,9 @@ public final class LensConfConstants { > > public static final String SSL_KEYSTORE_PASSWORD = SERVER_PFX + > "ssl.password"; > > + /** > + * Message map for configured policy > + */ > + public static final String RETRY_MESSAGE_MAP = > "retry.messages.contains.map"; > + > } > > http://git-wip-us.apache.org/repos/asf/lens/blob/4a24fb94/ > lens-server-api/src/main/java/org/apache/lens/server/api/ > retry/BackOffRetryHandler.java > ---------------------------------------------------------------------- > diff --git a/lens-server-api/src/main/java/org/apache/lens/server/ > api/retry/BackOffRetryHandler.java b/lens-server-api/src/main/ > java/org/apache/lens/server/api/retry/BackOffRetryHandler.java > index 5ea5710..2d8b8e4 100644 > --- a/lens-server-api/src/main/java/org/apache/lens/server/ > api/retry/BackOffRetryHandler.java > +++ b/lens-server-api/src/main/java/org/apache/lens/server/ > api/retry/BackOffRetryHandler.java > @@ -41,7 +41,10 @@ import java.io.Serializable; > * } > * } > * > - * Note that this is only one of the possible use cases, other complex > use cases are in retry framework. > + * * Note > + * 1. This is only one of the possible use cases, other complex use > cases are in retry framework. > + * 2. Every implementation needs an all String arguments constructor > which can be used in > + * {@link SubstringMessagePolicyDecider} > */ > public interface BackOffRetryHandler<FC extends FailureContext> extends > Serializable { > > > http://git-wip-us.apache.org/repos/asf/lens/blob/4a24fb94/ > lens-server-api/src/main/java/org/apache/lens/server/api/retry/ > FibonacciExponentialBackOffRetryHandler.java > ---------------------------------------------------------------------- > diff --git a/lens-server-api/src/main/java/org/apache/lens/server/ > api/retry/FibonacciExponentialBackOffRetryHandler.java > b/lens-server-api/src/main/java/org/apache/lens/server/api/retry/ > FibonacciExponentialBackOffRetryHandler.java > index 01da25d..be3fc52 100644 > --- a/lens-server-api/src/main/java/org/apache/lens/server/api/retry/ > FibonacciExponentialBackOffRetryHandler.java > +++ b/lens-server-api/src/main/java/org/apache/lens/server/api/retry/ > FibonacciExponentialBackOffRetryHandler.java > @@ -33,6 +33,10 @@ public class FibonacciExponentialBackOffRetryHandler<FC > extends FailureContext> > final long maxDelay; > final long waitMillis; > > + public FibonacciExponentialBackOffRetryHandler(String numRetries, > String maxDelay, String waitMillis) { > + this(Integer.parseInt(numRetries), Long.parseLong(maxDelay), > Long.parseLong(waitMillis)); > + } > + > public FibonacciExponentialBackOffRetryHandler(int numRetries, long > maxDelay, long waitMillis) { > checkArgument(numRetries > 2); > fibonacci = new int[numRetries]; > > http://git-wip-us.apache.org/repos/asf/lens/blob/4a24fb94/ > lens-server-api/src/main/java/org/apache/lens/server/api/ > retry/ImmediateRetryHandler.java > ---------------------------------------------------------------------- > diff --git a/lens-server-api/src/main/java/org/apache/lens/server/ > api/retry/ImmediateRetryHandler.java b/lens-server-api/src/main/ > java/org/apache/lens/server/api/retry/ImmediateRetryHandler.java > index c1c0126..cea9df3 100644 > --- a/lens-server-api/src/main/java/org/apache/lens/server/api/retry/ > ImmediateRetryHandler.java > +++ b/lens-server-api/src/main/java/org/apache/lens/server/api/retry/ > ImmediateRetryHandler.java > @@ -29,6 +29,10 @@ public class ImmediateRetryHandler<FC extends > FailureContext> implements BackOff > this(1); > } > > + public ImmediateRetryHandler(String retries) { > + this(Integer.parseInt(retries)); > + } > + > @Override > public boolean canTryOpNow(FC failContext) { > return true; > @@ -41,6 +45,6 @@ public class ImmediateRetryHandler<FC extends > FailureContext> implements BackOff > > @Override > public boolean hasExhaustedRetries(FC failContext) { > - return ++retriesDone > retries; > + return failContext.getFailCount() > retries; > } > } > > http://git-wip-us.apache.org/repos/asf/lens/blob/4a24fb94/ > lens-server-api/src/main/java/org/apache/lens/server/api/ > retry/NoRetryHandler.java > ---------------------------------------------------------------------- > diff --git > a/lens-server-api/src/main/java/org/apache/lens/server/api/retry/NoRetryHandler.java > b/lens-server-api/src/main/java/org/apache/lens/server/ > api/retry/NoRetryHandler.java > index df68a48..0a2dac1 100644 > --- a/lens-server-api/src/main/java/org/apache/lens/server/ > api/retry/NoRetryHandler.java > +++ b/lens-server-api/src/main/java/org/apache/lens/server/ > api/retry/NoRetryHandler.java > @@ -32,4 +32,9 @@ public class NoRetryHandler<FC extends FailureContext> > extends ImmediateRetryHan > public long getOperationNextTime(FC failContext) { > return Long.MAX_VALUE; > } > + > + @Override > + public boolean hasExhaustedRetries(FC failContext) { > + return true; > + } > } > > http://git-wip-us.apache.org/repos/asf/lens/blob/4a24fb94/ > lens-server/src/main/java/org/apache/lens/server/query/ > QueryExecutionServiceImpl.java > ---------------------------------------------------------------------- > diff --git > a/lens-server/src/main/java/org/apache/lens/server/query/QueryExecutionServiceImpl.java > b/lens-server/src/main/java/org/apache/lens/server/query/ > QueryExecutionServiceImpl.java > index 3f1731e..4023a24 100644 > --- a/lens-server/src/main/java/org/apache/lens/server/query/ > QueryExecutionServiceImpl.java > +++ b/lens-server/src/main/java/org/apache/lens/server/query/ > QueryExecutionServiceImpl.java > @@ -47,6 +47,7 @@ import org.apache.lens.api.query.*; > import org.apache.lens.api.query.QueryStatus.Status; > import org.apache.lens.cube.metadata.DateUtil; > import org.apache.lens.driver.hive.HiveDriver; > +import org.apache.lens.driver.jdbc.JDBCDriver; > import org.apache.lens.server.BaseLensService; > import org.apache.lens.server.LensServerConf; > import org.apache.lens.server.LensServices; > @@ -913,7 +914,11 @@ public class QueryExecutionServiceImpl extends > BaseLensService implements QueryE > if (removeFromLaunchedQueries(ctx)) { > processWaitingQueriesAsync(ctx); > } > - if (ctx.getDriverStatus().failed() && > !getDriverRetryPolicy(ctx).hasExhaustedRetries(ctx)) > { > + > + /*Upon server restarts, the driver status isn't reliable in case of > jdbc due to query not found error, > + hence we fall back to the context status and retry */ > + if ((ctx.getDriverStatus().failed() || (ctx.getStatus().failing() > + && ctx.getSelectedDriver() instanceof JDBCDriver)) && > !getDriverRetryPolicy(ctx).hasExhaustedRetries(ctx)) { > log.info("query {} will be retried on the same driver {}", > ctx.getQueryHandle(), ctx.getSelectedDriver(). > getFullyQualifiedName()); > ctx.extractFailedAttempt(); > @@ -975,8 +980,10 @@ public class QueryExecutionServiceImpl extends > BaseLensService implements QueryE > > private BackOffRetryHandler<QueryContext> getDriverRetryPolicy(QueryContext > ctx) { > if (ctx.getDriverRetryPolicy() == null) { > + String errorMessage = ctx.getDriverStatus().getErrorMessage() != > null ? ctx.getDriverStatus().getErrorMessage() > + : ctx.getStatus().getErrorMessage(); > ctx.setDriverRetryPolicy(ctx.getSelectedDriver(). > getRetryPolicyDecider() > - .decidePolicy(ctx.getDriverStatus().getErrorMessage())); > + .decidePolicy(errorMessage)); > } > return ctx.getDriverRetryPolicy(); > } > > http://git-wip-us.apache.org/repos/asf/lens/blob/4a24fb94/ > src/site/apt/admin/hivedriver-config.apt > ---------------------------------------------------------------------- > diff --git a/src/site/apt/admin/hivedriver-config.apt > b/src/site/apt/admin/hivedriver-config.apt > index 71208bb..5d99892 100644 > --- a/src/site/apt/admin/hivedriver-config.apt > +++ b/src/site/apt/admin/hivedriver-config.apt > @@ -78,4 +78,8 @@ Hive driver configuration > *--+--+---+--+ > |20|lens.driver.hive.waiting.queries.selection.policy.factories| > |Factories used to instantiate driver specific waiting queries selection > policies. Every factory should be an implementation of > org.apache.lens.server.api.common.ConfigBasedObjectCreationFactory and > create an implementation of org.apache.lens.server.api.query.collect. > WaitingQueriesSelectionPolicy.| > *--+--+---+--+ > +|21|query.retry.policy.classes|org.apache.lens.server.api.retry. > SubstringMessagePolicyDecider|List of policy decider classes| > +*--+--+---+--+ > +|22|retry.messages.contains.map|Session handle not > found=org.apache.lens.server.api.retry.ImmediateRetryHandler(2)|Comma > separated error messages and retry policy| > +*--+--+---+--+ > The configuration parameters and their default values > > http://git-wip-us.apache.org/repos/asf/lens/blob/4a24fb94/ > src/site/apt/admin/jdbcdriver-config.apt > ---------------------------------------------------------------------- > diff --git a/src/site/apt/admin/jdbcdriver-config.apt > b/src/site/apt/admin/jdbcdriver-config.apt > index c50872b..792278e 100644 > --- a/src/site/apt/admin/jdbcdriver-config.apt > +++ b/src/site/apt/admin/jdbcdriver-config.apt > @@ -101,4 +101,8 @@ Jdbc driver configuration > *--+--+---+--+ > |38|lens.query.timeout.millis|3600000|The runtime(millis) of the query > after which query will be timedout and cancelled. Default is 1 hour for > jdbc queries.| > *--+--+---+--+ > +|39|query.retry.policy.classes|org.apache.lens.server.api.retry. > SubstringMessagePolicyDecider|List of classes to decide policies| > +*--+--+---+--+ > +|40|retry.messages.contains.map|Query not found=org.apache.lens.server. > api.retry.ImmediateRetryHandler(2)|Comma separated error messages and > retry policy| > +*--+--+---+--+ > The configuration parameters and their default values > >
