Repository: tajo Updated Branches: refs/heads/master 957d2d3f4 -> 8824ba559
TAJO-738: NPE occurs when failed in QueryMaster's GlobalPlanner.build(). (hyoungjunkim via hyunsik) Project: http://git-wip-us.apache.org/repos/asf/tajo/repo Commit: http://git-wip-us.apache.org/repos/asf/tajo/commit/8824ba55 Tree: http://git-wip-us.apache.org/repos/asf/tajo/tree/8824ba55 Diff: http://git-wip-us.apache.org/repos/asf/tajo/diff/8824ba55 Branch: refs/heads/master Commit: 8824ba5590003be7feb1fac72558d73e870ecdb7 Parents: 957d2d3 Author: Hyunsik Choi <[email protected]> Authored: Tue Apr 8 16:36:07 2014 +0900 Committer: Hyunsik Choi <[email protected]> Committed: Tue Apr 8 16:36:07 2014 +0900 ---------------------------------------------------------------------- CHANGES.txt | 3 + .../main/java/org/apache/tajo/cli/TajoCli.java | 3 + .../tajo/master/querymaster/QueryMaster.java | 33 ++++--- .../master/querymaster/QueryMasterTask.java | 95 +++++++++++--------- .../tajo/worker/TajoWorkerClientService.java | 24 +++-- .../src/main/resources/webapps/admin/query.jsp | 6 +- .../src/main/resources/webapps/worker/index.jsp | 17 ++-- .../resources/webapps/worker/querydetail.jsp | 20 ++++- 8 files changed, 128 insertions(+), 73 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/tajo/blob/8824ba55/CHANGES.txt ---------------------------------------------------------------------- diff --git a/CHANGES.txt b/CHANGES.txt index 214b398..cd3a189 100644 --- a/CHANGES.txt +++ b/CHANGES.txt @@ -306,6 +306,9 @@ Release 0.8.0 - unreleased BUG FIXES + TAJO-738: NPE occur when failed in QueryMaster's GlobalPlanner.build(). + (hyoungjunkim via hyunsik) + TAJO-739: A subquery with the same column alias caused planning error. (hyoungjunkim via hyunsik) http://git-wip-us.apache.org/repos/asf/tajo/blob/8824ba55/tajo-client/src/main/java/org/apache/tajo/cli/TajoCli.java ---------------------------------------------------------------------- diff --git a/tajo-client/src/main/java/org/apache/tajo/cli/TajoCli.java b/tajo-client/src/main/java/org/apache/tajo/cli/TajoCli.java index 426c115..2a49d0b 100644 --- a/tajo-client/src/main/java/org/apache/tajo/cli/TajoCli.java +++ b/tajo-client/src/main/java/org/apache/tajo/cli/TajoCli.java @@ -366,6 +366,9 @@ public class TajoCli { if (status.getState() == QueryState.QUERY_ERROR) { sout.println("Internal error!"); + if(status.getErrorMessage() != null && !status.getErrorMessage().isEmpty()) { + sout.println(status.getErrorMessage()); + } } else if (status.getState() == QueryState.QUERY_FAILED) { sout.println("Query failed!"); } else if (status.getState() == QueryState.QUERY_KILLED) { http://git-wip-us.apache.org/repos/asf/tajo/blob/8824ba55/tajo-core/tajo-core-backend/src/main/java/org/apache/tajo/master/querymaster/QueryMaster.java ---------------------------------------------------------------------- diff --git a/tajo-core/tajo-core-backend/src/main/java/org/apache/tajo/master/querymaster/QueryMaster.java b/tajo-core/tajo-core-backend/src/main/java/org/apache/tajo/master/querymaster/QueryMaster.java index abdc214..523f5ba 100644 --- a/tajo-core/tajo-core-backend/src/main/java/org/apache/tajo/master/querymaster/QueryMaster.java +++ b/tajo-core/tajo-core-backend/src/main/java/org/apache/tajo/master/querymaster/QueryMaster.java @@ -354,16 +354,19 @@ public class QueryMaster extends CompositeService implements EventHandler { } private TajoHeartbeat buildTajoHeartBeat(QueryMasterTask queryMasterTask) { - TajoHeartbeat queryHeartbeat = TajoHeartbeat.newBuilder() - .setTajoWorkerHost(workerContext.getQueryMasterManagerService().getBindAddr().getHostName()) - .setTajoQueryMasterPort(workerContext.getQueryMasterManagerService().getBindAddr().getPort()) - .setTajoWorkerClientPort(workerContext.getTajoWorkerClientService().getBindAddr().getPort()) - .setState(queryMasterTask.getState()) - .setQueryId(queryMasterTask.getQueryId().getProto()) - .setQueryProgress(queryMasterTask.getQuery().getProgress()) - .setQueryFinishTime(queryMasterTask.getQuery().getFinishTime()) - .build(); - return queryHeartbeat; + TajoHeartbeat.Builder builder = TajoHeartbeat.newBuilder(); + + builder.setTajoWorkerHost(workerContext.getQueryMasterManagerService().getBindAddr().getHostName()); + builder.setTajoQueryMasterPort(workerContext.getQueryMasterManagerService().getBindAddr().getPort()); + builder.setTajoWorkerClientPort(workerContext.getTajoWorkerClientService().getBindAddr().getPort()); + builder.setState(queryMasterTask.getState()); + builder.setQueryId(queryMasterTask.getQueryId().getProto()); + + if (queryMasterTask.getQuery() != null) { + builder.setQueryProgress(queryMasterTask.getQuery().getProgress()); + builder.setQueryFinishTime(queryMasterTask.getQuery().getFinishTime()); + } + return builder.build(); } private class QueryStartEventHandler implements EventHandler<QueryStartEvent> { @@ -374,10 +377,18 @@ public class QueryMaster extends CompositeService implements EventHandler { event.getQueryId(), event.getSession(), event.getQueryContext(), event.getSql(), event.getLogicalPlanJson()); queryMasterTask.init(systemConf); - queryMasterTask.start(); + if (!queryMasterTask.isInitError()) { + queryMasterTask.start(); + } + synchronized(queryMasterTasks) { queryMasterTasks.put(event.getQueryId(), queryMasterTask); } + + if (queryMasterTask.isInitError()) { + queryMasterContext.stopQuery(queryMasterTask.getQueryId()); + return; + } } } http://git-wip-us.apache.org/repos/asf/tajo/blob/8824ba55/tajo-core/tajo-core-backend/src/main/java/org/apache/tajo/master/querymaster/QueryMasterTask.java ---------------------------------------------------------------------- diff --git a/tajo-core/tajo-core-backend/src/main/java/org/apache/tajo/master/querymaster/QueryMasterTask.java b/tajo-core/tajo-core-backend/src/main/java/org/apache/tajo/master/querymaster/QueryMasterTask.java index 79b4a08..271eaf9 100644 --- a/tajo-core/tajo-core-backend/src/main/java/org/apache/tajo/master/querymaster/QueryMasterTask.java +++ b/tajo-core/tajo-core-backend/src/main/java/org/apache/tajo/master/querymaster/QueryMasterTask.java @@ -27,6 +27,7 @@ import org.apache.hadoop.fs.Path; import org.apache.hadoop.fs.permission.FsPermission; import org.apache.hadoop.security.UserGroupInformation; import org.apache.hadoop.service.CompositeService; +import org.apache.hadoop.util.StringUtils; import org.apache.hadoop.yarn.event.EventHandler; import org.apache.hadoop.yarn.util.Clock; import org.apache.tajo.*; @@ -109,6 +110,8 @@ public class QueryMasterTask extends CompositeService { private TajoMetrics queryMetrics; + private Throwable initError; + public QueryMasterTask(QueryMaster.QueryMasterContext queryMasterContext, QueryId queryId, Session session, QueryContext queryContext, String sql, String logicalPlanJson) { @@ -153,8 +156,9 @@ public class QueryMasterTask extends CompositeService { queryMetrics = new TajoMetrics(queryId.toString()); super.init(systemConf); - } catch (IOException e) { - LOG.error(e.getMessage(), e); + } catch (Throwable t) { + LOG.error(t.getMessage(), t); + initError = t; } } @@ -294,49 +298,42 @@ public class QueryMasterTask extends CompositeService { } public synchronized void startQuery() { - if(query != null) { - LOG.warn("Query already started"); - return; - } - CatalogService catalog = getQueryTaskContext().getQueryMasterContext().getWorkerContext().getCatalog(); - LogicalPlanner planner = new LogicalPlanner(catalog); - LogicalOptimizer optimizer = new LogicalOptimizer(systemConf); - Expr expr; - if (queryContext.isHiveQueryMode()) { - HiveQLAnalyzer HiveQLAnalyzer = new HiveQLAnalyzer(); - expr = HiveQLAnalyzer.parse(sql); - } else { - SQLAnalyzer analyzer = new SQLAnalyzer(); - expr = analyzer.parse(sql); - } - LogicalPlan plan = null; try { - plan = planner.createPlan(session, expr); + if (query != null) { + LOG.warn("Query already started"); + return; + } + CatalogService catalog = getQueryTaskContext().getQueryMasterContext().getWorkerContext().getCatalog(); + LogicalPlanner planner = new LogicalPlanner(catalog); + LogicalOptimizer optimizer = new LogicalOptimizer(systemConf); + Expr expr; + if (queryContext.isHiveQueryMode()) { + HiveQLAnalyzer HiveQLAnalyzer = new HiveQLAnalyzer(); + expr = HiveQLAnalyzer.parse(sql); + } else { + SQLAnalyzer analyzer = new SQLAnalyzer(); + expr = analyzer.parse(sql); + } + LogicalPlan plan = planner.createPlan(session, expr); optimizer.optimize(plan); - } catch (PlanningException e) { - //TODO how set query failed(???) - LOG.error(e.getMessage(), e); - } - - GlobalEngine.DistributedQueryHookManager hookManager = new GlobalEngine.DistributedQueryHookManager(); - hookManager.addHook(new GlobalEngine.InsertHook()); - hookManager.doHooks(queryContext, plan); - try { + GlobalEngine.DistributedQueryHookManager hookManager = new GlobalEngine.DistributedQueryHookManager(); + hookManager.addHook(new GlobalEngine.InsertHook()); + hookManager.doHooks(queryContext, plan); for (LogicalPlan.QueryBlock block : plan.getQueryBlocks()) { LogicalNode[] scanNodes = PlannerUtil.findAllNodes(block.getRoot(), NodeType.SCAN); - if(scanNodes != null) { - for(LogicalNode eachScanNode: scanNodes) { - ScanNode scanNode = (ScanNode)eachScanNode; + if (scanNodes != null) { + for (LogicalNode eachScanNode : scanNodes) { + ScanNode scanNode = (ScanNode) eachScanNode; tableDescMap.put(scanNode.getCanonicalName(), scanNode.getTableDesc()); } } scanNodes = PlannerUtil.findAllNodes(block.getRoot(), NodeType.PARTITIONS_SCAN); - if(scanNodes != null) { - for(LogicalNode eachScanNode: scanNodes) { - ScanNode scanNode = (ScanNode)eachScanNode; + if (scanNodes != null) { + for (LogicalNode eachScanNode : scanNodes) { + ScanNode scanNode = (ScanNode) eachScanNode; tableDescMap.put(scanNode.getCanonicalName(), scanNode.getTableDesc()); } } @@ -349,11 +346,9 @@ public class QueryMasterTask extends CompositeService { dispatcher.register(QueryEventType.class, query); queryTaskContext.getEventHandler().handle(new QueryEvent(queryId, QueryEventType.START)); - } catch (Exception e) { - LOG.error(e.getMessage(), e); - //TODO how set query failed(???) - //send FAIL query status - //this.statusMessage = StringUtils.stringifyException(e); + } catch (Throwable t) { + LOG.error(t.getMessage(), t); + initError = t; } } @@ -471,14 +466,34 @@ public class QueryMasterTask extends CompositeService { return queryId; } + public boolean isInitError() { + return initError != null; + } + public QueryState getState() { if(query == null) { - return QueryState.QUERY_NOT_ASSIGNED; + if (isInitError()) { + return QueryState.QUERY_ERROR; + } else { + return QueryState.QUERY_NOT_ASSIGNED; + } } else { return query.getState(); } } + public String getErrorMessage() { + if (isInitError()) { + return StringUtils.stringifyException(initError); + } else { + return null; + } + } + + public long getQuerySubmitTime() { + return this.querySubmitTime; + } + public class QueryMasterTaskContext { EventHandler eventHandler; public QueryMaster.QueryMasterContext getQueryMasterContext() { http://git-wip-us.apache.org/repos/asf/tajo/blob/8824ba55/tajo-core/tajo-core-backend/src/main/java/org/apache/tajo/worker/TajoWorkerClientService.java ---------------------------------------------------------------------- diff --git a/tajo-core/tajo-core-backend/src/main/java/org/apache/tajo/worker/TajoWorkerClientService.java b/tajo-core/tajo-core-backend/src/main/java/org/apache/tajo/worker/TajoWorkerClientService.java index a73623f..937d886 100644 --- a/tajo-core/tajo-core-backend/src/main/java/org/apache/tajo/worker/TajoWorkerClientService.java +++ b/tajo-core/tajo-core-backend/src/main/java/org/apache/tajo/worker/TajoWorkerClientService.java @@ -184,20 +184,26 @@ public class TajoWorkerClientService extends AbstractService { return builder.build(); } - queryMasterTask.touchSessionTime(); - Query query = queryMasterTask.getQuery(); - - builder.setState(query.getState()); - builder.setProgress(query.getProgress()); - builder.setSubmitTime(query.getAppSubmitTime()); builder.setHasResult( !(queryMasterTask.getQueryTaskContext().getQueryContext().isCreateTable() || queryMasterTask.getQueryTaskContext().getQueryContext().isInsert()) ); - if (query.getState() == TajoProtos.QueryState.QUERY_SUCCEEDED) { - builder.setFinishTime(query.getFinishTime()); + + queryMasterTask.touchSessionTime(); + Query query = queryMasterTask.getQuery(); + + if (query != null) { + builder.setState(query.getState()); + builder.setProgress(query.getProgress()); + builder.setSubmitTime(query.getAppSubmitTime()); + if (query.getState() == TajoProtos.QueryState.QUERY_SUCCEEDED) { + builder.setFinishTime(query.getFinishTime()); + } else { + builder.setFinishTime(System.currentTimeMillis()); + } } else { - builder.setFinishTime(System.currentTimeMillis()); + builder.setState(queryMasterTask.getState()); + builder.setErrorMessage(queryMasterTask.getErrorMessage()); } } return builder.build(); http://git-wip-us.apache.org/repos/asf/tajo/blob/8824ba55/tajo-core/tajo-core-backend/src/main/resources/webapps/admin/query.jsp ---------------------------------------------------------------------- diff --git a/tajo-core/tajo-core-backend/src/main/resources/webapps/admin/query.jsp b/tajo-core/tajo-core-backend/src/main/resources/webapps/admin/query.jsp index c1fccb8..6f15a0e 100644 --- a/tajo-core/tajo-core-backend/src/main/resources/webapps/admin/query.jsp +++ b/tajo-core/tajo-core-backend/src/main/resources/webapps/admin/query.jsp @@ -111,7 +111,7 @@ <tr></tr><th>QueryId</th><th>Query Master</th><th>Started</th><th>Finished</th><th>Time</th><th>Status</th><th>sql</th></tr> <% for(QueryInProgress eachQuery: finishedQueries) { - long runTime = eachQuery.getQueryInfo().getFinishTime() >= 0 ? + long runTime = eachQuery.getQueryInfo().getFinishTime() > 0 ? eachQuery.getQueryInfo().getFinishTime() - eachQuery.getQueryInfo().getStartTime() : -1; String detailView = "http://"; + eachQuery.getQueryInfo().getQueryMasterHost() + ":" + portMap.get(eachQuery.getQueryInfo().getQueryMasterHost()) + "/querydetail.jsp?queryId=" + eachQuery.getQueryId(); @@ -120,8 +120,8 @@ <td><a href='<%=detailView%>'><%=eachQuery.getQueryId()%></a></td> <td><%=eachQuery.getQueryInfo().getQueryMasterHost()%></td> <td><%=df.format(eachQuery.getQueryInfo().getStartTime())%></td> - <td><%=eachQuery.getQueryInfo().getFinishTime() >= 0 ? df.format(eachQuery.getQueryInfo().getFinishTime()) : "N/A"%></td> - <td><%=runTime == -1 ? "N/A" : StringUtils.formatTime(runTime) %></td> + <td><%=eachQuery.getQueryInfo().getFinishTime() > 0 ? df.format(eachQuery.getQueryInfo().getFinishTime()) : "-"%></td> + <td><%=runTime == -1 ? "-" : StringUtils.formatTime(runTime) %></td> <td><%=eachQuery.getQueryInfo().getQueryState()%></td> <td><%=eachQuery.getQueryInfo().getSql()%></td> </tr> http://git-wip-us.apache.org/repos/asf/tajo/blob/8824ba55/tajo-core/tajo-core-backend/src/main/resources/webapps/worker/index.jsp ---------------------------------------------------------------------- diff --git a/tajo-core/tajo-core-backend/src/main/resources/webapps/worker/index.jsp b/tajo-core/tajo-core-backend/src/main/resources/webapps/worker/index.jsp index 1150ade..c30a72d 100644 --- a/tajo-core/tajo-core-backend/src/main/resources/webapps/worker/index.jsp +++ b/tajo-core/tajo-core-backend/src/main/resources/webapps/worker/index.jsp @@ -70,13 +70,14 @@ if(tajoWorker.getWorkerContext().isQueryMasterMode()) { } else { %> <table width="100%" border="1" class="border_table"> - <tr><th>QueryId</th><th>StartTime</th><th>FinishTime</th><th>Progress</th><th>RunTime</th></tr> + <tr><th>QueryId</th><th>Status</th><th>StartTime</th><th>FinishTime</th><th>Progress</th><th>RunTime</th></tr> <% for(QueryMasterTask eachQueryMasterTask: queryMasterTasks) { Query query = eachQueryMasterTask.getQuery(); %> <tr> <td align='center'><a href='querydetail.jsp?queryId=<%=query.getId()%>'><%=query.getId()%></a></td> + <td align='center'><%=eachQueryMasterTask.getState()%></td> <td align='center'><%=df.format(query.getStartTime())%></td> <td align='center'><%=query.getFinishTime() == 0 ? "-" : df.format(query.getFinishTime())%></td> <td align='center'><%=(int)(query.getProgress()*100.0f)%>%</td> @@ -96,17 +97,19 @@ if(tajoWorker.getWorkerContext().isQueryMasterMode()) { } else { %> <table width="100%" border="1" class="border_table"> - <tr><th>QueryId</th><th>StartTime</th><th>FinishTime</th><th>Progress</th><th>RunTime</th></tr> + <tr><th>QueryId</th><th>Status</th><th>StartTime</th><th>FinishTime</th><th>Progress</th><th>RunTime</th></tr> <% for(QueryMasterTask eachQueryMasterTask: finishedQueryMasterTasks) { Query query = eachQueryMasterTask.getQuery(); + long startTime = query != null ? query.getStartTime() : eachQueryMasterTask.getQuerySubmitTime(); %> <tr> - <td align='center'><a href='querydetail.jsp?queryId=<%=query.getId()%>'><%=query.getId()%></a></td> - <td align='center'><%=df.format(query.getStartTime())%></td> - <td align='center'><%=query.getFinishTime() == 0 ? "-" : df.format(query.getFinishTime())%></td> - <td align='center'><%=(int)(query.getProgress()*100.0f)%>%</td> - <td align='right'><%=JSPUtil.getElapsedTime(query.getStartTime(), query.getFinishTime())%></td> + <td align='center'><a href='querydetail.jsp?queryId=<%=eachQueryMasterTask.getQueryId()%>'><%=eachQueryMasterTask.getQueryId()%></a></td> + <td align='center'><%=eachQueryMasterTask.getState()%></td> + <td align='center'><%=df.format(startTime)%></td> + <td align='center'><%=(query == null || query.getFinishTime() == 0) ? "-" : df.format(query.getFinishTime())%></td> + <td align='center'><%=(query == null) ? "-" : (int)(query.getProgress()*100.0f)%>%</td> + <td align='right'><%=(query == null) ? "-" : JSPUtil.getElapsedTime(query.getStartTime(), query.getFinishTime())%></td> </tr> <% } //end of for http://git-wip-us.apache.org/repos/asf/tajo/blob/8824ba55/tajo-core/tajo-core-backend/src/main/resources/webapps/worker/querydetail.jsp ---------------------------------------------------------------------- diff --git a/tajo-core/tajo-core-backend/src/main/resources/webapps/worker/querydetail.jsp b/tajo-core/tajo-core-backend/src/main/resources/webapps/worker/querydetail.jsp index 2d867ed..3de20fe 100644 --- a/tajo-core/tajo-core-backend/src/main/resources/webapps/worker/querydetail.jsp +++ b/tajo-core/tajo-core-backend/src/main/resources/webapps/worker/querydetail.jsp @@ -34,12 +34,15 @@ QueryMasterTask queryMasterTask = tajoWorker.getWorkerContext() .getQueryMasterManagerService().getQueryMaster().getQueryMasterTask(queryId, true); - if(queryMasterTask == null) { + if (queryMasterTask == null) { out.write("<script type='text/javascript'>alert('no query'); history.back(0); </script>"); return; } Query query = queryMasterTask.getQuery(); - List<SubQuery> subQueries = JSPUtil.sortSubQuery(query.getSubQueries()); + List<SubQuery> subQueries = null; + if (query != null) { + subQueries = JSPUtil.sortSubQuery(query.getSubQueries()); + } SimpleDateFormat df = new SimpleDateFormat("yyyy-MM-dd HH:mm:ss"); %> @@ -56,8 +59,16 @@ <div class='contents'> <h2>Tajo Worker: <a href='index.jsp'><%=tajoWorker.getWorkerContext().getWorkerName()%></a></h2> <hr/> +<% +if (query == null) { + String errorMessage = queryMasterTask.getErrorMessage(); + out.write("Query Status: " + queryMasterTask.getState()); + if (errorMessage != null && !errorMessage.isEmpty()) { + out.write("<p/>Message:<p/><pre>" + errorMessage + "</pre>"); + } +} else { +%> <h3><%=queryId.toString()%> <a href='queryplan.jsp?queryId=<%=queryId%>'>[Query Plan]</a></h3> - <table width="100%" border="1" class="border_table"> <tr><th>ID</th><th>State</th><th>Started</th><th>Finished</th><th>Running time</th><th>Progress</th><th>Tasks</th></tr> <% @@ -86,6 +97,9 @@ for(SubQuery eachSubQuery: subQueries) { <h3>Distributed Query Plan</h3> <pre style="white-space:pre-wrap;"><%=query.getPlan().toString()%></pre> <hr/> +<% +} //end of else [if (query == null)] +%> </div> </body> </html> \ No newline at end of file
