> On Nov. 5, 2015, 10:38 a.m., Ajay Yadava wrote:
> > scheduler/src/main/java/org/apache/falcon/state/StateService.java, line 139
> > <https://reviews.apache.org/r/39588/diff/2/?file=1114737#file1114737line139>
> >
> >     Why is it being overridden again after being fetched from statestore 
> > just 2 lines before.

Yes it is needed as awaiting predicates in instances updated in between and it 
needs to be updated in DB


> On Nov. 5, 2015, 10:38 a.m., Ajay Yadava wrote:
> > scheduler/src/main/java/org/apache/falcon/state/store/jdbc/BeanMapperUtil.java,
> >  line 73
> > <https://reviews.apache.org/r/39588/diff/2/?file=1114742#file1114742line73>
> >
> >     Can we use EntityState instead of instance, please?

Sure Thanks.


> On Nov. 5, 2015, 10:38 a.m., Ajay Yadava wrote:
> > scheduler/src/main/java/org/apache/falcon/state/store/jdbc/BeanMapperUtil.java,
> >  line 90
> > <https://reviews.apache.org/r/39588/diff/2/?file=1114742#file1114742line90>
> >
> >     Let's not overload the word "instance" please. It is very confusing.

sure.


> On Nov. 5, 2015, 10:38 a.m., Ajay Yadava wrote:
> > scheduler/src/main/java/org/apache/falcon/state/store/jdbc/BeanMapperUtil.java,
> >  line 144
> > <https://reviews.apache.org/r/39588/diff/2/?file=1114742#file1114742line144>
> >
> >     Didn't check the implementation but if an instance is created, 
> > shouldn't it always be in some state?

Yes by default it should be in some state . Will remove this check. Even for 
type also


> On Nov. 5, 2015, 10:38 a.m., Ajay Yadava wrote:
> > scheduler/src/main/java/org/apache/falcon/state/store/jdbc/BeanMapperUtil.java,
> >  line 165
> > <https://reviews.apache.org/r/39588/diff/2/?file=1114742#file1114742line165>
> >
> >     This will make database language dependent, as you will not be able to 
> > read it in any other language except Java.

I understand . As there are complex data types involved, We feel this is best 
to use.


> On Nov. 5, 2015, 10:38 a.m., Ajay Yadava wrote:
> > scheduler/src/main/java/org/apache/falcon/state/store/jdbc/BeanMapperUtil.java,
> >  line 167
> > <https://reviews.apache.org/r/39588/diff/2/?file=1114742#file1114742line167>
> >
> >     Any reason for not storing predicates in a simple readable format, like 
> > in their own table/column? It would be immensely useful to read predicate 
> > states.

We don't want to use joins by storing perdicates in another table. We want to 
make it simple thats why storing predicate as object itsself.


> On Nov. 5, 2015, 10:38 a.m., Ajay Yadava wrote:
> > scheduler/src/main/java/org/apache/falcon/state/store/jdbc/BeanMapperUtil.java,
> >  line 169
> > <https://reviews.apache.org/r/39588/diff/2/?file=1114742#file1114742line169>
> >
> >     You should use IOUtils.closeQuietly() from commons.io.

Sure will use it.


> On Nov. 5, 2015, 10:38 a.m., Ajay Yadava wrote:
> > scheduler/src/main/java/org/apache/falcon/state/store/jdbc/BeanMapperUtil.java,
> >  line 172
> > <https://reviews.apache.org/r/39588/diff/2/?file=1114742#file1114742line172>
> >
> >     It is not required to close it as it will be automatically closed when 
> > you call close on out.

Sequence is changed . First we will close this before output stream


> On Nov. 5, 2015, 10:38 a.m., Ajay Yadava wrote:
> > scheduler/src/main/java/org/apache/falcon/state/store/jdbc/BeanMapperUtil.java,
> >  line 213
> > <https://reviews.apache.org/r/39588/diff/2/?file=1114742#file1114742line213>
> >
> >     Use IOUtils.

sure


