Re: Review Request 39576: LENS-833 : Limit number of open sessions per user on session service

[Raju Bairishetti <ra...@gmail.com>]

> On Nov. 6, 2015, 6:08 a.m., Puneet Gupta wrote:
> > lens-server/src/main/java/org/apache/lens/server/BaseLensService.java, lines 120-122
> > <https://reviews.apache.org/r/39576/diff/3/?file=1113532#file1113532line120>
> >
> >     There might be another thread updating the session count for the same user at the same time. Should we do a strict check (atleast at user level) ?
> 
> Raju Bairishetti wrote:
>     Seems this is already threadsafe as get() call is thread safe as we are using concurrent hash map. Not updating any session counts in this method.

There are some race codnition issues if the following operations are not in atomic operation(i.e. not in single syncronized block).

1) Check if user can eligible to open a session
2) Open a new session
3) Update session count for user

All the three operations should be done in a single step(i.e. in synchronized block) to make sure only one thread gets access to session count of user.

Two approaches to solve this issue:
1) We can take lock on SESSIONS_PER_USER (i.e. syncronized(SESSIONS_PER_USER)). But taking lock on entire map is not a good idea as other user threads also will get blocked. Lock/syncronization should be within user level
2) Right now, User is a String instance. Syncronizing on **String** object is not a good idea. We can create a user instance and syncronize on user instance  Let to fix it.

I am thinking to go ahead with second approach. Please let me know your suggestions/concerns


- Raju


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


On Dec. 29, 2015, 11:50 p.m., Raju Bairishetti wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/39576/
> -----------------------------------------------------------
> 
> (Updated Dec. 29, 2015, 11:50 p.m.)
> 
> 
> Review request for lens, Amareshwari Sriramadasu and Rajat Khandelwal.
> 
> 
> Bugs: LENS-833
>     https://issues.apache.org/jira/browse/LENS-833
> 
> 
> Repository: lens
> 
> 
> Description
> -------
> 
> Failing the open session operation if user creates more sessions than configured limit.
> 
> 
> Diffs
> -----
> 
>   lens-api/src/main/java/org/apache/lens/api/error/ErrorCollectionFactory.java 741630b 
>   lens-api/src/main/resources/lens-errors.conf 06960a0 
>   lens-cli/src/test/java/org/apache/lens/cli/TestLensConnectionCliCommands.java 558e97f 
>   lens-cli/src/test/java/org/apache/lens/cli/TestLensCubeCommands.java 43d0722 
>   lens-cli/src/test/java/org/apache/lens/cli/TestLensDatabaseCommands.java 32ed7b0 
>   lens-cli/src/test/java/org/apache/lens/cli/TestLensDimensionCommands.java 42c6bae 
>   lens-cli/src/test/java/org/apache/lens/cli/TestLensDimensionTableCommands.java 30f4ec1 
>   lens-cli/src/test/java/org/apache/lens/cli/TestLensFactCommands.java 448d0f6 
>   lens-cli/src/test/java/org/apache/lens/cli/TestLensFactCommandsWithMissingWeight.java 9fce233 
>   lens-cli/src/test/java/org/apache/lens/cli/TestLensLogResourceCommands.java f4b043e 
>   lens-cli/src/test/java/org/apache/lens/cli/TestLensNativeTableCommands.java d453803 
>   lens-cli/src/test/java/org/apache/lens/cli/TestLensStorageCommands.java 8bccac2 
>   lens-server-api/src/main/java/org/apache/lens/server/api/LensConfConstants.java 88e5a01 
>   lens-server/src/main/java/org/apache/lens/server/BaseLensService.java 0821fe7 
>   lens-server/src/main/java/org/apache/lens/server/error/LensServerErrorCode.java dc20f0f 
>   lens-server/src/main/java/org/apache/lens/server/session/SessionResource.java 3ba5edd 
>   lens-server/src/main/resources/lensserver-default.xml cac641a 
>   lens-server/src/test/java/org/apache/lens/server/TestServerMode.java 75f21e1 
>   lens-server/src/test/java/org/apache/lens/server/auth/FooBarAuthenticationProvider.java 8e22837 
>   lens-server/src/test/java/org/apache/lens/server/metastore/TestMetastoreService.java e0c0923 
>   lens-server/src/test/java/org/apache/lens/server/query/QueryAPIErrorResponseTest.java 69c3f46 
>   lens-server/src/test/java/org/apache/lens/server/query/TestLensDAO.java 01e846a 
>   lens-server/src/test/java/org/apache/lens/server/session/TestSessionResource.java 3055ce5 
>   lens-server/src/test/java/org/apache/lens/server/ui/TestSessionUIResource.java 6f7c216 
>   lens-server/src/test/resources/lens-site.xml 9cb4a6f 
>   src/site/apt/admin/config.apt 54f827e 
> 
> Diff: https://reviews.apache.org/r/39576/diff/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Raju Bairishetti
> 
>