> On May 13, 2016, 8:34 a.m., Jonathan Hurley wrote: > > contrib/views/utils/src/main/java/org/apache/ambari/view/utils/UserLocal.java, > > lines 87-89 > > <https://reviews.apache.org/r/47305/diff/1/?file=1381317#file1381317line87> > > > > Double-checked locking here; you'll need to make this volatile if > > you're keeping this code. > > Nitiraj Rathore wrote: > Do you mean to make viewSingletonObjects volatile? I don't think making > viewSingletonObjects as volatile is required here as we are not changing the > reference of viewSingletonObjects or assigning it new value. It is assigned > once statically. And we are using contains method to check for existence of > key inside that map. I would welcome any reason for making it volatile.
I took a look at the implementation for the CHMap and it does look like it will commit to memory correctly. So we can drop this. - Jonathan ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/47305/#review133079 ----------------------------------------------------------- On May 12, 2016, 10:04 a.m., Nitiraj Rathore wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/47305/ > ----------------------------------------------------------- > > (Updated May 12, 2016, 10:04 a.m.) > > > Review request for Ambari, DIPAYAN BHOWMICK, Jonathan Hurley, Mahadev Konar, > Rohit Choudhary, and Ashwin Rajeev. > > > Bugs: AMBARI-16146 > https://issues.apache.org/jira/browse/AMBARI-16146 > > > Repository: ambari > > > Description > ------- > > Earlier : The synchronized UserLocalConnection.initialValue() was taking time > to get hive connection and all other threads were blocking to get inside this > synchronized block. > > In this patch : > 1. Removed synchronized from UserLocalConnection.initialValue() and from > UserLocalHiveAuthCredentials.initialValue() > 2. Introduced locking on specific key and not the complete method. Key would > be like Connection_INSTANCE-NAME:USER-NAME. So threads for other keys will > not get into contension with this lock. This avoids blocking of thread that > do not want to initialize different keys. > 3. Used tryLock and fail with exception if lock cannot be obtained > immediately. This avoids blocking of threads that want to initialize same key > as one key should be initialized only once. > 4. corrected synchronization at several places inside UserLocal class. > > > Diffs > ----- > > > contrib/views/hive/src/main/java/org/apache/ambari/view/hive/client/UserLocalConnection.java > c80a4c4 > > contrib/views/hive/src/main/java/org/apache/ambari/view/hive/client/UserLocalHiveAuthCredentials.java > 9c72863 > > contrib/views/utils/src/main/java/org/apache/ambari/view/utils/UserLocal.java > 40c8e6e > > Diff: https://reviews.apache.org/r/47305/diff/ > > > Testing > ------- > > Manual testing done by introducing Thread.sleep in > UserLocalConnection.initialValue() and creating more requests with following > combinations and logs. > 0. first thread for connection + user + view combination obtains the lock and > goes inside UserLocalConnection.initialValue() and sleeps > 1. same user, same hive view requests for connections fails to get lock with > exception. > 2. same user, different hive view -- if first request gets new lock and goes > inside UserLocalConnection.initialValue(). > 3. same user, different hive view -- if connection created once then works > fine. does not block. > 4. different user, same view -- gets new lock and goes inside > UserLocalConnection.initialValue() if first, else works fine. > 5. different user, different view -- works as 4. > 6. once the first thread come out of initialValue(), rest threads don't try > to get lock and don't fail either, works fine. > > > Thanks, > > Nitiraj Rathore > >