> 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 > >
