> 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. > > Puneet Gupta wrote: > 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 ?
Merged into one property - Puneet ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/46968/#review131786 ----------------------------------------------------------- On May 7, 2016, 10:58 a.m., Puneet Gupta wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/46968/ > ----------------------------------------------------------- > > (Updated May 7, 2016, 10:58 a.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-api/src/main/java/org/apache/lens/server/api/LensConfConstants.java > 23537cb > lens-server/src/main/java/org/apache/lens/server/LensServices.java 48b3e00 > lens-server/src/main/resources/lensserver-default.xml 1a15658 > lens-server/src/test/java/org/apache/lens/server/TestServerRestart.java > 1fa61ef > src/site/apt/admin/config.apt 5466e7a > src/site/apt/admin/deployment.apt b4f4d0a > > 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 > >