> On Nov. 5, 2015, 10:38 a.m., Ajay Yadava wrote:
> > scheduler/src/main/java/org/apache/falcon/state/store/jdbc/BeanMapperUtil.java,
> >  line 216
> > <https://reviews.apache.org/r/39588/diff/2/?file=1114742#file1114742line216>
> >
> >     Redundant.

will remove


> On Nov. 5, 2015, 10:38 a.m., Ajay Yadava wrote:
> > scheduler/src/main/java/org/apache/falcon/state/store/jdbc/BeanMapperUtil.java,
> >  line 278
> > <https://reviews.apache.org/r/39588/diff/2/?file=1114742#file1114742line278>
> >
> >     use IOUtils

ok


> On Nov. 5, 2015, 10:38 a.m., Ajay Yadava wrote:
> > scheduler/src/main/java/org/apache/falcon/state/store/jdbc/BeanMapperUtil.java,
> >  line 281
> > <https://reviews.apache.org/r/39588/diff/2/?file=1114742#file1114742line281>
> >
> >     Redundant.

will remove


> On Nov. 5, 2015, 10:38 a.m., Ajay Yadava wrote:
> > scheduler/src/main/java/org/apache/falcon/state/store/jdbc/EntityBean.java, 
> > line 39
> > <https://reviews.apache.org/r/39588/diff/2/?file=1114743#file1114743line39>
> >
> >     nit: GET_ENTITIES_FOR_TYPE

Yes makes sense


> On Nov. 5, 2015, 10:38 a.m., Ajay Yadava wrote:
> > scheduler/src/main/java/org/apache/falcon/state/store/jdbc/EntityBean.java, 
> > line 44
> > <https://reviews.apache.org/r/39588/diff/2/?file=1114743#file1114743line44>
> >
> >     It's not a good convention to have table name in all caps or 
> > plural(makes it difficult in certain scenarios to guess/spell correctly)

I think it is good to have Tables in Capitals, in queries also it is easy to 
differentiate table names with column names and also OOZIE follows this 
convention


> On Nov. 5, 2015, 10:38 a.m., Ajay Yadava wrote:
> > scheduler/src/main/java/org/apache/falcon/state/store/jdbc/EntityBean.java, 
> > line 65
> > <https://reviews.apache.org/r/39588/diff/2/?file=1114743#file1114743line65>
> >
> >     Do you need the prefix current? There is no other similar column to be 
> > disambiguated.

Not sure why it is added like this. I followed from Base patch thinking in 
future some other states will be added


> On Nov. 5, 2015, 10:38 a.m., Ajay Yadava wrote:
> > scheduler/src/main/java/org/apache/falcon/state/store/jdbc/InstanceBean.java,
> >  line 41
> > <https://reviews.apache.org/r/39588/diff/2/?file=1114744#file1114744line41>
> >
> >     Why like and not =? Can't this be done with an equal on entity_id?

Yes Ajay you are correct. EntityId was not there before , it was recently 
added, Thanks for bringing this up.


> On Nov. 5, 2015, 10:38 a.m., Ajay Yadava wrote:
> > scheduler/src/main/java/org/apache/falcon/state/store/jdbc/InstanceBean.java,
> >  line 43
> > <https://reviews.apache.org/r/39588/diff/2/?file=1114744#file1114744line43>
> >
> >     Same as above for pattern matching on id.

Will take care


> On Nov. 5, 2015, 10:38 a.m., Ajay Yadava wrote:
> > scheduler/src/main/java/org/apache/falcon/state/store/jdbc/InstanceBean.java,
> >  line 44
> > <https://reviews.apache.org/r/39588/diff/2/?file=1114744#file1114744line44>
> >
> >     Same as above on pattern matching

Will take care


> On Nov. 5, 2015, 10:38 a.m., Ajay Yadava wrote:
> > scheduler/src/main/java/org/apache/falcon/state/store/jdbc/InstanceBean.java,
> >  line 46
> > <https://reviews.apache.org/r/39588/diff/2/?file=1114744#file1114744line46>
> >
> >     Why actualStartTime and actualEndTime? They are not deterministic so we 
> > always query on the basis of nominal time.

Yes Completely agreed. We will discuss this with pallavi on this


