You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@falcon.apache.org by Balu Vellanki <bv...@hortonworks.com> on 2015/11/10 02:23:43 UTC
Review Request 40121: Falcon-1372 : Retention does not work in corner
cases
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/40121/
-----------------------------------------------------------
Review request for Falcon, Ajay Yadava, Sowmya Ramesh, and Venkat Ranganathan.
Bugs: Falcon-1372
https://issues.apache.org/jira/browse/Falcon-1372
Repository: falcon-git
Description
-------
The bug is caused due to the way java.util.Calendar handles DAY_OF_MONTH. In FeedHelper, the getDate(...) method sets DAY_OF_MONTH to 0 for date patterns like ${YEAR}/${MONTH}.
This causes the month to be setback by 1. So "2015/11" will be treated as Oct 31st, 2015. Hence this instance will be evicted. For a date like "2015/11" , Calender should be set to cal.set(2015, 10, 1, 0, 0, 0); The MONTH should be "value - 1" and The DAY_OF_MONTH begins with a 1 instead of 0.
Diffs
-----
common/src/main/java/org/apache/falcon/entity/FeedHelper.java 8c55e41
common/src/main/java/org/apache/falcon/entity/common/FeedDataPath.java afe913d
common/src/test/java/org/apache/falcon/entity/FeedHelperTest.java 10dac49
Diff: https://reviews.apache.org/r/40121/diff/
Testing
-------
Testing done end2end, added few more test cases to verify getDate() method in FeedHelper works as expected.
Thanks,
Balu Vellanki
Re: Review Request 40121: Falcon-1372 : Retention does not work in
corner cases
Posted by Balu Vellanki <bv...@hortonworks.com>.
> On Nov. 15, 2015, 8:22 a.m., Ajay Yadava wrote:
> > common/src/main/java/org/apache/falcon/entity/FeedHelper.java, line 486
> > <https://reviews.apache.org/r/40121/diff/1/?file=1121198#file1121198line486>
> >
> > Is this really required?
Not setting 2015/10 should be Nov 1st, 2015 midnight. If the millisecond is not set, this field is set to millisecond of now(). If the retention coord is run exactly at midnight, this will cause problems. It is just cleaner to do it this way.
> On Nov. 15, 2015, 8:22 a.m., Ajay Yadava wrote:
> > common/src/main/java/org/apache/falcon/entity/common/FeedDataPath.java, line 33
> > <https://reviews.apache.org/r/40121/diff/1/?file=1121199#file1121199line33>
> >
> > Any reasons for converting it to non-static field?
Enums are implicitly static in Java and IDE was throwing a warning about it. So I fixed it.
> On Nov. 15, 2015, 8:22 a.m., Ajay Yadava wrote:
> > common/src/main/java/org/apache/falcon/entity/common/FeedDataPath.java, line 39
> > <https://reviews.apache.org/r/40121/diff/1/?file=1121199#file1121199line39>
> >
> > You have introduced a new level of granularity (SECOND) which was not earlier supported. Is it really required as part of this JIRA?
It is true it is not really required as part of the Jira but it felt the code is cleaner doing it this way.
- Balu
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/40121/#review106553
-----------------------------------------------------------
On Nov. 10, 2015, 1:23 a.m., Balu Vellanki wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/40121/
> -----------------------------------------------------------
>
> (Updated Nov. 10, 2015, 1:23 a.m.)
>
>
> Review request for Falcon, Ajay Yadava, Sowmya Ramesh, and Venkat Ranganathan.
>
>
> Bugs: Falcon-1372
> https://issues.apache.org/jira/browse/Falcon-1372
>
>
> Repository: falcon-git
>
>
> Description
> -------
>
> The bug is caused due to the way java.util.Calendar handles DAY_OF_MONTH. In FeedHelper, the getDate(...) method sets DAY_OF_MONTH to 0 for date patterns like ${YEAR}/${MONTH}.
> This causes the month to be setback by 1. So "2015/11" will be treated as Oct 31st, 2015. Hence this instance will be evicted. For a date like "2015/11" , Calender should be set to cal.set(2015, 10, 1, 0, 0, 0); The MONTH should be "value - 1" and The DAY_OF_MONTH begins with a 1 instead of 0.
>
>
> Diffs
> -----
>
> common/src/main/java/org/apache/falcon/entity/FeedHelper.java 8c55e41
> common/src/main/java/org/apache/falcon/entity/common/FeedDataPath.java afe913d
> common/src/test/java/org/apache/falcon/entity/FeedHelperTest.java 10dac49
>
> Diff: https://reviews.apache.org/r/40121/diff/
>
>
> Testing
> -------
>
> Testing done end2end, added few more test cases to verify getDate() method in FeedHelper works as expected.
>
>
> Thanks,
>
> Balu Vellanki
>
>
Re: Review Request 40121: Falcon-1372 : Retention does not work in
corner cases
Posted by Ajay Yadava <aj...@gmail.com>.
> On Nov. 15, 2015, 8:22 a.m., Ajay Yadava wrote:
> > common/src/main/java/org/apache/falcon/entity/FeedHelper.java, line 486
> > <https://reviews.apache.org/r/40121/diff/1/?file=1121198#file1121198line486>
> >
> > Is this really required?
>
> Balu Vellanki wrote:
> Not setting 2015/10 should be Nov 1st, 2015 midnight. If the millisecond is not set, this field is set to millisecond of now(). If the retention coord is run exactly at midnight, this will cause problems. It is just cleaner to do it this way.
Makes sense. Thank you for the explanation.
> On Nov. 15, 2015, 8:22 a.m., Ajay Yadava wrote:
> > common/src/main/java/org/apache/falcon/entity/common/FeedDataPath.java, line 33
> > <https://reviews.apache.org/r/40121/diff/1/?file=1121199#file1121199line33>
> >
> > Any reasons for converting it to non-static field?
>
> Balu Vellanki wrote:
> Enums are implicitly static in Java and IDE was throwing a warning about it. So I fixed it.
Yes, I had forgotten :-) Thank you!
- Ajay
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/40121/#review106553
-----------------------------------------------------------
On Nov. 15, 2015, 6:21 p.m., Balu Vellanki wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/40121/
> -----------------------------------------------------------
>
> (Updated Nov. 15, 2015, 6:21 p.m.)
>
>
> Review request for Falcon, Ajay Yadava, Sowmya Ramesh, and Venkat Ranganathan.
>
>
> Bugs: Falcon-1372
> https://issues.apache.org/jira/browse/Falcon-1372
>
>
> Repository: falcon-git
>
>
> Description
> -------
>
> The bug is caused due to the way java.util.Calendar handles DAY_OF_MONTH. In FeedHelper, the getDate(...) method sets DAY_OF_MONTH to 0 for date patterns like ${YEAR}/${MONTH}.
> This causes the month to be setback by 1. So "2015/11" will be treated as Oct 31st, 2015. Hence this instance will be evicted. For a date like "2015/11" , Calender should be set to cal.set(2015, 10, 1, 0, 0, 0); The MONTH should be "value - 1" and The DAY_OF_MONTH begins with a 1 instead of 0.
>
>
> Diffs
> -----
>
> common/src/main/java/org/apache/falcon/entity/FeedHelper.java 8c55e41
> common/src/main/java/org/apache/falcon/entity/common/FeedDataPath.java afe913d
> common/src/test/java/org/apache/falcon/entity/FeedHelperTest.java 10dac49
>
> Diff: https://reviews.apache.org/r/40121/diff/
>
>
> Testing
> -------
>
> Testing done end2end, added few more test cases to verify getDate() method in FeedHelper works as expected.
>
>
> Thanks,
>
> Balu Vellanki
>
>
Re: Review Request 40121: Falcon-1372 : Retention does not work in
corner cases
Posted by Ajay Yadava <aj...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/40121/#review106553
-----------------------------------------------------------
common/src/main/java/org/apache/falcon/entity/FeedHelper.java (line 486)
<https://reviews.apache.org/r/40121/#comment165256>
Is this really required?
common/src/main/java/org/apache/falcon/entity/common/FeedDataPath.java (line 33)
<https://reviews.apache.org/r/40121/#comment165257>
Any reasons for converting it to non-static field?
common/src/main/java/org/apache/falcon/entity/common/FeedDataPath.java (line 39)
<https://reviews.apache.org/r/40121/#comment165255>
You have introduced a new level of granularity (SECOND) which was not earlier supported. Is it really required as part of this JIRA?
- Ajay Yadava
On Nov. 10, 2015, 1:23 a.m., Balu Vellanki wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/40121/
> -----------------------------------------------------------
>
> (Updated Nov. 10, 2015, 1:23 a.m.)
>
>
> Review request for Falcon, Ajay Yadava, Sowmya Ramesh, and Venkat Ranganathan.
>
>
> Bugs: Falcon-1372
> https://issues.apache.org/jira/browse/Falcon-1372
>
>
> Repository: falcon-git
>
>
> Description
> -------
>
> The bug is caused due to the way java.util.Calendar handles DAY_OF_MONTH. In FeedHelper, the getDate(...) method sets DAY_OF_MONTH to 0 for date patterns like ${YEAR}/${MONTH}.
> This causes the month to be setback by 1. So "2015/11" will be treated as Oct 31st, 2015. Hence this instance will be evicted. For a date like "2015/11" , Calender should be set to cal.set(2015, 10, 1, 0, 0, 0); The MONTH should be "value - 1" and The DAY_OF_MONTH begins with a 1 instead of 0.
>
>
> Diffs
> -----
>
> common/src/main/java/org/apache/falcon/entity/FeedHelper.java 8c55e41
> common/src/main/java/org/apache/falcon/entity/common/FeedDataPath.java afe913d
> common/src/test/java/org/apache/falcon/entity/FeedHelperTest.java 10dac49
>
> Diff: https://reviews.apache.org/r/40121/diff/
>
>
> Testing
> -------
>
> Testing done end2end, added few more test cases to verify getDate() method in FeedHelper works as expected.
>
>
> Thanks,
>
> Balu Vellanki
>
>
Re: Review Request 40121: Falcon-1372 : Retention does not work in
corner cases
Posted by Venkat Ranganathan <n....@live.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/40121/#review106516
-----------------------------------------------------------
Ship it!
Ship It!
- Venkat Ranganathan
On Nov. 9, 2015, 5:23 p.m., Balu Vellanki wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/40121/
> -----------------------------------------------------------
>
> (Updated Nov. 9, 2015, 5:23 p.m.)
>
>
> Review request for Falcon, Ajay Yadava, Sowmya Ramesh, and Venkat Ranganathan.
>
>
> Bugs: Falcon-1372
> https://issues.apache.org/jira/browse/Falcon-1372
>
>
> Repository: falcon-git
>
>
> Description
> -------
>
> The bug is caused due to the way java.util.Calendar handles DAY_OF_MONTH. In FeedHelper, the getDate(...) method sets DAY_OF_MONTH to 0 for date patterns like ${YEAR}/${MONTH}.
> This causes the month to be setback by 1. So "2015/11" will be treated as Oct 31st, 2015. Hence this instance will be evicted. For a date like "2015/11" , Calender should be set to cal.set(2015, 10, 1, 0, 0, 0); The MONTH should be "value - 1" and The DAY_OF_MONTH begins with a 1 instead of 0.
>
>
> Diffs
> -----
>
> common/src/main/java/org/apache/falcon/entity/FeedHelper.java 8c55e41
> common/src/main/java/org/apache/falcon/entity/common/FeedDataPath.java afe913d
> common/src/test/java/org/apache/falcon/entity/FeedHelperTest.java 10dac49
>
> Diff: https://reviews.apache.org/r/40121/diff/
>
>
> Testing
> -------
>
> Testing done end2end, added few more test cases to verify getDate() method in FeedHelper works as expected.
>
>
> Thanks,
>
> Balu Vellanki
>
>
Re: Review Request 40121: Falcon-1372 : Retention does not work in
corner cases
Posted by Balu Vellanki <bv...@hortonworks.com>.
> On Nov. 11, 2015, 3:53 p.m., Venkat Ranganathan wrote:
> > common/src/main/java/org/apache/falcon/entity/FeedHelper.java, line 484
> > <https://reviews.apache.org/r/40121/diff/1/?file=1121198#file1121198line484>
> >
> > Minor nit: Do we have tabs here instead of spaces?
I can confirm these are not tabs. It just shows that the } is moved four spaces to the right.
- Balu
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/40121/#review106072
-----------------------------------------------------------
On Nov. 10, 2015, 1:23 a.m., Balu Vellanki wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/40121/
> -----------------------------------------------------------
>
> (Updated Nov. 10, 2015, 1:23 a.m.)
>
>
> Review request for Falcon, Ajay Yadava, Sowmya Ramesh, and Venkat Ranganathan.
>
>
> Bugs: Falcon-1372
> https://issues.apache.org/jira/browse/Falcon-1372
>
>
> Repository: falcon-git
>
>
> Description
> -------
>
> The bug is caused due to the way java.util.Calendar handles DAY_OF_MONTH. In FeedHelper, the getDate(...) method sets DAY_OF_MONTH to 0 for date patterns like ${YEAR}/${MONTH}.
> This causes the month to be setback by 1. So "2015/11" will be treated as Oct 31st, 2015. Hence this instance will be evicted. For a date like "2015/11" , Calender should be set to cal.set(2015, 10, 1, 0, 0, 0); The MONTH should be "value - 1" and The DAY_OF_MONTH begins with a 1 instead of 0.
>
>
> Diffs
> -----
>
> common/src/main/java/org/apache/falcon/entity/FeedHelper.java 8c55e41
> common/src/main/java/org/apache/falcon/entity/common/FeedDataPath.java afe913d
> common/src/test/java/org/apache/falcon/entity/FeedHelperTest.java 10dac49
>
> Diff: https://reviews.apache.org/r/40121/diff/
>
>
> Testing
> -------
>
> Testing done end2end, added few more test cases to verify getDate() method in FeedHelper works as expected.
>
>
> Thanks,
>
> Balu Vellanki
>
>
Re: Review Request 40121: Falcon-1372 : Retention does not work in
corner cases
Posted by Venkat Ranganathan <n....@live.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/40121/#review106072
-----------------------------------------------------------
Good work tracking this and fixing it.
common/src/main/java/org/apache/falcon/entity/FeedHelper.java (line 484)
<https://reviews.apache.org/r/40121/#comment164789>
Minor nit: Do we have tabs here instead of spaces?
G
- Venkat Ranganathan
On Nov. 9, 2015, 5:23 p.m., Balu Vellanki wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/40121/
> -----------------------------------------------------------
>
> (Updated Nov. 9, 2015, 5:23 p.m.)
>
>
> Review request for Falcon, Ajay Yadava, Sowmya Ramesh, and Venkat Ranganathan.
>
>
> Bugs: Falcon-1372
> https://issues.apache.org/jira/browse/Falcon-1372
>
>
> Repository: falcon-git
>
>
> Description
> -------
>
> The bug is caused due to the way java.util.Calendar handles DAY_OF_MONTH. In FeedHelper, the getDate(...) method sets DAY_OF_MONTH to 0 for date patterns like ${YEAR}/${MONTH}.
> This causes the month to be setback by 1. So "2015/11" will be treated as Oct 31st, 2015. Hence this instance will be evicted. For a date like "2015/11" , Calender should be set to cal.set(2015, 10, 1, 0, 0, 0); The MONTH should be "value - 1" and The DAY_OF_MONTH begins with a 1 instead of 0.
>
>
> Diffs
> -----
>
> common/src/main/java/org/apache/falcon/entity/FeedHelper.java 8c55e41
> common/src/main/java/org/apache/falcon/entity/common/FeedDataPath.java afe913d
> common/src/test/java/org/apache/falcon/entity/FeedHelperTest.java 10dac49
>
> Diff: https://reviews.apache.org/r/40121/diff/
>
>
> Testing
> -------
>
> Testing done end2end, added few more test cases to verify getDate() method in FeedHelper works as expected.
>
>
> Thanks,
>
> Balu Vellanki
>
>
Re: Review Request 40121: Falcon-1372 : Retention does not work in
corner cases
Posted by Ajay Yadava <aj...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/40121/#review106568
-----------------------------------------------------------
Ship it!
Ship It!
- Ajay Yadava
On Nov. 15, 2015, 6:21 p.m., Balu Vellanki wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/40121/
> -----------------------------------------------------------
>
> (Updated Nov. 15, 2015, 6:21 p.m.)
>
>
> Review request for Falcon, Ajay Yadava, Sowmya Ramesh, and Venkat Ranganathan.
>
>
> Bugs: Falcon-1372
> https://issues.apache.org/jira/browse/Falcon-1372
>
>
> Repository: falcon-git
>
>
> Description
> -------
>
> The bug is caused due to the way java.util.Calendar handles DAY_OF_MONTH. In FeedHelper, the getDate(...) method sets DAY_OF_MONTH to 0 for date patterns like ${YEAR}/${MONTH}.
> This causes the month to be setback by 1. So "2015/11" will be treated as Oct 31st, 2015. Hence this instance will be evicted. For a date like "2015/11" , Calender should be set to cal.set(2015, 10, 1, 0, 0, 0); The MONTH should be "value - 1" and The DAY_OF_MONTH begins with a 1 instead of 0.
>
>
> Diffs
> -----
>
> common/src/main/java/org/apache/falcon/entity/FeedHelper.java 8c55e41
> common/src/main/java/org/apache/falcon/entity/common/FeedDataPath.java afe913d
> common/src/test/java/org/apache/falcon/entity/FeedHelperTest.java 10dac49
>
> Diff: https://reviews.apache.org/r/40121/diff/
>
>
> Testing
> -------
>
> Testing done end2end, added few more test cases to verify getDate() method in FeedHelper works as expected.
>
>
> Thanks,
>
> Balu Vellanki
>
>
Re: Review Request 40121: Falcon-1372 : Retention does not work in
corner cases
Posted by Balu Vellanki <bv...@hortonworks.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/40121/
-----------------------------------------------------------
(Updated Nov. 15, 2015, 6:21 p.m.)
Review request for Falcon, Ajay Yadava, Sowmya Ramesh, and Venkat Ranganathan.
Changes
-------
This patch incorporates Ajay's suggestion. Granularity at level of SECONDs is removed.
Bugs: Falcon-1372
https://issues.apache.org/jira/browse/Falcon-1372
Repository: falcon-git
Description
-------
The bug is caused due to the way java.util.Calendar handles DAY_OF_MONTH. In FeedHelper, the getDate(...) method sets DAY_OF_MONTH to 0 for date patterns like ${YEAR}/${MONTH}.
This causes the month to be setback by 1. So "2015/11" will be treated as Oct 31st, 2015. Hence this instance will be evicted. For a date like "2015/11" , Calender should be set to cal.set(2015, 10, 1, 0, 0, 0); The MONTH should be "value - 1" and The DAY_OF_MONTH begins with a 1 instead of 0.
Diffs (updated)
-----
common/src/main/java/org/apache/falcon/entity/FeedHelper.java 8c55e41
common/src/main/java/org/apache/falcon/entity/common/FeedDataPath.java afe913d
common/src/test/java/org/apache/falcon/entity/FeedHelperTest.java 10dac49
Diff: https://reviews.apache.org/r/40121/diff/
Testing
-------
Testing done end2end, added few more test cases to verify getDate() method in FeedHelper works as expected.
Thanks,
Balu Vellanki