> On Jan. 9, 2016, 2:18 a.m., Amareshwari Sriramadasu wrote: > > lens-server/src/test/resources/lens-site.xml, line 192 > > <https://reviews.apache.org/r/39576/diff/5/?file=1181363#file1181363line192> > > > > Why 20? It should be lesser than the default as well.
Tests are failing with too many open sessions. Almost all of the opened sessions are getting clsoed in every unit test. Test failures were because of running multiple unit tests in parallel. Increased num sessions to 20 to avoid **timing issues** in test cases > On Jan. 9, 2016, 2:18 a.m., Amareshwari Sriramadasu wrote: > > lens-server/src/main/java/org/apache/lens/server/BaseLensService.java, > > lines 290-293 > > <https://reviews.apache.org/r/39576/diff/5/?file=1181352#file1181352line290> > > > > Not synchronized? Syncronizing inside the decrementSessionCountForUser() > On Jan. 9, 2016, 2:18 a.m., Amareshwari Sriramadasu wrote: > > lens-server/src/main/java/org/apache/lens/server/BaseLensService.java, > > lines 200-203 > > <https://reviews.apache.org/r/39576/diff/5/?file=1181352#file1181352line200> > > > > Not inside synchronized block ? It is in syncronized block. Syncronized on SessionUser instance. > On Jan. 9, 2016, 2:18 a.m., Amareshwari Sriramadasu wrote: > > lens-server/src/main/java/org/apache/lens/server/BaseLensService.java, line > > 108 > > <https://reviews.apache.org/r/39576/diff/5/?file=1181352#file1181352line108> > > > > Why is this class required? Added this class to get a user instance for acuiring lock on User instance while opening & closing sessions. I can remove the class and creting a new String instance with the user and store it in SESSION_USER_INSTANCE_MAP. ``` My earlier comment: 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 ``` > On Jan. 9, 2016, 2:18 a.m., Amareshwari Sriramadasu wrote: > > lens-cli/src/test/java/org/apache/lens/cli/TestLensFactCommandsWithMissingWeight.java, > > line 118 > > <https://reviews.apache.org/r/39576/diff/5/?file=1181347#file1181347line118> > > > > Arent both the commands using the same client? No. Commands are using two diffrent client instances. - Raju ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/39576/#review113594 ----------------------------------------------------------- On Jan. 5, 2016, 12:42 a.m., Raju Bairishetti wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/39576/ > ----------------------------------------------------------- > > (Updated Jan. 5, 2016, 12:42 a.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 > >