> On Nov. 5, 2015, 10:38 a.m., Ajay Yadava wrote:
> > scheduler/src/main/java/org/apache/falcon/state/store/jdbc/InstanceBean.java,
> >  line 47
> > <https://reviews.apache.org/r/39588/diff/2/?file=1114744#file1114744line47>
> >
> >     There is no limit, so how is it ensuring that you only get the last 
> > instance?

Limit is not supported in open jpa as it is specific to few databases. But 
while retriving we can specify query.setMaxResults(1) to make sure only one 
result will come.


> On Nov. 5, 2015, 10:38 a.m., Ajay Yadava wrote:
> > scheduler/src/main/java/org/apache/falcon/state/store/jdbc/InstanceBean.java,
> >  line 48
> > <https://reviews.apache.org/r/39588/diff/2/?file=1114744#file1114744line48>
> >
> >     DELETE_TABLE

Ok will make it DELETE_INSTANCES_TABLE. later if entities table also might need 
delete


> On Nov. 5, 2015, 10:38 a.m., Ajay Yadava wrote:
> > scheduler/src/main/java/org/apache/falcon/state/store/jdbc/JDBCStateStore.java,
> >  line 53
> > <https://reviews.apache.org/r/39588/diff/2/?file=1114745#file1114745line53>
> >
> >     I understand that you might need clear method for tests but it is not 
> > useful, rather risky, in production scenarios. So can we throw an exception 
> > here?

Sure will add debug statement over here


> On Nov. 5, 2015, 10:38 a.m., Ajay Yadava wrote:
> > scheduler/src/main/java/org/apache/falcon/state/store/jdbc/JDBCStateStore.java,
> >  line 151
> > <https://reviews.apache.org/r/39588/diff/2/?file=1114745#file1114745line151>
> >
> >     Same as above for "clear"

sure will fix this.


> On Nov. 5, 2015, 10:38 a.m., Ajay Yadava wrote:
> > scheduler/src/main/java/org/apache/falcon/state/store/service/FalconJPAService.java,
> >  line 58
> > <https://reviews.apache.org/r/39588/diff/2/?file=1114746#file1114746line58>
> >
> >     Can you please document the role of this field?

sure


> On Nov. 5, 2015, 10:38 a.m., Ajay Yadava wrote:
> > scheduler/src/main/java/org/apache/falcon/state/store/service/FalconJPAService.java,
> >  line 59
> > <https://reviews.apache.org/r/39588/diff/2/?file=1114746#file1114746line59>
> >
> >     Make it final please.

sure will make


> On Nov. 5, 2015, 10:38 a.m., Ajay Yadava wrote:
> > scheduler/src/main/java/org/apache/falcon/state/store/service/FalconJPAService.java,
> >  line 82
> > <https://reviews.apache.org/r/39588/diff/2/?file=1114746#file1114746line82>
> >
> >     .getSimpleName() will be more readable.

Sure will make


> On Nov. 5, 2015, 10:38 a.m., Ajay Yadava wrote:
> > scheduler/src/main/java/org/apache/falcon/tools/FalconStateStoreDBCLI.java, 
> > line 52
> > <https://reviews.apache.org/r/39588/diff/2/?file=1114747#file1114747line52>
> >
> >     What is the purpose of this field?

Will add comment . This represents whether DB Instance exists or not.


> On Nov. 5, 2015, 10:38 a.m., Ajay Yadava wrote:
> > scheduler/src/main/java/org/apache/falcon/tools/FalconStateStoreDBCLI.java, 
> > line 53
> > <https://reviews.apache.org/r/39588/diff/2/?file=1114747#file1114747line53>
> >
> >     Any reasons for this limitation? Most people will use MySQL or postgres 
> > in production environments.

No reasons for limitation. I alpha phase we just want to test with Derby. It 
will work with my sql also. We have to test it .


> On Nov. 5, 2015, 10:38 a.m., Ajay Yadava wrote:
> > scheduler/src/main/java/org/apache/falcon/tools/FalconStateStoreDBCLI.java, 
> > line 129
> > <https://reviews.apache.org/r/39588/diff/2/?file=1114747#file1114747line129>
> >
> >     redundant blank lines?

