----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/47174/#review133295 -----------------------------------------------------------
lens-api/src/main/resources/lens-errors.conf (line 29) <https://reviews.apache.org/r/47174/#comment197584> Can we make this more readable ? lens-server-api/src/main/java/org/apache/lens/server/api/SessionValidator.java (line 25) <https://reviews.apache.org/r/47174/#comment197585> TODO is still needed ? lens-server/src/main/java/org/apache/lens/server/BaseLensService.java (line 306) <https://reviews.apache.org/r/47174/#comment197586> We need to consider the sitution where a user has submitted 100 queries in 100 different sessions and closed session after each submission. If the session limit for user is 100, and all the queries are either queued/running, then he won't be able to open any more sessions, even though he had issues closed on each opened session. lens-server/src/main/java/org/apache/lens/server/BaseLensService.java (line 342) <https://reviews.apache.org/r/47174/#comment197587> Should we use "SESSION_ID_NOT_PROVIDED" ? lens-server/src/main/java/org/apache/lens/server/metastore/MetastoreResource.java (lines 65 - 67) <https://reviews.apache.org/r/47174/#comment197588> This may not be required. Check LENS-957 : Add GenericExceptionMapper to map all non LensException as well. lens-server/src/main/java/org/apache/lens/server/query/QueryExecutionServiceImpl.java (line 1313) <https://reviews.apache.org/r/47174/#comment197589> Do we need to add the logic to delete the previously formatted(partially/completely) result ,if any, in ResultFormatter? lens-server/src/main/java/org/apache/lens/server/query/QueryExecutionServiceImpl.java (line 2902) <https://reviews.apache.org/r/47174/#comment197593> Not sure why this method is here. This method is only used in test classes. Should we remove it and let test classes use SessionService to close session ? lens-server/src/main/java/org/apache/lens/server/query/QueryServiceResource.java (line 72) <https://reviews.apache.org/r/47174/#comment197591> This is not required after lens-957 lens-server/src/main/java/org/apache/lens/server/query/QueryServiceResource.java (line 76) <https://reviews.apache.org/r/47174/#comment197592> should we merge this with checkSessionId() ? lens-server/src/main/java/org/apache/lens/server/session/HiveSessionService.java (lines 493 - 498) <https://reviews.apache.org/r/47174/#comment197594> Should we move this code to BaseLensService#closeSession ? The If check will not be required then lens-server/src/main/java/org/apache/lens/server/session/LensSessionImpl.java (line 614) <https://reviews.apache.org/r/47174/#comment197595> Remove all system outs . lens-server/src/test/java/org/apache/lens/server/query/TestQueryIndependenceFromSession.java (line 58) <https://reviews.apache.org/r/47174/#comment197596> should we add close to name ? TestQueryIndependenceFromSessionClose lens-server/src/test/java/org/apache/lens/server/query/TestQueryIndependenceFromSession.java (line 173) <https://reviews.apache.org/r/47174/#comment197630> +1 for testing re-start secanrio - Puneet Gupta On May 15, 2016, 7:19 p.m., Rajat Khandelwal wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/47174/ > ----------------------------------------------------------- > > (Updated May 15, 2016, 7:19 p.m.) > > > Review request for lens. > > > Bugs: LENS-904 > https://issues.apache.org/jira/browse/LENS-904 > > > Repository: lens > > > Description > ------- > > In the current scenario, if the queries are queued from lens side (because of > throttling), then these queries fails on session close. > > > Diffs > ----- > > lens-api/src/main/resources/lens-errors.conf > 395d63b87b385607fbb0435bd99ab05b65ca51dd > lens-client/src/test/java/org/apache/lens/client/TestLensClient.java > 5cf7417eef469e9f1bfbd35fc0ecfbbe64733c8a > lens-driver-hive/src/main/java/org/apache/lens/driver/hive/HiveDriver.java > b85a66f2677c731b751d019c9e1b204bb0791114 > > lens-driver-hive/src/test/java/org/apache/lens/driver/hive/TestRemoteHiveDriver.java > 00d2a1c7565371c42d545156b9f8881a6174e080 > > lens-server-api/src/main/java/org/apache/lens/server/api/SessionValidator.java > PRE-CREATION > > lens-server-api/src/main/java/org/apache/lens/server/api/metastore/CubeMetastoreService.java > 431164f861bc25da624baf24a7cb7a295c019d70 > > lens-server-api/src/main/java/org/apache/lens/server/api/query/QueryExecutionService.java > 15ed2229dc21a730140efa5a7297d9c0329cabbc > lens-server/src/main/java/org/apache/lens/server/BaseLensService.java > 9f8ee7251789cd943e7c6a1091feb44684f248c8 > > lens-server/src/main/java/org/apache/lens/server/error/LensServerErrorCode.java > 2443fecea303ed963bfcd82071f4ca69ded46227 > > lens-server/src/main/java/org/apache/lens/server/metastore/MetastoreResource.java > 7395c832b8cbe6ccde811a8b76fc2e7f97d18c4e > > lens-server/src/main/java/org/apache/lens/server/query/QueryExecutionServiceImpl.java > c9f9b974aa2698f10f78b449d9f250447144a89c > > lens-server/src/main/java/org/apache/lens/server/query/QueryServiceResource.java > c70689b110462e9623e1e3b5d37af97270c673dc > > lens-server/src/main/java/org/apache/lens/server/session/HiveSessionService.java > 97d5f1684df62c227aac6da7f88168b860c4cb17 > > lens-server/src/main/java/org/apache/lens/server/session/LensSessionImpl.java > 88406c5f0de5da9833c1a1da5353fde0c3cc1c0e > lens-server/src/test/java/org/apache/lens/server/LensJerseyTest.java > b5d54829ce4c41145eda39702af9f26ed0958fde > > lens-server/src/test/java/org/apache/lens/server/query/TestQueryConstraints.java > 8493d8598adc07609be1ddf4de5734513db7b1eb > > lens-server/src/test/java/org/apache/lens/server/query/TestQueryIndependenceFromSession.java > PRE-CREATION > > lens-server/src/test/java/org/apache/lens/server/query/TestQueryService.java > 692a0a017e72089e0f78e570f39b954339c401a9 > > Diff: https://reviews.apache.org/r/47174/diff/ > > > Testing > ------- > > > Thanks, > > Rajat Khandelwal > >
