> On Dec. 29, 2014, 3:37 p.m., Suma Shivaprasad wrote: > > lens-server/src/main/java/org/apache/lens/server/LensServer.java, line 159 > > <https://reviews.apache.org/r/29416/diff/2/?file=801340#file801340line159> > > > > Am not convinced with timeout here since it is causing the thread to > > almost always wakeup unnecessarily every 2 secs(given that stop is rare) > > and check flag, hold lock again etc...I dont see any documentation anywhere > > reg usage of wait without timeout leading to issues in the javadoc as well > > ...@Laxman can you point out any link which describes the issue with wait > > without timeout which I might have missed. > > Srikanth Sundarrajan wrote: > I dont see an issue in changing the wait(...) to wait() as long as it is > guarded. > > Srikanth Sundarrajan wrote: > Actually there might be a situation where the system can go into a > perpetual wait() if used without timeout.
My initial comment was on "unguarded wait". Java doesnot guarantee that wait will be unblocked only by notify call only. Please refer to "java.lang.Object" javadoc (search for spurious wakeup). Now let me try to answer why this should be time based wait. Just elaborating Srikanth's comment (perpetual wait) - Assume that T1 (thread) waiting in wait() call (Note that this is wait without timeout) - Assume that T2 (thread) is the shutdownhook thread - T1 is unblocked from wait call on "so-called spurious wakeup" and it is out of the synchrnized block and has seen "canRun" while condition to true - User triggered a shutdown - T2 thread is invoked - T1 and T2 contends for lock/monitor - T2 acquires the lock - Proceeds with notify which is listened by no one - T1 acquires lock and waits indefinitely - Laxman ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/29416/#review66241 ----------------------------------------------------------- On Dec. 29, 2014, 9:15 p.m., Srikanth Sundarrajan wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/29416/ > ----------------------------------------------------------- > > (Updated Dec. 29, 2014, 9:15 p.m.) > > > Review request for lens. > > > Bugs: LENS-163 > https://issues.apache.org/jira/browse/LENS-163 > > > Repository: lens > > > Description > ------- > > LensServer imports the following dependencies > > import sun.misc.Signal; > import sun.misc.SignalHandler; > > and this is being used to register essentially shutdownhook. > > ... > Signal.handle(new Signal("TERM"), new SignalHandler() { > > @Override > public void handle(Signal signal) { > ... > > We should use Runtime::addShutdownHook() instead. > > > Diffs > ----- > > lens-server/pom.xml 55d5d58 > lens-server/src/main/java/org/apache/lens/server/LensServer.java 66abbcd > lens-server/src/main/java/org/apache/lens/server/LensServices.java fe2fc75 > lens-server/src/main/resources/lens-build-info.properties PRE-CREATION > pom.xml 2fb1005 > > Diff: https://reviews.apache.org/r/29416/diff/ > > > Testing > ------- > > No new tests added. Existing tests ran alright. > > [INFO] Reactor Summary: > [INFO] > [INFO] Lens Checkstyle Rules ............................. SUCCESS [0.937s] > [INFO] Lens .............................................. SUCCESS [3.139s] > [INFO] Lens API .......................................... SUCCESS [5.696s] > [INFO] Lens API for server and extensions ................ SUCCESS [3.089s] > [INFO] Lens Cube ......................................... SUCCESS [5:21.532s] > [INFO] Lens DB storage ................................... SUCCESS [28.502s] > [INFO] Lens Query Library ................................ SUCCESS [19.518s] > [INFO] Lens Hive Driver .................................. SUCCESS > [11:37.892s] > [INFO] Lens Driver for Cloudera Impala ................... SUCCESS [2.718s] > [INFO] Lens Driver for JDBC .............................. SUCCESS [36.736s] > [INFO] Lens Server ....................................... SUCCESS > [12:09.370s] > [INFO] Lens client ....................................... SUCCESS [54.261s] > [INFO] Lens CLI .......................................... SUCCESS [3:44.160s] > [INFO] Lens Examples ..................................... SUCCESS [0.534s] > [INFO] Lens Distribution ................................. SUCCESS [3.451s] > [INFO] Lens Client Distribution .......................... SUCCESS [2.473s] > [INFO] Lens ML Lib ....................................... SUCCESS [1:36.809s] > [INFO] > ------------------------------------------------------------------------ > [INFO] BUILD SUCCESS > [INFO] > ------------------------------------------------------------------------ > [INFO] Total time: 37:11.229s > [INFO] Finished at: Thu Dec 25 12:43:50 IST 2014 > [INFO] Final Memory: 135M/801M > [INFO] > ------------------------------------------------------------------------ > > > Thanks, > > Srikanth Sundarrajan > >