Just for more readability it was added. Will try to clean up


> On Nov. 5, 2015, 10:38 a.m., Ajay Yadava wrote:
> > scheduler/src/main/java/org/apache/falcon/tools/FalconStateStoreDBCLI.java, 
> > line 137
> > <https://reviews.apache.org/r/39588/diff/2/?file=1114747#file1114747line137>
> >
> >     "Falcon DB" is ambiguous.

Out of curious , Why Falcon DB is confusing and ambigous ? I don't understand


> On Nov. 5, 2015, 10:38 a.m., Ajay Yadava wrote:
> > scheduler/src/main/java/org/apache/falcon/tools/FalconStateStoreDBCLI.java, 
> > line 151
> > <https://reviews.apache.org/r/39588/diff/2/?file=1114747#file1114747line151>
> >
> >     Why double spacing everywhere?

Just for more readability as it is tool . Will remove it


> On Nov. 5, 2015, 10:38 a.m., Ajay Yadava wrote:
> > scheduler/src/main/java/org/apache/falcon/tools/FalconStateStoreDBCLI.java, 
> > line 179
> > <https://reviews.apache.org/r/39588/diff/2/?file=1114747#file1114747line179>
> >
> >     More descriptive message please.

I understand your concern. Done always comes with previous message with what is 
happening 
Ex: 
Creating DB Table
DONE

If i add again Creating DB Table done it will redundant isn't it ?


> On Nov. 5, 2015, 10:38 a.m., Ajay Yadava wrote:
> > scheduler/src/main/java/org/apache/falcon/tools/FalconStateStoreDBCLI.java, 
> > line 205
> > <https://reviews.apache.org/r/39588/diff/2/?file=1114747#file1114747line205>
> >
> >     More descriptive message please.

Same i described above


> On Nov. 5, 2015, 10:38 a.m., Ajay Yadava wrote:
> > scheduler/src/main/java/org/apache/falcon/tools/FalconStateStoreDBCLI.java, 
> > line 297
> > <https://reviews.apache.org/r/39588/diff/2/?file=1114747#file1114747line297>
> >
> >     A more descriptive message on what operation got completed please. It 
> > will be clearer in cases like calling one function from another.

Same as i mentioned earlier


> On Nov. 5, 2015, 10:38 a.m., Ajay Yadava wrote:
> > scheduler/src/main/java/org/apache/falcon/tools/FalconStateStoreDBCLI.java, 
> > line 308
> > <https://reviews.apache.org/r/39588/diff/2/?file=1114747#file1114747line308>
> >
> >     nit: Checking

Ok will fix


> On Nov. 5, 2015, 10:38 a.m., Ajay Yadava wrote:
> > scheduler/src/main/java/org/apache/falcon/tools/FalconStateStoreDBCLI.java, 
> > line 329
> > <https://reviews.apache.org/r/39588/diff/2/?file=1114747#file1114747line329>
> >
> >     More descriptive message please.

Described in previous comments


> On Nov. 5, 2015, 10:38 a.m., Ajay Yadava wrote:
> > scheduler/src/main/java/org/apache/falcon/tools/FalconStateStoreDBCLI.java, 
> > line 360
> > <https://reviews.apache.org/r/39588/diff/2/?file=1114747#file1114747line360>
> >
> >     nit: Validating

acked


> On Nov. 5, 2015, 10:38 a.m., Ajay Yadava wrote:
> > scheduler/src/main/java/org/apache/falcon/tools/FalconStateStoreDBCLI.java, 
> > line 363
> > <https://reviews.apache.org/r/39588/diff/2/?file=1114747#file1114747line363>
> >
> >     More descriptive message please.

Validating DB Connection.
DONE

I think this is enough for user to understand operation. Otherwise it looks 
redundant


> On Nov. 5, 2015, 10:38 a.m., Ajay Yadava wrote:
> > scheduler/src/main/java/org/apache/falcon/tools/FalconStateStoreDBCLI.java, 
> > line 403
> > <https://reviews.apache.org/r/39588/diff/2/?file=1114747#file1114747line403>
> >
> >     More descriptive message please.

