----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/69554/#review211463 -----------------------------------------------------------
lens-server-api/src/main/java/org/apache/lens/server/api/SessionValidator.java Lines 31 (patched) <https://reviews.apache.org/r/69554/#comment296617> Can we not retain the same name validateSession? lens-server/src/main/java/org/apache/lens/server/BaseLensService.java Line 392 (original), 401 (patched) <https://reviews.apache.org/r/69554/#comment296673> validatesession not being called here lens-server/src/main/java/org/apache/lens/server/BaseLensService.java Lines 604 (patched) <https://reviews.apache.org/r/69554/#comment296669> possibility of npe lens-server/src/main/java/org/apache/lens/server/BaseLensService.java Lines 604 (patched) <https://reviews.apache.org/r/69554/#comment296670> no null check on persistinfo, possibility of npe lens-server/src/main/java/org/apache/lens/server/BaseLensService.java Lines 607 (patched) <https://reviews.apache.org/r/69554/#comment296671> Any particular reason we are throwing e here and not daoE? May be throw e should be outside the if condition. lens-server/src/main/java/org/apache/lens/server/BaseLensService.java Lines 611 (patched) <https://reviews.apache.org/r/69554/#comment296668> can validateSession and validateSessionId api's be swapped here such that no changes will be required where validateSession api is called? lens-server/src/main/java/org/apache/lens/server/query/LensServerDAO.java Lines 501 (patched) <https://reviews.apache.org/r/69554/#comment296676> Can we have same naming convention for better clarity? we can have insertActionSession like insertActiveQuery instead of insertActionSessions , same applies to update lens-server/src/main/java/org/apache/lens/server/query/LensServerDAO.java Lines 701 (patched) <https://reviews.apache.org/r/69554/#comment296618> These should be moved to final block? lens-server/src/main/java/org/apache/lens/server/query/QueryExecutionServiceImpl.java Lines 142 (patched) <https://reviews.apache.org/r/69554/#comment296621> can we have a more intuitive name like? ACTIVE_QUERY_INSERT_ERROR_COUNTER lens-server/src/main/java/org/apache/lens/server/query/QueryExecutionServiceImpl.java Lines 1175 (patched) <https://reviews.apache.org/r/69554/#comment296672> I am afraid if these continuous db updates will slow down the system lens-server/src/main/java/org/apache/lens/server/query/QueryExecutionServiceImpl.java Line 1541 (original), 1566 (patched) <https://reviews.apache.org/r/69554/#comment296620> why is db update statement NOT called outside the switch statement and only in EXECUTED case ? lens-server/src/main/java/org/apache/lens/server/session/HiveSessionService.java Lines 429 (patched) <https://reviews.apache.org/r/69554/#comment296674> can this be reverted? lens-server/src/main/java/org/apache/lens/server/session/HiveSessionService.java Lines 538 (patched) <https://reviews.apache.org/r/69554/#comment296675> Why is this required? Can we not implement these counters in the existing way by calling notifyevent? - Rajitha R On Dec. 18, 2018, 12:20 p.m., Ankit Kailaswar wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/69554/ > ----------------------------------------------------------- > > (Updated Dec. 18, 2018, 12:20 p.m.) > > > Review request for lens, Amareshwari Sriramadasu and Rajitha R. > > > Bugs: LENS-1538 > https://issues.apache.org/jira/browse/LENS-1538 > > > Repository: lens > > > Description > ------- > > Implementation > > Session > > We need to persist all client session in database. > This involve persisting client session’s id and configuration in mysql > server. persisting entire session object is not required. We need to do this > as first thing while creating user session. > For each request from client lens server should not validate session id > from its in memory map. It should rather validate it from persisted session > in mysql server. > > if > > session is present in mysql and not present in lens server’s in memory > map(session manager) then it should create a lens session with same session > id and add it to in memory map > > else If > > session is not present in mysql and present in server’s in memory map then it > signifies that user have initiated close session from other host and we will > need to remove this session from current host’s in memory map as well > > else if > > session is not present in mysql and in server’s in memory map then return > “invalid session”. > > Whenever session expired/closed then we need to remove this session from > lens server’s in memory map and mysql. > > > > Query > > For sync queries lens server will close connection as soon as it is > stopped or failed with appropriate failure message. Client will be retrying > the query. > For async-light queries and async-heavy queries lens server takes care of > rescheduling all queries which were in running or queued state at the time of > restart. we can use this to make query ids available with both servers. > For async queries we need to persist all non finished async user queries > in mysql server in new table current_query. > If user executes query on host1 and it goes down and nginx points to > host2 then user should be able to poll on query status. Lens server will > first check query id in its inmemory maps if it is not present then it will > check in “finished_query” table in mysql else in “current_query” table in > mysql. In this case we will continue showing old status of query since it is > scheduled by host1. As soon as host1 comes up it will reschedule these > queries and will change the status. Optionally we can have host2 to move > these queries in “allqueries” map of its query service which will take care > of recovering, in this case we don't need to wait for host1 to come up and > reschedule. > While moving query from ”finished_quey” map to “finished_query” table in > mysql we will need to remove it from “current_query” tables as well. > This flow will remain same for request ids/query ids generated for query > plan/estimate. > > > > Issue addressed > > Lens downtime will be zero in case of deployment and if primary fail. > User will not have to create a new session whenever switch happens > User will be able to get status of all async queries seamlessly. > > > Diffs > ----- > > > lens-server-api/src/main/java/org/apache/lens/server/api/SessionValidator.java > 65403191 > > lens-server-api/src/main/java/org/apache/lens/server/api/events/AsyncEventListener.java > 94734658 > lens-server/src/main/java/org/apache/lens/server/BaseLensService.java > 9364872d > > lens-server/src/main/java/org/apache/lens/server/error/LensServerErrorCode.java > ef43371a > > lens-server/src/main/java/org/apache/lens/server/metastore/MetastoreResource.java > cb29c9ea > lens-server/src/main/java/org/apache/lens/server/query/LensServerDAO.java > cc6ca7d4 > > lens-server/src/main/java/org/apache/lens/server/query/QueryExecutionServiceImpl.java > 07a2107a > > lens-server/src/main/java/org/apache/lens/server/query/QueryServiceResource.java > 47b40a8f > > lens-server/src/main/java/org/apache/lens/server/scheduler/ScheduleResource.java > 1d5959b2 > > lens-server/src/main/java/org/apache/lens/server/session/HiveSessionService.java > f6d43d70 > > > Diff: https://reviews.apache.org/r/69554/diff/5/ > > > Testing > ------- > > Unit testing > > > Thanks, > > Ankit Kailaswar > >
