> On Oct. 26, 2015, 11:35 a.m., Pallavi Rao wrote: > > scheduler/src/main/java/org/apache/falcon/state/store/ValidateConnectionBean.java, > > line 31 > > <https://reviews.apache.org/r/39588/diff/1/?file=1104028#file1104028line31> > > > > Why do we need this? Especially since we are anyway validating against > > EntityBean and InstanceBean, can't we use one of them?
sure will remove > On Oct. 26, 2015, 11:35 a.m., Pallavi Rao wrote: > > scheduler/src/main/java/org/apache/falcon/state/store/service/FalconJPAService.java, > > line 88 > > <https://reviews.apache.org/r/39588/diff/1/?file=1104029#file1104029line88> > > > > This class will need to be registered as a service in > > startup.properties for this method to get invoked. yes > On Oct. 26, 2015, 11:35 a.m., Pallavi Rao wrote: > > scheduler/src/main/java/org/apache/falcon/state/store/service/FalconJPAService.java, > > line 114 > > <https://reviews.apache.org/r/39588/diff/1/?file=1104029#file1104029line114> > > > > May be a concern if you expect admins to specify the DB password in > > clear text. May be we can add an option to also get it from the command > > prompt (admin types it in), if the password is not specified. Currently oozie also does the same. If it is needed further , we will discuss and raise jira jira to track it > On Oct. 26, 2015, 11:35 a.m., Pallavi Rao wrote: > > scheduler/src/main/java/org/apache/falcon/state/store/service/FalconJPAService.java, > > line 171 > > <https://reviews.apache.org/r/39588/diff/1/?file=1104029#file1104029line171> > > > > Can't we cache the entity manager? Every call of PersistentStateStore > > invokes getEntityManager; creating every time will be expensive. No. All transactions requires entity manager , otherwise it is leading to exceptions > On Oct. 26, 2015, 11:35 a.m., Pallavi Rao wrote: > > scheduler/src/main/java/org/apache/falcon/tools/FalconStateStoreDBCLI.java, > > line 53 > > <https://reviews.apache.org/r/39588/diff/1/?file=1104030#file1104030line53> > > > > Why Derby DB alone? As long as it has a JDBC driver, it should be fine. > > right? In our production env., we will use MySQL. Currently it was supported derby, will remove it once my sql support is tested as well. > On Oct. 26, 2015, 11:35 a.m., Pallavi Rao wrote: > > scheduler/src/main/java/org/apache/falcon/tools/FalconStateStoreDBCLI.java, > > line 255 > > <https://reviews.apache.org/r/39588/diff/1/?file=1104030#file1104030line255> > > > > The tables may exist, but, may not be of for the current version of > > Falcon. What do we do in such a case? We should at the least verify the > > version and message out accordingly. Only once create table will be used later it should be update, in that we have taken care of Checking versions of table. > On Oct. 26, 2015, 11:35 a.m., Pallavi Rao wrote: > > scheduler/src/main/java/org/apache/falcon/tools/FalconStateStoreDBCLI.java, > > line 379 > > <https://reviews.apache.org/r/39588/diff/1/?file=1104030#file1104030line379> > > > > Need to check for instance table too? Its just checking whether table created. We are not querying anything. I think this is enough. > On Oct. 26, 2015, 11:35 a.m., Pallavi Rao wrote: > > scheduler/src/main/java/org/apache/falcon/tools/FalconStateStoreDBCLI.java, > > line 405 > > <https://reviews.apache.org/r/39588/diff/1/?file=1104030#file1104030line405> > > > > Falcon DB Tool and Falcon version are same. isn't it? If so, we can > > just say "Falcon server version:" sure will add > On Oct. 26, 2015, 11:35 a.m., Pallavi Rao wrote: > > scheduler/src/main/resources/META-INF/persistence.xml, line 44 > > <https://reviews.apache.org/r/39588/diff/1/?file=1104031#file1104031line44> > > > > With this setting and the fact that we are not modifying the result of > > a query, I think the queries can be optimized for read and don't need a > > transational boundary. Ties back to my comment on PersistentStateStore. Will remove transactional boundaries for get queries > On Oct. 26, 2015, 11:35 a.m., Pallavi Rao wrote: > > scheduler/src/test/java/org/apache/falcon/state/EntityStateServiceTest.java, > > line 48 > > <https://reviews.apache.org/r/39588/diff/1/?file=1104036#file1104036line48> > > > > This test just tests the valid transitions of state of an entity. It > > should not require any persistence. Please remove the DB dependency and let > > it use the InMemory DB. ok will make it use InmemoryStore itelf . But i dont see any disadvantage using Persistent Store > On Oct. 26, 2015, 11:35 a.m., Pallavi Rao wrote: > > src/conf/startup.properties, line 253 > > <https://reviews.apache.org/r/39588/diff/1/?file=1104042#file1104042line253> > > > > Missing JPAService? Also, as mentioned, we can keep this commented and > > allow users to uncomment it if they use native scheduler. Sure will add - pavan kumar ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/39588/#review103989 ----------------------------------------------------------- On Oct. 23, 2015, 11:17 a.m., pavan kumar kolamuri wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/39588/ > ----------------------------------------------------------- > > (Updated Oct. 23, 2015, 11:17 a.m.) > > > Review request for Falcon. > > > Bugs: https://issues.apache.org/jira/browse/FALCON-1234 > > https://issues.apache.org/jira/browse/https://issues.apache.org/jira/browse/FALCON-1234 > > > Repository: falcon-git > > > Description > ------- > > Persistent State Store for Falcon Native Scheduler > > > Diffs > ----- > > checkstyle/src/main/resources/falcon/checkstyle.xml 2130e73 > checkstyle/src/main/resources/falcon/findbugs-exclude.xml 0a7580d > common/src/main/resources/startup.properties cc5212a > pom.xml 87c55e3 > scheduler/pom.xml 20a91d2 > scheduler/src/main/java/org/apache/falcon/execution/ExecutionInstance.java > 3869ff2 > > scheduler/src/main/java/org/apache/falcon/execution/FalconExecutionService.java > b959320 > > scheduler/src/main/java/org/apache/falcon/execution/ProcessExecutionInstance.java > 19089c4 > scheduler/src/main/java/org/apache/falcon/predicate/Predicate.java fb4ce82 > scheduler/src/main/java/org/apache/falcon/state/ID.java 420c856 > scheduler/src/main/java/org/apache/falcon/state/StateService.java 81357a4 > > scheduler/src/main/java/org/apache/falcon/state/store/AbstractStateStore.java > 67e047f > scheduler/src/main/java/org/apache/falcon/state/store/BeanMapperUtil.java > PRE-CREATION > scheduler/src/main/java/org/apache/falcon/state/store/EntityBean.java > PRE-CREATION > scheduler/src/main/java/org/apache/falcon/state/store/EntityStateStore.java > 4aa6fdb > > scheduler/src/main/java/org/apache/falcon/state/store/InMemoryStateStore.java > 3822860 > scheduler/src/main/java/org/apache/falcon/state/store/InstanceBean.java > PRE-CREATION > > scheduler/src/main/java/org/apache/falcon/state/store/InstanceStateStore.java > d6a4b49 > > scheduler/src/main/java/org/apache/falcon/state/store/PersistentStateStore.java > PRE-CREATION > scheduler/src/main/java/org/apache/falcon/state/store/StateStore.java > f595c26 > > scheduler/src/main/java/org/apache/falcon/state/store/ValidateConnectionBean.java > PRE-CREATION > > scheduler/src/main/java/org/apache/falcon/state/store/service/FalconJPAService.java > PRE-CREATION > scheduler/src/main/java/org/apache/falcon/tools/FalconStateStoreDBCLI.java > PRE-CREATION > scheduler/src/main/resources/META-INF/persistence.xml PRE-CREATION > scheduler/src/main/resources/falcon-buildinfo.properties PRE-CREATION > > scheduler/src/test/java/org/apache/falcon/execution/FalconExecutionServiceTest.java > b2f9e59 > > scheduler/src/test/java/org/apache/falcon/notification/service/SchedulerServiceTest.java > b4a0f35 > > scheduler/src/test/java/org/apache/falcon/state/AbstractSchedulerTestBase.java > PRE-CREATION > scheduler/src/test/java/org/apache/falcon/state/EntityStateServiceTest.java > 2f32b43 > > scheduler/src/test/java/org/apache/falcon/state/InstanceStateServiceTest.java > d27ac7e > > scheduler/src/test/java/org/apache/falcon/state/service/TestFalconJPAService.java > PRE-CREATION > > scheduler/src/test/java/org/apache/falcon/state/service/store/TestPersistentStateStore.java > PRE-CREATION > > scheduler/src/test/java/org/apache/falcon/tools/TestFalconStateStoreDBCLI.java > PRE-CREATION > scheduler/src/test/resources/startup.properties PRE-CREATION > src/conf/startup.properties dc9e393 > unit/src/main/resources/startup.properties fe6f430 > > Diff: https://reviews.apache.org/r/39588/diff/ > > > Testing > ------- > > I have written unit tests. I will also test externally by setting up > everything > > > Thanks, > > pavan kumar kolamuri > >