Ok I will discuss with you offline


> On Nov. 5, 2015, 10:38 a.m., Ajay Yadava wrote:
> > scheduler/src/test/java/org/apache/falcon/state/service/store/TestJDBCStateStore.java,
> >  line 127
> > <https://reviews.apache.org/r/39588/diff/2/?file=1114756#file1114756line127>
> >
> >     Not the best way to do it. Break this into several test cases. Put 
> > expected exception and message on each of them. You can extract common code 
> > to a private function.

Sure will fix this as part of another jira. As this is already big. Dont want 
to do bigger items. Tracked in https://issues.apache.org/jira/browse/FALCON-1599


> On Nov. 5, 2015, 10:38 a.m., Ajay Yadava wrote:
> > scheduler/src/main/resources/falcon-buildinfo.properties, line 20
> > <https://reviews.apache.org/r/39588/diff/2/?file=1114749#file1114749line20>
> >
> >     Is it required to have this copy in scheduler as well?

Yes it is required to get the version of falcon


> On Nov. 5, 2015, 10:38 a.m., Ajay Yadava wrote:
> > scheduler/src/main/resources/falcon-buildinfo.properties, line 1
> > <https://reviews.apache.org/r/39588/diff/2/?file=1114749#file1114749line1>
> >
> >     Is another copy of this file required in scheduler module?

Yes required to get the falcon version during upgrade DB


> On Nov. 5, 2015, 10:38 a.m., Ajay Yadava wrote:
> > scheduler/src/main/java/org/apache/falcon/tools/FalconStateStoreDBCLI.java, 
> > line 307
> > <https://reviews.apache.org/r/39588/diff/2/?file=1114747#file1114747line307>
> >
> >     I think this function shouldn't throw exception on the basis of an 
> > argument. Caller functions should decide how they want to handle the 
> > scenario and throw exception if required.
> >     
> >     If you need to do it many times then create another function called 
> > assertFalconPropsTableExists() which should throw an error.

Sure agreed will fix.


> On Nov. 5, 2015, 10:38 a.m., Ajay Yadava wrote:
> > scheduler/src/main/java/org/apache/falcon/tools/FalconStateStoreDBCLI.java, 
> > line 307
> > <https://reviews.apache.org/r/39588/diff/2/?file=1114747#file1114747line307>
> >
> >     Functions shouldn't throw exceptions on the basis of arguments. Let the 
> > caller decide how he wants to handle it.
> >     
> >     If you require same exception throwing behavior then create another 
> > function called assertFalconPropsTableExists which calls this function and 
> > throws error if it doesn't exist.

Sure will fix


> On Nov. 5, 2015, 10:38 a.m., Ajay Yadava wrote:
> > scheduler/src/main/java/org/apache/falcon/tools/FalconStateStoreDBCLI.java, 
> > line 301
> > <https://reviews.apache.org/r/39588/diff/2/?file=1114747#file1114747line301>
> >
> >     There should only be a method called falconPropsTableExists() which 
> > returns true or false.

Agreed will fix.


> On Nov. 5, 2015, 10:38 a.m., Ajay Yadava wrote:
> > scheduler/src/main/java/org/apache/falcon/tools/FalconStateStoreDBCLI.java, 
> > line 301
> > <https://reviews.apache.org/r/39588/diff/2/?file=1114747#file1114747line301>
> >
> >     There should be a function called falconPropsTableExists() and it 
> > should return true or false depending on whether the table exists.

Sure will fix


> On Nov. 5, 2015, 10:38 a.m., Ajay Yadava wrote:
> > scheduler/src/main/java/org/apache/falcon/state/store/jdbc/JDBCStateStore.java,
> >  line 87
> > <https://reviews.apache.org/r/39588/diff/2/?file=1114745#file1114745line87>
> >
> >     I guess empty is returned when there are no results, so when is null 
> > returned?

Will Fix


