----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/38617/#review99980 -----------------------------------------------------------
lens-query-lib/src/main/java/org/apache/lens/lib/query/AbstractOutputFormatter.java (line 115) <https://reviews.apache.org/r/38617/#comment157038> Should we use lombok annonations to generate getters? lens-query-lib/src/main/java/org/apache/lens/lib/query/AbstractOutputFormatter.java (line 236) <https://reviews.apache.org/r/38617/#comment157041> Is it possible to add a Test Case to check this Error Scenario in Future? One hack/easy way is to seralize outputformatter in test case ,then read it back and then try to create instance of LensPersistentResult(LensResultSetMetadata, String, Integer, Long)using the de-serailized information. If LensPersistentResult constructor changes in future to take few more params,the test case will fail/not compile to make the developer realize that serailization flow is broken and added params need to be serailized to. This may not be very effective if QueryExecutionServiceImpl.getResultset(QueryHandle) code changes in future. Another way can be to test the exact scenario(Cannot retrieve query results on lens server restart), which may be more effort . lens-query-lib/src/main/java/org/apache/lens/lib/query/WrappedFileFormatter.java (line 104) <https://reviews.apache.org/r/38617/#comment157044> Is this done to take care of serailization ( as the serialization code is in AbstractOutputFormatter)? Can we instead override readExternal and writeExtrenal? lens-server-api/src/main/java/org/apache/lens/server/api/driver/LensResultSetMetadata.java (line 52) <https://reviews.apache.org/r/38617/#comment157046> Duplicate code in QueryExecutionServiceImpl.initalizeFinishedQueryStore(Configuration). Can we remobve this code form QueryExecutionServiceImpl since its better encapsulated in this calss. lens-server-api/src/main/java/org/apache/lens/server/api/driver/LensResultSetMetadata.java (line 124) <https://reviews.apache.org/r/38617/#comment157047> +1 lens-server-api/src/main/java/org/apache/lens/server/api/driver/LensResultSetMetadata.java (line 128) <https://reviews.apache.org/r/38617/#comment157048> +1 lens-server/src/main/java/org/apache/lens/server/query/QueryExecutionServiceImpl.java (line 2452) <https://reviews.apache.org/r/38617/#comment157037> Insted of calling writeExternal() directly (I see its done in many other places - all services, driver,etc.... not sure if that is intended for some special handling), we can try to plugin in natural serailization flow. In this case we can make QueryContext.queryOutputFormatter Non Transient. The seralization and de-serailization will be handled automatically (as in QueryOutputFormatter.writeExternal() will be called) when QueryContext is serailized. - Puneet Gupta On Sept. 22, 2015, 9:46 a.m., Deepak Barr wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/38617/ > ----------------------------------------------------------- > > (Updated Sept. 22, 2015, 9:46 a.m.) > > > Review request for lens. > > > Repository: lens > > > Description > ------- > > The fix involves serializing QueryOutputFormatter > > > Diffs > ----- > > > lens-query-lib/src/main/java/org/apache/lens/lib/query/AbstractFileFormatter.java > ae5af03 > > lens-query-lib/src/main/java/org/apache/lens/lib/query/AbstractOutputFormatter.java > 06f37ee > > lens-query-lib/src/main/java/org/apache/lens/lib/query/WrappedFileFormatter.java > e28c17b > > lens-server-api/src/main/java/org/apache/lens/server/api/driver/LensResultSetMetadata.java > ef8aeed > > lens-server-api/src/main/java/org/apache/lens/server/api/query/QueryOutputFormatter.java > 0a6cc6b > > lens-server/src/main/java/org/apache/lens/server/query/QueryExecutionServiceImpl.java > 9e27dd4 > > Diff: https://reviews.apache.org/r/38617/diff/ > > > Testing > ------- > > Yes. Tested with all cases of lens persisted,driver persisted and inmemory > result sets. > > > [INFO] Reactor Summary: > [INFO] > [INFO] Lens Checkstyle Rules .............................. SUCCESS [ 3.019 > s] > [INFO] Lens ............................................... SUCCESS [ 4.462 > s] > [INFO] Lens API ........................................... SUCCESS [ 29.054 > s] > [INFO] Lens API for server and extensions ................. SUCCESS [ 25.465 > s] > [INFO] Lens Cube .......................................... SUCCESS [07:00 > min] > [INFO] Lens DB storage .................................... SUCCESS [ 30.185 > s] > [INFO] Lens Query Library ................................. SUCCESS [ 21.199 > s] > [INFO] Lens Hive Driver ................................... SUCCESS [04:09 > min] > [INFO] Lens Driver for JDBC ............................... SUCCESS [ 51.156 > s] > [INFO] Lens Elastic Search Driver ......................... SUCCESS [ 22.259 > s] > [INFO] Lens Server ........................................ SUCCESS [08:06 > min] > [INFO] Lens client ........................................ SUCCESS [ 47.492 > s] > [INFO] Lens CLI ........................................... SUCCESS [03:46 > min] > [INFO] Lens Examples ...................................... SUCCESS [ 12.575 > s] > [INFO] Lens Distribution .................................. SUCCESS [ 12.408 > s] > [INFO] Lens ML Lib ........................................ SUCCESS [02:06 > min] > [INFO] Lens ML Ext Distribution ........................... SUCCESS [ 3.021 > s] > [INFO] Lens Regression .................................... SUCCESS [ 15.657 > s] > [INFO] > ------------------------------------------------------------------------ > [INFO] BUILD SUCCESS > [INFO] > ------------------------------------------------------------------------ > [INFO] Total time: 29:48 min > [INFO] Finished at: 2015-09-22T14:45:48+05:30 > [INFO] Final Memory: 228M/3007M > [INFO] > ------------------------------------------------------------------------ > > > Thanks, > > Deepak Barr > >
