> On Dec. 21, 2018, 6:10 a.m., Rajitha R wrote:
> > lens-server-api/src/main/java/org/apache/lens/server/api/SessionValidator.java
> > Lines 31 (patched)
> > <https://reviews.apache.org/r/69554/diff/5/?file=2114506#file2114506line31>
> >
> >     Can we not retain the same name validateSession?

changed name


> On Dec. 21, 2018, 6:10 a.m., Rajitha R wrote:
> > lens-server/src/main/java/org/apache/lens/server/BaseLensService.java
> > Line 392 (original), 401 (patched)
> > <https://reviews.apache.org/r/69554/diff/5/?file=2114508#file2114508line401>
> >
> >     validatesession not being called here

getsession is internal call. We have to validate user session in mysql only for 
user requests and not for internal session calls. There wont be a scenario 
where getsession is called and session is not present in memory because it will 
be created itself on user's first request only. Also on other hand if we try to 
call validatesession here then it will be called for lot of time in query 
prep/execute/submission phases. Not an good idea. That could be the reason it 
was not in place for older implementation as well.


> On Dec. 21, 2018, 6:10 a.m., Rajitha R wrote:
> > lens-server/src/main/java/org/apache/lens/server/BaseLensService.java
> > Lines 607 (patched)
> > <https://reviews.apache.org/r/69554/diff/5/?file=2114508#file2114508line608>
> >
> >     Any particular reason we are throwing e here and not daoE? May be throw 
> > e should be outside the if condition.

need to throw exception on condition here, corrected.


> On Dec. 21, 2018, 6:10 a.m., Rajitha R wrote:
> > lens-server/src/main/java/org/apache/lens/server/BaseLensService.java
> > Lines 611 (patched)
> > <https://reviews.apache.org/r/69554/diff/5/?file=2114508#file2114508line612>
> >
> >     can validateSession and validateSessionId api's be swapped here such 
> > that no changes will be required where validateSession api is called?

changed name


> On Dec. 21, 2018, 6:10 a.m., Rajitha R wrote:
> > lens-server/src/main/java/org/apache/lens/server/query/QueryExecutionServiceImpl.java
> > Lines 1175 (patched)
> > <https://reviews.apache.org/r/69554/diff/5/?file=2114512#file2114512line1175>
> >
> >     I am afraid if these continuous db updates will slow down the system

This update happens 3-4 times in query lifecycle, once for each state change 
(SUBMITTED, QUEUED, RUNNING, FAILED). Considering query execution time this 
should not slow down the system.


> On Dec. 21, 2018, 6:10 a.m., Rajitha R wrote:
> > lens-server/src/main/java/org/apache/lens/server/query/QueryExecutionServiceImpl.java
> > Line 1541 (original), 1566 (patched)
> > <https://reviews.apache.org/r/69554/diff/5/?file=2114512#file2114512line1566>
> >
> >     why is db update statement NOT called outside the switch statement and 
> > only in EXECUTED case ?

it has to be for all, moved.


> On Dec. 21, 2018, 6:10 a.m., Rajitha R wrote:
> > lens-server/src/main/java/org/apache/lens/server/session/HiveSessionService.java
> > Lines 538 (patched)
> > <https://reviews.apache.org/r/69554/diff/5/?file=2114515#file2114515line540>
> >
> >     Why is this required? Can we not implement these counters in the 
> > existing way by calling notifyevent?

I cant see any event implementation for metric service in  code also we are 
using metric service directly to handle metric counters at all places in code. 
Correct me if I am wrong.


- Ankit


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/69554/#review211463
-----------------------------------------------------------


On Jan. 8, 2019, 10:52 a.m., Ankit Kailaswar wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/69554/
> -----------------------------------------------------------
> 
> (Updated Jan. 8, 2019, 10:52 a.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/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/session/HiveSessionService.java
>  f6d43d70 
>   lens-server/src/test/java/org/apache/lens/server/TestBaseLensService.java 
> PRE-CREATION 
>   lens-server/src/test/java/org/apache/lens/server/query/TestLensDAO.java 
> 066525b7 
> 
> 
> Diff: https://reviews.apache.org/r/69554/diff/7/
> 
> 
> Testing
> -------
> 
> Unit testing
> 
> 
> Thanks,
> 
> Ankit Kailaswar
> 
>

Reply via email to