> On May 5, 2016, 6:18 a.m., Amareshwari Sriramadasu wrote: > > lens-server/src/main/java/org/apache/lens/server/LensServices.java, line 242 > > <https://reviews.apache.org/r/46968/diff/2/?file=1370733#file1370733line242> > > > > Right now, We have two different configurations - which are independent: > > > > <property> > > <name>lens.server.restart.enabled</name> > > <value>true</value> > > <description>If flag is enabled, all the services will be persisted > > to persistent > > location passed. > > </description> > > </property> > > > > <property> > > <name>lens.server.recover.onrestart</name> > > <value>true</value> > > <description>If the flag is enabled, all the services will be > > started from last > > saved state, if disabled all the services will start afresh > > </description> > > </property> > > > > With this change done, we are coupling them. If > > lens.server.recover.onrestart is true, then lens.server.restart.enabled > > should also be true. > > > > I'm fine with the change. But just calling out.
Rajat had similar query. Actually this happned by mistake while refatoring code (i was not expecting mutiple configs for this feature and overlooked the second one). But now if I think of it, one configuration should suffice as poointed out by Amareshwari We can merge the two options into one and make it simpler. If we do this then to achieve the following, we would need some manual intervention Use Case : lens.server.restart.enabled = true and lens.server.recover.onrestart = true. Now we shutdown the server and change lens.server.restart.enabled to false. In this case the server will recover using old state but will not persist any new states. Now if we restart the server again, it will use the old state once again to recover. To achieve the same after merging the features (there will be only one property lens.server.state.persistence.enabled = true) , user will have to copy persisted files after the shutdown (say copy A). Then restart the server which will use the last persisted state (same as copy A) to recover and will start persisting the new states. Now if we want to restart once again from old state (A), user will have to manually overwrite the persisted files with copied persisted files (copy A) before starting the server. This is a rare use case though ( mostly for testing) pros of merging : simplicity and single flow cons : manual intervention in some rare cases Thoughts ? - Puneet ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/46968/#review131786 ----------------------------------------------------------- On May 4, 2016, 12:03 p.m., Puneet Gupta wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/46968/ > ----------------------------------------------------------- > > (Updated May 4, 2016, 12:03 p.m.) > > > Review request for lens. > > > Bugs: lens-1029 > https://issues.apache.org/jira/browse/lens-1029 > > > Repository: lens > > > Description > ------- > > - Service level persistence isolation. If persisting one service fails, other > services should still be persisted. > - Persistence thread to run only in case SERVER_RESTART_ENABLED = true > - Moved form Timer to ScheduledExecutorService. Graceful shutdown of > ScheduledExecutorService enabled to allow a running persistence task, if any, > to finish > - Catching Exception instead of IOException to prevent the persistence task > from dying. > - using writeObject() instead of writeInt() for automatic null handling. > > > Diffs > ----- > > > lens-query-lib/src/main/java/org/apache/lens/lib/query/AbstractFileFormatter.java > 8c06621 > lens-server/src/main/java/org/apache/lens/server/LensServices.java 48b3e00 > > Diff: https://reviews.apache.org/r/46968/diff/ > > > Testing > ------- > > Relying on exiting test case to check persistence TestServerRestart. > > **mvn test -Dtest=org.apache.lens.server.TestServerRestart#testQueryService** > Running org.apache.lens.server.TestServerRestart > Tests run: 1, Failures: 0, Errors: 0, Skipped: 0, Time elapsed: 206.408 sec - > in org.apache.lens.server.TestServerRestart > > Results : > > Tests run: 1, Failures: 0, Errors: 0, Skipped: 0 > > > **mvn test > -Dtest=org.apache.lens.server.TestServerRestart#testSessionRestart** > Running org.apache.lens.server.TestServerRestart > Tests run: 1, Failures: 0, Errors: 0, Skipped: 0, Time elapsed: 21.711 sec - > in org.apache.lens.server.TestServerRestart > > Results : > > Tests run: 1, Failures: 0, Errors: 0, Skipped: 0 > > > **Apart from this** > - Did some local testing for making sure null Integers are persisted using > writeObject and can be read back as well. > - Did some local testing to check graceful shutdown of > ScheduledExecutorService > Don't think we need a test cases for above two cince its supported out of box > by java > > > Thanks, > > Puneet Gupta > >
