You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@falcon.apache.org by pavan kumar kolamuri <pa...@gmail.com> on 2015/11/03 18:41:33 UTC
Re: Review Request 39588: State Store for instances scheduled by
Falcon Native Scheduler
> 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
>
>