----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/39842/#review105437 -----------------------------------------------------------
Overall changes looked clean and nice. I really enjoyed reviewing the changes :) Looking forward for the updated patch with documentation and testcases added. lens-driver-es/src/test/java/org/apache/lens/driver/es/ESDriverTest.java (line 38) <https://reviews.apache.org/r/39842/#comment164038> Will it be possible to define driver types as constant strings or enum? - Amareshwari Sriramadasu On Nov. 2, 2015, 4:08 a.m., Puneet Gupta wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/39842/ > ----------------------------------------------------------- > > (Updated Nov. 2, 2015, 4:08 a.m.) > > > Review request for lens, Amareshwari Sriramadasu and Rajat Khandelwal. > > > Bugs: LENS-123 > https://issues.apache.org/jira/browse/LENS-123 > > > Repository: lens > > > Description > ------- > > Ability to load multiple drivers on lens server. > > As of now only one driver instance of each type (hive,jdbc,es,etc) can be > loaded by lens server. Hence lens can not support for example, two jdbc > instances, one for MySql and one for Vertica or just two different MySQl > deployments. This can be a big limitation for some deployments. > > Made below changes in fix > 1. Based on the approaches discussed in JIRA(LENS-123), the below folder > structure was picked. (Please refer to the discussion on JIRA if required - > last few comments ) > -conf > --drivers > ---hive > ----driver1 > -----hivedriver-site.xml > ----driver2 > -----hivedriver-site.xml > ---jdbc > ----driver1 > -----jdbcdriver-site.xml > ----driver2 > -----jdbcdriver-site.xml > Note: drivers configuration is read from "drivers" directory under conf > location. Conf loaction is set while starting the server as -Dconfig.location > = "<conf path>" > > 2. Added an abstract class > "org.apache.lens.server.api.driver.AbstractLensDriver". This has common > implemenation for getting lens driver's fully qualified name and some methods > for getting driver specific resource paths. All existing driver > implementations extend this class. In future, we can also add common > functionality here(if required) without affecting existing driver > implementations that extend this abstract class. > > 3. Updated driver interface > -configure method now takes driver name and type configuration parameters. > -added getFullyQualifiedName() method to identify the driver uniquely. > > 4. Value of lens.server.drivers property in lens-site.xml (and > default-lens-site) updated to include driver type and driver class name > (earlier only driver class name was present). > <value>hive:org.apache.lens.driver.hive.HiveDriver,jdbc:org.apache.lens.driver.jdbc.JDBCDriver</value> > > 5. Updated LensServerDAO to use driver qualified name instead of driver class > name. The table column name is chnaged to drivername form driverclass > > 6. The driver instance was earlier referred to by its class name > (Example:org.apache.lens.driver.hive.HiveDriver) everywhere in code. This is > now changed to driver's fully qualified name which has the driver type and > driver name (Example :hive/hive1) > > 7. All metrics were also relying on driver class name. This is also changed > to driver's fully qualified name. > > 8. Updated driver folder struture for test cases and lens server build (as > per point1). > > 9. Query execution service will fail to start if no drivers are found. (This > is a change from previous flow . We can discuss this if req.) > > > Pending ( will upload this soon) > 1. Adding test cases to test multiple drivers > 2. updating documentation > > Future Enhancements(that can be taken up as a separate JIRA) > 1.Multiple versions of a datasource to be supported ( say Hive and Hive on > spark use different versions of Hive. This will not work as of now as we have > common Hive jars as part of war classpath. We need to have driver specific > jars which can have different version. For this, the jars and other hive > version specific resources should be picked from driver's folder by a > separate class loader). > > > Diffs > ----- > > lens-api/src/main/java/org/apache/lens/api/query/LensPreparedQuery.java > 9595ce9 > lens-api/src/main/java/org/apache/lens/api/query/LensQuery.java 204ecee > lens-cli/src/main/java/org/apache/lens/cli/commands/LensQueryCommands.java > 096fd7a > lens-cli/src/test/resources/drivers/hive/hive1/hivedriver-site.xml > PRE-CREATION > lens-client/src/main/java/org/apache/lens/client/LensStatement.java 0a511f0 > lens-client/src/test/resources/drivers/hive/hive1/hivedriver-site.xml > PRE-CREATION > > lens-cube/src/test/java/org/apache/lens/driver/cube/TestMinCostSelector.java > 72f1497 > lens-driver-es/src/main/java/org/apache/lens/driver/es/ESDriver.java > 14d9f99 > lens-driver-es/src/test/java/org/apache/lens/driver/es/ESDriverTest.java > f453416 > lens-driver-hive/src/main/java/org/apache/lens/driver/hive/HiveDriver.java > c96ef20 > > lens-driver-hive/src/test/java/org/apache/lens/driver/hive/TestHiveDriver.java > fc57c94 > > lens-driver-hive/src/test/java/org/apache/lens/driver/hive/TestRemoteHiveDriver.java > 98edc28 > lens-driver-hive/src/test/resources/drivers/hive/hive1/hivedriver-site.xml > PRE-CREATION > lens-driver-hive/src/test/resources/hivedriver-site.xml 613938d > lens-driver-jdbc/src/main/java/org/apache/lens/driver/jdbc/JDBCDriver.java > a8b980f > > lens-driver-jdbc/src/test/java/org/apache/lens/driver/jdbc/TestColumnarSQLRewriter.java > 7772d16 > > lens-driver-jdbc/src/test/java/org/apache/lens/driver/jdbc/TestJDBCFinal.java > 053e20d > > lens-driver-jdbc/src/test/java/org/apache/lens/driver/jdbc/TestJdbcDriver.java > 425bd6f > lens-driver-jdbc/src/test/resources/drivers/jdbc/jdbc1/jdbcdriver-site.xml > PRE-CREATION > lens-driver-jdbc/src/test/resources/jdbcdriver-site.xml 1202074 > lens-ml-lib/src/test/resources/drivers/hive/hive1/hivedriver-site.xml > PRE-CREATION > lens-ml-lib/src/test/resources/lens-site.xml 3d5dbef > > lens-query-lib/src/test/java/org/apache/lens/lib/query/TestAbstractFileFormatter.java > 40e1cdc > > lens-server-api/src/main/java/org/apache/lens/server/api/LensConfConstants.java > 7ee0749 > > lens-server-api/src/main/java/org/apache/lens/server/api/driver/AbstractLensDriver.java > PRE-CREATION > > lens-server-api/src/main/java/org/apache/lens/server/api/driver/LensDriver.java > a5a60d7 > > lens-server-api/src/main/java/org/apache/lens/server/api/query/AbstractQueryContext.java > 0c980a2 > > lens-server-api/src/main/java/org/apache/lens/server/api/query/DriverSelectorQueryContext.java > feac938 > > lens-server-api/src/main/java/org/apache/lens/server/api/query/FinishedLensQuery.java > 89053aa > > lens-server-api/src/main/java/org/apache/lens/server/api/query/PreparedQueryContext.java > b6f669b > > lens-server-api/src/main/java/org/apache/lens/server/api/query/QueryContext.java > 9b491d1 > > lens-server-api/src/test/java/org/apache/lens/server/api/driver/MockDriver.java > 2d86589 > > lens-server-api/src/test/java/org/apache/lens/server/api/query/MockQueryContext.java > fd6b560 > > lens-server-api/src/test/java/org/apache/lens/server/api/query/TestAbstractQueryContext.java > e41f2f4 > lens-server/src/main/java/org/apache/lens/server/query/LensServerDAO.java > b9dd286 > > lens-server/src/main/java/org/apache/lens/server/query/QueryExecutionServiceImpl.java > 4d8ae51 > lens-server/src/main/java/org/apache/lens/server/rewrite/RewriteUtil.java > 6c464fb > lens-server/src/main/resources/lensserver-default.xml 5f268cb > lens-server/src/test/java/org/apache/lens/server/LensJerseyTest.java > 3dad050 > > lens-server/src/test/java/org/apache/lens/server/query/TestEventService.java > 1dab35e > lens-server/src/test/java/org/apache/lens/server/query/TestLensDAO.java > bc1463f > > lens-server/src/test/java/org/apache/lens/server/query/TestQueryService.java > c37b0ed > lens-server/src/test/java/org/apache/lens/server/rewrite/TestRewriting.java > 2827b96 > lens-server/src/test/resources/drivers/hive/hive1/hivedriver-site.xml > PRE-CREATION > lens-server/src/test/resources/drivers/jdbc/jdbc1/jdbcdriver-site.xml > PRE-CREATION > > lens-server/src/test/resources/drivers/mock/fail1/failing-query-driver-site.xml > PRE-CREATION > lens-server/src/test/resources/failing-query-driver-site.xml fee022d > lens-server/src/test/resources/hivedriver-site.xml f2aed88 > lens-server/src/test/resources/jdbcdriver-site.xml 1b14f54 > lens-server/src/test/resources/lens-site.xml cc887ef > src/site/apt/admin/config.apt 88c1489 > tools/conf-pseudo-distr/server/drivers/hive/hive1/hivedriver-site.xml > PRE-CREATION > tools/conf-pseudo-distr/server/drivers/jdbc/jdbc1/jdbcdriver-site.xml > PRE-CREATION > tools/conf-pseudo-distr/server/hivedriver-site.xml 4804356 > tools/conf-pseudo-distr/server/jdbcdriver-site.xml 37540dd > tools/conf-pseudo-distr/server/lens-site.xml f43d07e > tools/conf/server/drivers/hive/hive1/hivedriver-site.xml PRE-CREATION > tools/conf/server/drivers/jdbc/jdbc1/jdbcdriver-site.xml PRE-CREATION > tools/conf/server/hivedriver-site.xml 2e8e7fa > tools/conf/server/jdbcdriver-site.xml 37540dd > tools/conf/server/lens-site.xml 2b12b83 > > Diff: https://reviews.apache.org/r/39842/diff/ > > > Testing > ------- > > [INFO] Reactor Summary: > [INFO] > [INFO] Lens Checkstyle Rules ............................. SUCCESS [2.090s] > [INFO] Lens .............................................. SUCCESS [2.900s] > [INFO] Lens API .......................................... SUCCESS [25.172s] > [INFO] Lens API for server and extensions ................ SUCCESS [24.540s] > [INFO] Lens Cube ......................................... SUCCESS [5:17.973s] > [INFO] Lens DB storage ................................... SUCCESS [20.026s] > [INFO] Lens Query Library ................................ SUCCESS [15.835s] > [INFO] Lens Hive Driver .................................. SUCCESS [2:54.352s] > [INFO] Lens Driver for JDBC .............................. SUCCESS [39.054s] > [INFO] Lens Elastic Search Driver ........................ SUCCESS [19.714s] > [INFO] Lens Server ....................................... SUCCESS [8:17.418s] > [INFO] Lens client ....................................... SUCCESS [38.221s] > [INFO] Lens CLI .......................................... SUCCESS [56.723s] > [INFO] Lens Examples ..................................... SUCCESS [9.847s] > [INFO] Lens Ship Jars to Distributed Cache ............... SUCCESS [1.488s] > [INFO] Lens Distribution ................................. SUCCESS [9.897s] > [INFO] Lens ML Lib ....................................... SUCCESS [1:22.714s] > [INFO] Lens ML Ext Distribution .......................... SUCCESS [1.841s] > [INFO] Lens Regression ................................... SUCCESS [12.622s] > [INFO] Lens UI ........................................... SUCCESS [26.741s] > [INFO] > ------------------------------------------------------------------------ > [INFO] BUILD SUCCESS > [INFO] > ------------------------------------------------------------------------ > [INFO] Total time: 23:00.146s > [INFO] Finished at: Mon Nov 02 03:14:06 UTC 2015 > [INFO] Final Memory: 198M/2037M > [INFO] > ------------------------------------------------------------------------ > > Tested lens-examples also after making the changes. > > > Thanks, > > Puneet Gupta > >
