> On March 19, 2016, 2:54 p.m., Rajat Khandelwal wrote: > > lens-cli/src/test/java/org/apache/lens/cli/TestLensQueryCommands.java, line > > 174 > > <https://reviews.apache.org/r/44171/diff/5/?file=1300904#file1300904line174> > > > > Why do we have to wait? when query gives the result, it should already > > have been succeeded, right?
This is required for stremaing case. The result us returned when state is EXECUTED. > On March 19, 2016, 2:54 p.m., Rajat Khandelwal wrote: > > lens-cli/src/test/java/org/apache/lens/cli/TestLensQueryCommands.java, line > > 154 > > <https://reviews.apache.org/r/44171/diff/5/?file=1300904#file1300904line154> > > > > I think functions of query command should only be used when they are > > being tested. Here, I think `client.getQueries(state, queryName, user, > > driver, fromDate, toDate)` would be a better choice. Followed the exeisting structure of the test case where LensQueryCommands output was being parsed. If you feel we need to change, we can do it under a separate JIRA and change in all test cases in this class. Also I have seen few users who are invoking lens CLI via a script and then relying on the parsed output (which may not be apt usage though). This should cover that scenario > On March 19, 2016, 2:54 p.m., Rajat Khandelwal wrote: > > lens-cli/src/test/java/org/apache/lens/cli/TestLensQueryCommands.java, > > lines 156-160 > > <https://reviews.apache.org/r/44171/diff/5/?file=1300904#file1300904line156> > > > > This volatile parsing can be avoided with usage of client's method of > > listing queries. Followed the exeisting structure of the test case where LensQueryCommands output was being parsed. If you feel we need to change, we can do it under a separate JIRA and change in all test cases in this class. Also I have seen few users who are invoking lens CLI via a script and then relying on the parsed output(which may not be apt usage though). This should cover that scenario. > On March 19, 2016, 2:54 p.m., Rajat Khandelwal wrote: > > lens-cli/src/test/java/org/apache/lens/cli/TestLensQueryCommands.java, line > > 555 > > <https://reviews.apache.org/r/44171/diff/5/?file=1300904#file1300904line555> > > > > Why can't we just compare objects always? Why do we need to compare > > `toString`s first? In some cases objects dont have overridden equals like QueryStatus. In such cases the comparison fails. Wanted to make use of toString() in that case, which should be ok from functionality perspective considering the way toString() is implemented in ToYAMLString . > On March 19, 2016, 2:54 p.m., Rajat Khandelwal wrote: > > lens-cli/src/test/java/org/apache/lens/cli/TestLensQueryCommands.java, line > > 553 > > <https://reviews.apache.org/r/44171/diff/5/?file=1300904#file1300904line553> > > > > `toString` can be different if there's an un-ordered collection in the > > object like set/map. e.g. LensConf has a map. so `toString` of lensConf is > > not deterministic. As of now only QueryHandle and QueryStatus are using ToYAMLString.toString() comparison. It should be ok for these. Incase such a situation arises later, we can handle then in test case. > On March 19, 2016, 2:54 p.m., Rajat Khandelwal wrote: > > lens-client/src/main/java/org/apache/lens/cleint/util/ProxyLensQuery.java, > > line 72 > > <https://reviews.apache.org/r/44171/diff/5/?file=1300905#file1300905line72> > > > > can't we just throw `e`? Logged exeception stacktrace in ProxyLensQuery (not sure if all current and future callers will do that). So only message is thrown without stack trace. Also this scenario should never arise as these methods are always present in Object Class. > On March 19, 2016, 2:54 p.m., Rajat Khandelwal wrote: > > lens-client/src/main/java/org/apache/lens/cleint/util/ProxyLensQuery.java, > > line 94 > > <https://reviews.apache.org/r/44171/diff/5/?file=1300905#file1300905line94> > > > > Can you add a comment saying this is only true assuming proxies are > > only created when you submit queries with the client. The constructor of Proxy needs LensStatement and handle. So it should be obvious that its a client side call. Comment may not be needed > On March 19, 2016, 2:54 p.m., Rajat Khandelwal wrote: > > lens-client/src/main/java/org/apache/lens/client/LensStatement.java, line > > 102 > > <https://reviews.apache.org/r/44171/diff/5/?file=1300908#file1300908line102> > > > > Should this return `LensClientResultSetWithStats`? Hoping for others' > > thoughts. In that case we'll need two methods, the other one being async, > > and returning `QueryHandle`. This method is used for async execution with an extra option to wait for query to finish. QueryHandle is the minimum information that we can return considering all cases. Don't want to add more execute methods and confuse users. We can take this into account when we refactor LensClient. LensClient has a mtehod LensClient.getResults(String, String) which returns LensClientResultSetWithStats > On March 19, 2016, 2:54 p.m., Rajat Khandelwal wrote: > > lens-client/src/main/java/org/apache/lens/client/LensStatement.java, line > > 127 > > <https://reviews.apache.org/r/44171/diff/5/?file=1300908#file1300908line127> > > > > From the code it looks like it's not waiting for query execution to > > finish. Can you double-check? Its using "EXECUTE_WITH_TIMEOUT" . The server QueryExeceutioinService will wait . The client doesn't need to wat explicitly. > On March 19, 2016, 2:54 p.m., Rajat Khandelwal wrote: > > lens-api/src/main/java/org/apache/lens/api/query/LensQueryDetails.java, > > line 82 > > <https://reviews.apache.org/r/44171/diff/5/?file=1300897#file1300897line82> > > > > Remove comment Will Remove in next patch. > On March 19, 2016, 2:54 p.m., Rajat Khandelwal wrote: > > lens-cli/src/test/java/org/apache/lens/cli/TestLensQueryCommands.java, line > > 552 > > <https://reviews.apache.org/r/44171/diff/5/?file=1300904#file1300904line552> > > > > casting not needed. Will remove. - Puneet ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/44171/#review124406 ----------------------------------------------------------- On March 16, 2016, 10:15 a.m., Puneet Gupta wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/44171/ > ----------------------------------------------------------- > > (Updated March 16, 2016, 10:15 a.m.) > > > Review request for lens. > > > Bugs: LENS_915 > https://issues.apache.org/jira/browse/LENS_915 > > > Repository: lens > > > Description > ------- > > Incremental patch on top of LENS-964 (will re-submit once lens-964 is > committed) > - - Updated cli query execution to use EXECUTE_WITH_TIMEOUT > - Added execute with timeout option to LensClient > - Default timeout value is 10 secs ( can be configured by client) > > > Diffs > ----- > > lens-api/src/main/java/org/apache/lens/api/query/LensQuery.java e0ee761 > lens-api/src/main/java/org/apache/lens/api/query/LensQueryDetails.java > PRE-CREATION > lens-api/src/main/java/org/apache/lens/api/query/QueryHandle.java 88e4b0f > lens-api/src/main/java/org/apache/lens/api/query/QueryStatus.java 7a9ada1 > lens-cli/src/main/java/org/apache/lens/cli/commands/BaseLensCommand.java > be1ca12 > > lens-cli/src/main/java/org/apache/lens/cli/commands/LensConnectionCommands.java > b760dad > lens-cli/src/main/java/org/apache/lens/cli/commands/LensQueryCommands.java > 2c7b17f > > lens-cli/src/main/java/org/apache/lens/cli/config/LensCliConfigConstants.java > PRE-CREATION > lens-cli/src/test/java/org/apache/lens/cli/TestLensQueryCommands.java > 616bf5d > lens-client/src/main/java/org/apache/lens/cleint/util/ProxyLensQuery.java > PRE-CREATION > lens-client/src/main/java/org/apache/lens/client/LensClient.java ea0cd76 > lens-client/src/main/java/org/apache/lens/client/LensConnection.java > eeb473a > lens-client/src/main/java/org/apache/lens/client/LensStatement.java 33c26e1 > > lens-client/src/main/java/org/apache/lens/client/jdbc/LensJdbcStatement.java > 10f7155 > lens-client/src/main/resources/lens-client-default.xml 35b2d28 > lens-examples/src/main/java/org/apache/lens/examples/SampleQueries.java > 805a282 > lens-ml-lib/src/main/java/org/apache/lens/ml/impl/LensMLImpl.java e090e68 > lens-ml-lib/src/main/java/org/apache/lens/rdd/LensRDDClient.java b4f43ec > > lens-server-api/src/main/java/org/apache/lens/server/api/query/QueryContext.java > 96846c1 > > lens-server/src/main/java/org/apache/lens/server/query/QueryExecutionServiceImpl.java > 49ab241 > > lens-server/src/main/java/org/apache/lens/server/query/QueryServiceResource.java > a043550 > > lens-server/src/main/java/org/apache/lens/server/ui/QueryServiceUIResource.java > 304dc8e > lens-server/src/test/java/org/apache/lens/server/LensServerTestUtil.java > b651b79 > lens-server/src/test/java/org/apache/lens/server/TestServerMode.java > fce6e5f > lens-server/src/test/java/org/apache/lens/server/TestServerRestart.java > 0f55d9e > > lens-server/src/test/java/org/apache/lens/server/common/RestAPITestUtil.java > 0e39b52 > > lens-server/src/test/java/org/apache/lens/server/query/TestQueryService.java > 699fa68 > > lens-server/src/test/java/org/apache/lens/server/query/TestResultFormatting.java > f66f89d > src/site/apt/user/client-config.apt 714db18 > > Diff: https://reviews.apache.org/r/44171/diff/ > > > Testing > ------- > > 1. Tested new test cases > ------------------------------------------------------- > T E S T S > ------------------------------------------------------- > Running org.apache.lens.cli.TestLensQueryCommands > Tests run: 4, Failures: 0, Errors: 0, Skipped: 0, Time elapsed: 88.629 sec - > in org.apache.lens.cli.TestLensQueryCommands > > 2. Tested CLI > lens-shell>set lens.query.enable.persistent.resultset.indriver=false > lens-shell>set lens.query.enable.persistent.resultset=true > Set param succeeded > lens-shell>cube select sample_dim.name, sample_dim2.name from sample_dim join > sample_dim2 on sample_dim.d2id=sample_dim2.id ORDER BY sample_dim.name DESC > 29 Feb 2016 13:08:58 [Spring Shell] INFO cliLogger - Executing query with > timeout of 10000 milliseconds > sample_dim.name sample_dim2.name > Result available in memory, attaching here: > > third thirteen > six sixteen > second twelve > fourth fourteen > first eleven > fifth fifteen > eight eighteen > 7 rows processed in (8) seconds. > lens-shell>set lens.cli.query.execute.timeout.millis=1000 > Client side Set lens.cli.query.execute.timeout.millis=1000 > lens-shell>cube select sample_dim.name, sample_dim2.name from sample_dim join > sample_dim2 on sample_dim.d2id=sample_dim2.id ORDER BY sample_dim.name DESC > 29 Feb 2016 13:08:38 [Spring Shell] INFO cliLogger - Executing query with > timeout of 1000 milliseconds > 29 Feb 2016 13:08:47 [Spring Shell] INFO cliLogger - Couldn't complete query > execution within timeout. Waiting for completion > 29 Feb 2016 13:08:47 [Spring Shell] INFO cliLogger - Query handle: > b071a81f-eea3-4da2-a0fd-451623dc23af > 29 Feb 2016 13:08:47 [Spring Shell] INFO cliLogger - User query: 'cube > select sample_dim.name, sample_dim2.name from sample_dim join sample_dim2 on > sample_dim.d2id=sample_dim2.id ORDER BY sample_dim.name DESC' was submitted > to hive/hive1 > 29 Feb 2016 13:08:47 [Spring Shell] INFO cliLogger - Driver query: 'SELECT > ( sample_dim . name ), ( sample_dim2 . name ) FROM db2.local_dim_table > sample_dim inner JOIN db2.local_dim_table2 sample_dim2 ON (( sample_dim . > d2id ) = ( sample_dim2 . id )) AND ((sample_dim.dt = 'latest')) AND > ((sample_dim2.dt = 'latest')) ORDER BY sample_dim . name desc' and Driver > handle: OperationHandle [opType=EXECUTE_STATEMENT, > getHandleIdentifier()=58a8efb4-08cf-4e3e-a279-87125bec9444] > 29 Feb 2016 13:08:47 [Spring Shell] INFO cliLogger - Query Status:Progress: > 1.0 > Status: SUCCESSFUL > Status Message: Query is successful! > Is Result Set Available: true > > sample_dim.name sample_dim2.name > Results of query stored at : > file:/tmp/lensreports/b071a81f-eea3-4da2-a0fd-451623dc23af.csv 7 rows > processed in (9) seconds. > > > lens-shell>set lens.query.enable.persistent.resultset.indriver=true > lens-shell>set lens.query.enable.persistent.resultset=true > lens-shell>cube select sample_dim.name, sample_dim2.name from sample_dim join > sample_dim2 on sample_dim.d2id=sample_dim2.id ORDER BY sample_dim.name DESC > 29 Feb 2016 13:06:37 [Spring Shell] INFO cliLogger - Executing query with > timeout of 10000 milliseconds > sample_dim.name sample_dim2.name > Results of query stored at : > file:/tmp/lensreports/ce0bcc86-bbb2-4a1c-b457-3eac18ca7cfb.csv 7 rows > processed in (8) seconds. > lens-shell>set lens.cli.query.execute.timeout.millis=1000 > Client side Set lens.cli.query.execute.timeout.millis=1000 > lens-shell>cube select sample_dim.name, sample_dim2.name from sample_dim join > sample_dim2 on sample_dim.d2id=sample_dim2.id ORDER BY sample_dim.name DESC > 29 Feb 2016 13:07:05 [Spring Shell] INFO cliLogger - Executing query with > timeout of 1000 milliseconds > 29 Feb 2016 13:07:15 [Spring Shell] INFO cliLogger - Couldn't complete query > execution within timeout. Waiting for completion > 29 Feb 2016 13:07:15 [Spring Shell] INFO cliLogger - Query handle: > 1eb498e5-13c5-454f-a093-0fc08171d539 > 29 Feb 2016 13:07:15 [Spring Shell] INFO cliLogger - User query: 'cube > select sample_dim.name, sample_dim2.name from sample_dim join sample_dim2 on > sample_dim.d2id=sample_dim2.id ORDER BY sample_dim.name DESC' was submitted > to hive/hive1 > 29 Feb 2016 13:07:15 [Spring Shell] INFO cliLogger - Driver query: 'INSERT > OVERWRITE DIRECTORY > "file:/tmp/lensreports/hdfsout/1eb498e5-13c5-454f-a093-0fc08171d539" SELECT > ( sample_dim . name ), ( sample_dim2 . name ) FROM db2.local_dim_table > sample_dim inner JOIN db2.local_dim_table2 sample_dim2 ON (( sample_dim . > d2id ) = ( sample_dim2 . id )) AND ((sample_dim.dt = 'latest')) AND > ((sample_dim2.dt = 'latest')) ORDER BY sample_dim . name desc ' and Driver > handle: OperationHandle [opType=EXECUTE_STATEMENT, > getHandleIdentifier()=e28bb36f-7243-4568-8b2f-3dfbbf34e6cf] > sample_dim.name sample_dim2.name > Results of query stored at : > file:/tmp/lensreports/1eb498e5-13c5-454f-a093-0fc08171d539.csv 7 rows > processed in (9) seconds. > > > Thanks, > > Puneet Gupta > >