> On Nov. 5, 2015, 10:38 a.m., Ajay Yadava wrote:
> > common/src/main/resources/startup.properties, line 261
> > <https://reviews.apache.org/r/39588/diff/2/?file=1114729#file1114729line261>
> >
> >     I think we should store all these properties in a separate file to 
> > avoid bloating of startup.properties. It will also help in avoiding the 
> > issue of credentials in startup.properties.

Yes we should do. Tracking in https://issues.apache.org/jira/browse/FALCON-1600


> On Nov. 5, 2015, 10:38 a.m., Ajay Yadava wrote:
> > scheduler/src/main/java/org/apache/falcon/state/store/jdbc/JDBCStateStore.java,
> >  line 42
> > <https://reviews.apache.org/r/39588/diff/2/?file=1114745#file1114745line42>
> >
> >     This class should be divided into a class per table so that it becomes 
> > more manageable.

Yes tracked in https://issues.apache.org/jira/browse/FALCON-1599


> On Nov. 5, 2015, 10:38 a.m., Ajay Yadava wrote:
> > scheduler/src/main/java/org/apache/falcon/tools/FalconStateStoreDBCLI.java, 
> > line 44
> > <https://reviews.apache.org/r/39588/diff/2/?file=1114747#file1114747line44>
> >
> >     How are we ensuring that this is not run by anyone else except admin?

Yes we should do tracked as part of 
https://issues.apache.org/jira/browse/FALCON-1601


> On Nov. 5, 2015, 10:38 a.m., Ajay Yadava wrote:
> > scheduler/src/main/java/org/apache/falcon/tools/FalconStateStoreDBCLI.java, 
> > line 66
> > <https://reviews.apache.org/r/39588/diff/2/?file=1114747#file1114747line66>
> >
> >     Can we rephrase it? It sounds like it is interactive. You should 
> > highlight that it won't ask for confirmation.

Yes will do


> On Nov. 5, 2015, 10:38 a.m., Ajay Yadava wrote:
> > scheduler/src/main/java/org/apache/falcon/tools/FalconStateStoreDBCLI.java, 
> > line 81
> > <https://reviews.apache.org/r/39588/diff/2/?file=1114747#file1114747line81>
> >
> >     Falcon DB is very confusing. It should be either state store version or 
> > underlying db version.

Will discuss offline with this


> On Nov. 5, 2015, 10:38 a.m., Ajay Yadava wrote:
> > scheduler/src/main/java/org/apache/falcon/tools/FalconStateStoreDBCLI.java, 
> > line 141
> > <https://reviews.apache.org/r/39588/diff/2/?file=1114747#file1114747line141>
> >
> >     We should document what will happen in case of rollbacks.

There is no roll back upgrade wont happen for the same version of Falcon


> On Nov. 5, 2015, 10:38 a.m., Ajay Yadava wrote:
> > scheduler/src/main/java/org/apache/falcon/tools/FalconStateStoreDBCLI.java, 
> > line 186
> > <https://reviews.apache.org/r/39588/diff/2/?file=1114747#file1114747line186>
> >
> >     Please avoid using Falcon DB for table names or database.

Will discuss offline regarding this


> On Nov. 5, 2015, 10:38 a.m., Ajay Yadava wrote:
> > scheduler/src/main/java/org/apache/falcon/tools/FalconStateStoreDBCLI.java, 
> > line 216
> > <https://reviews.apache.org/r/39588/diff/2/?file=1114747#file1114747line216>
> >
> >     Credentials shouldn't be stored in startup.properties

Yes we should fix. It is tracked in 
https://issues.apache.org/jira/browse/FALCON-1601


> On Nov. 5, 2015, 10:38 a.m., Ajay Yadava wrote:
> > scheduler/src/main/resources/META-INF/persistence.xml, line 1
> > <https://reviews.apache.org/r/39588/diff/2/?file=1114748#file1114748line1>
> >
> >     It will be useful to at least put in comments the MySQL / Postgres 
> > configurations wherever they are different.

will discuss this


> On Nov. 5, 2015, 10:38 a.m., Ajay Yadava wrote:
> > scheduler/src/test/resources/startup.properties, line 1
> > <https://reviews.apache.org/r/39588/diff/2/?file=1114758#file1114758line1>
> >
> >     Having startup.properties in each module seems verbose to me. Can't we 
> > leverage common startup.properties somehow?

Yes it is tracked in https://issues.apache.org/jira/browse/FALCON-1600


> On Nov. 5, 2015, 10:38 a.m., Ajay Yadava wrote:
> > src/bin/falcon-db.sh, line 14
> > <https://reviews.apache.org/r/39588/diff/2/?file=1114759#file1114759line14>
> >
> >     Sample usage/help documentation?

FalconStateStoreDBCLI Will print arguments if anything goes wrong.


> On Nov. 5, 2015, 10:38 a.m., Ajay Yadava wrote:
> > src/bin/falcon-db.sh, line 16
> > <https://reviews.apache.org/r/39588/diff/2/?file=1114759#file1114759line16>
> >
> >     Although not a hard requirement but it will be much better to have a 
> > python script instead of a bash script. Reason for this is windows 
> > compatibility. We already have a JIRA to convert existing scripts to python 
> > so we shouldn't add more tasks to it as much as possible.

Tracked in https://issues.apache.org/jira/browse/FALCON-1600


> On Nov. 5, 2015, 10:38 a.m., Ajay Yadava wrote:
> > src/conf/startup.properties, line 61
> > <https://reviews.apache.org/r/39588/diff/2/?file=1114760#file1114760line61>
> >
> >     Shouldn't all scheduler code be inside a package path containing 
> > "scheduler"?

We will do as part of https://issues.apache.org/jira/browse/FALCON-1600


> On Nov. 5, 2015, 10:38 a.m., Ajay Yadava wrote:
> > scheduler/src/main/java/org/apache/falcon/tools/FalconStateStoreDBCLI.java, 
> > line 403
> > <https://reviews.apache.org/r/39588/diff/2/?file=1114747#file1114747line403>
> >
> >     nit: a more descriptive message on what action got completed will be 
> > more user friendly.

As i said will discuss with this . I hope current one is enough


> On Nov. 5, 2015, 10:38 a.m., Ajay Yadava wrote:
> > scheduler/src/main/java/org/apache/falcon/tools/FalconStateStoreDBCLI.java, 
> > line 363
> > <https://reviews.apache.org/r/39588/diff/2/?file=1114747#file1114747line363>
> >
> >     nit: DB Connection validated successfully.

acked


- pavan kumar


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/39588/#review105205
-----------------------------------------------------------


On Nov. 3, 2015, 5:45 p.m., pavan kumar kolamuri wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/39588/
> -----------------------------------------------------------
> 
> (Updated Nov. 3, 2015, 5:45 p.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 6f2c480 
>   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/EntityStateStore.java 
> 4aa6fdb 
>   
> scheduler/src/main/java/org/apache/falcon/state/store/InMemoryStateStore.java 
> 3822860 
>   
> scheduler/src/main/java/org/apache/falcon/state/store/InstanceStateStore.java 
> d6a4b49 
>   scheduler/src/main/java/org/apache/falcon/state/store/StateStore.java 
> f595c26 
>   
> scheduler/src/main/java/org/apache/falcon/state/store/jdbc/BeanMapperUtil.java
>  PRE-CREATION 
>   scheduler/src/main/java/org/apache/falcon/state/store/jdbc/EntityBean.java 
> PRE-CREATION 
>   
> scheduler/src/main/java/org/apache/falcon/state/store/jdbc/InstanceBean.java 
> PRE-CREATION 
>   
> scheduler/src/main/java/org/apache/falcon/state/store/jdbc/JDBCStateStore.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/TestJDBCStateStore.java
>  PRE-CREATION 
>   
> scheduler/src/test/java/org/apache/falcon/tools/TestFalconStateStoreDBCLI.java
>  PRE-CREATION 
>   scheduler/src/test/resources/startup.properties PRE-CREATION 
>   src/bin/falcon-db.sh PRE-CREATION 
>   src/conf/startup.properties ce6e91f 
>   src/main/assemblies/distributed-package.xml 794eaef 
>   src/main/assemblies/standalone-package.xml fcff8d7 
>   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
> 
>

Reply via email to