You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@accumulo.apache.org by ke...@deenlo.com on 2014/03/25 03:53:51 UTC
Review Request 19601: ACCUMULO-2520 1.6.0-SNAPSHOT patch
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/19601/
-----------------------------------------------------------
Review request for accumulo.
Bugs: ACCUMULO-2520
https://issues.apache.org/jira/browse/ACCUMULO-2520
Repository: accumulo
Description
-------
These changes for 1.6 for ACCUMULO-2520 are very different than the changes made in 1.4 and 1.5. I merged the changes from 1.5 to 1.6 using -sours (as opposed to resolving conflicts in a merge in a radically different way). This patch makes the following changes.
* adds test for bad delete markers
* made changes do disallow a delete marker of hdfs://nn/, it used to pass even though "" and "/" did not in 1.6
* logs a warning about invalid delete markers instead of derailing garbage collection
Diffs
-----
server/gc/src/main/java/org/apache/accumulo/gc/GarbageCollectionAlgorithm.java 464d0d9
server/gc/src/test/java/org/apache/accumulo/gc/GarbageCollectionTest.java 1e9c1dd
test/src/test/java/org/apache/accumulo/test/functional/GarbageCollectorIT.java 3a9b404
Diff: https://reviews.apache.org/r/19601/diff/
Testing
-------
ran added/changed test
Thanks,
kturner
Re: Review Request 19601: ACCUMULO-2520 1.6.0-SNAPSHOT patch
Posted by Sean Busbey <se...@manvsbeard.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/19601/#review38491
-----------------------------------------------------------
Ship it!
Ship It!
- Sean Busbey
On March 25, 2014, 2:53 a.m., kturner wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/19601/
> -----------------------------------------------------------
>
> (Updated March 25, 2014, 2:53 a.m.)
>
>
> Review request for accumulo.
>
>
> Bugs: ACCUMULO-2520
> https://issues.apache.org/jira/browse/ACCUMULO-2520
>
>
> Repository: accumulo
>
>
> Description
> -------
>
> These changes for 1.6 for ACCUMULO-2520 are very different than the changes made in 1.4 and 1.5. I merged the changes from 1.5 to 1.6 using -sours (as opposed to resolving conflicts in a merge in a radically different way). This patch makes the following changes.
>
> * adds test for bad delete markers
> * made changes do disallow a delete marker of hdfs://nn/, it used to pass even though "" and "/" did not in 1.6
> * logs a warning about invalid delete markers instead of derailing garbage collection
>
>
> Diffs
> -----
>
> server/gc/src/main/java/org/apache/accumulo/gc/GarbageCollectionAlgorithm.java 464d0d9
> server/gc/src/test/java/org/apache/accumulo/gc/GarbageCollectionTest.java 1e9c1dd
> test/src/test/java/org/apache/accumulo/test/functional/GarbageCollectorIT.java 3a9b404
>
> Diff: https://reviews.apache.org/r/19601/diff/
>
>
> Testing
> -------
>
> ran added/changed test
>
>
> Thanks,
>
> kturner
>
>
Re: Review Request 19601: ACCUMULO-2520 1.6.0-SNAPSHOT patch
Posted by ke...@deenlo.com.
> On March 25, 2014, 7:36 a.m., Sean Busbey wrote:
> > server/gc/src/main/java/org/apache/accumulo/gc/GarbageCollectionAlgorithm.java, lines 118-120
> > <https://reviews.apache.org/r/19601/diff/1/?file=534978#file534978line118>
> >
> > Does this exception require immediate operator attention?
> >
> > I think no, right? At worst, we're leaking files in HDFS. but probably not so fast that they need to do something?
> >
> > If it doesn't require immediate attention, I'd like to lower this to WARN.
>
> kturner wrote:
> If this happens it indicates a bug somewhere in the Accumulo code. If its a harmless bug, the error would be annoying. Warn seems like a good middle ground between info and error. If it considers everything invalid, the high number of warning should hopefully get peoples attention.
If there are no other concerns I can change this before pushing. Would also change it in 1.4 and 1.5 to be consistent.
- kturner
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/19601/#review38426
-----------------------------------------------------------
On March 25, 2014, 2:53 a.m., kturner wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/19601/
> -----------------------------------------------------------
>
> (Updated March 25, 2014, 2:53 a.m.)
>
>
> Review request for accumulo.
>
>
> Bugs: ACCUMULO-2520
> https://issues.apache.org/jira/browse/ACCUMULO-2520
>
>
> Repository: accumulo
>
>
> Description
> -------
>
> These changes for 1.6 for ACCUMULO-2520 are very different than the changes made in 1.4 and 1.5. I merged the changes from 1.5 to 1.6 using -sours (as opposed to resolving conflicts in a merge in a radically different way). This patch makes the following changes.
>
> * adds test for bad delete markers
> * made changes do disallow a delete marker of hdfs://nn/, it used to pass even though "" and "/" did not in 1.6
> * logs a warning about invalid delete markers instead of derailing garbage collection
>
>
> Diffs
> -----
>
> server/gc/src/main/java/org/apache/accumulo/gc/GarbageCollectionAlgorithm.java 464d0d9
> server/gc/src/test/java/org/apache/accumulo/gc/GarbageCollectionTest.java 1e9c1dd
> test/src/test/java/org/apache/accumulo/test/functional/GarbageCollectorIT.java 3a9b404
>
> Diff: https://reviews.apache.org/r/19601/diff/
>
>
> Testing
> -------
>
> ran added/changed test
>
>
> Thanks,
>
> kturner
>
>
Re: Review Request 19601: ACCUMULO-2520 1.6.0-SNAPSHOT patch
Posted by ke...@deenlo.com.
> On March 25, 2014, 7:36 a.m., Sean Busbey wrote:
> > server/gc/src/main/java/org/apache/accumulo/gc/GarbageCollectionAlgorithm.java, lines 118-120
> > <https://reviews.apache.org/r/19601/diff/1/?file=534978#file534978line118>
> >
> > Does this exception require immediate operator attention?
> >
> > I think no, right? At worst, we're leaking files in HDFS. but probably not so fast that they need to do something?
> >
> > If it doesn't require immediate attention, I'd like to lower this to WARN.
If this happens it indicates a bug somewhere in the Accumulo code. If its a harmless bug, the error would be annoying. Warn seems like a good middle ground between info and error. If it considers everything invalid, the high number of warning should hopefully get peoples attention.
- kturner
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/19601/#review38426
-----------------------------------------------------------
On March 25, 2014, 2:53 a.m., kturner wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/19601/
> -----------------------------------------------------------
>
> (Updated March 25, 2014, 2:53 a.m.)
>
>
> Review request for accumulo.
>
>
> Bugs: ACCUMULO-2520
> https://issues.apache.org/jira/browse/ACCUMULO-2520
>
>
> Repository: accumulo
>
>
> Description
> -------
>
> These changes for 1.6 for ACCUMULO-2520 are very different than the changes made in 1.4 and 1.5. I merged the changes from 1.5 to 1.6 using -sours (as opposed to resolving conflicts in a merge in a radically different way). This patch makes the following changes.
>
> * adds test for bad delete markers
> * made changes do disallow a delete marker of hdfs://nn/, it used to pass even though "" and "/" did not in 1.6
> * logs a warning about invalid delete markers instead of derailing garbage collection
>
>
> Diffs
> -----
>
> server/gc/src/main/java/org/apache/accumulo/gc/GarbageCollectionAlgorithm.java 464d0d9
> server/gc/src/test/java/org/apache/accumulo/gc/GarbageCollectionTest.java 1e9c1dd
> test/src/test/java/org/apache/accumulo/test/functional/GarbageCollectorIT.java 3a9b404
>
> Diff: https://reviews.apache.org/r/19601/diff/
>
>
> Testing
> -------
>
> ran added/changed test
>
>
> Thanks,
>
> kturner
>
>
Re: Review Request 19601: ACCUMULO-2520 1.6.0-SNAPSHOT patch
Posted by Sean Busbey <se...@manvsbeard.com>.
> On March 25, 2014, 7:36 a.m., Sean Busbey wrote:
> > server/gc/src/main/java/org/apache/accumulo/gc/GarbageCollectionAlgorithm.java, lines 118-120
> > <https://reviews.apache.org/r/19601/diff/1/?file=534978#file534978line118>
> >
> > Does this exception require immediate operator attention?
> >
> > I think no, right? At worst, we're leaking files in HDFS. but probably not so fast that they need to do something?
> >
> > If it doesn't require immediate attention, I'd like to lower this to WARN.
>
> kturner wrote:
> If this happens it indicates a bug somewhere in the Accumulo code. If its a harmless bug, the error would be annoying. Warn seems like a good middle ground between info and error. If it considers everything invalid, the high number of warning should hopefully get peoples attention.
>
> kturner wrote:
> If there are no other concerns I can change this before pushing. Would also change it in 1.4 and 1.5 to be consistent.
sounds good to me.
- Sean
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/19601/#review38426
-----------------------------------------------------------
On March 25, 2014, 2:53 a.m., kturner wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/19601/
> -----------------------------------------------------------
>
> (Updated March 25, 2014, 2:53 a.m.)
>
>
> Review request for accumulo.
>
>
> Bugs: ACCUMULO-2520
> https://issues.apache.org/jira/browse/ACCUMULO-2520
>
>
> Repository: accumulo
>
>
> Description
> -------
>
> These changes for 1.6 for ACCUMULO-2520 are very different than the changes made in 1.4 and 1.5. I merged the changes from 1.5 to 1.6 using -sours (as opposed to resolving conflicts in a merge in a radically different way). This patch makes the following changes.
>
> * adds test for bad delete markers
> * made changes do disallow a delete marker of hdfs://nn/, it used to pass even though "" and "/" did not in 1.6
> * logs a warning about invalid delete markers instead of derailing garbage collection
>
>
> Diffs
> -----
>
> server/gc/src/main/java/org/apache/accumulo/gc/GarbageCollectionAlgorithm.java 464d0d9
> server/gc/src/test/java/org/apache/accumulo/gc/GarbageCollectionTest.java 1e9c1dd
> test/src/test/java/org/apache/accumulo/test/functional/GarbageCollectorIT.java 3a9b404
>
> Diff: https://reviews.apache.org/r/19601/diff/
>
>
> Testing
> -------
>
> ran added/changed test
>
>
> Thanks,
>
> kturner
>
>
Re: Review Request 19601: ACCUMULO-2520 1.6.0-SNAPSHOT patch
Posted by Sean Busbey <se...@manvsbeard.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/19601/#review38426
-----------------------------------------------------------
server/gc/src/main/java/org/apache/accumulo/gc/GarbageCollectionAlgorithm.java
<https://reviews.apache.org/r/19601/#comment70590>
Does this exception require immediate operator attention?
I think no, right? At worst, we're leaking files in HDFS. but probably not so fast that they need to do something?
If it doesn't require immediate attention, I'd like to lower this to WARN.
- Sean Busbey
On March 25, 2014, 2:53 a.m., kturner wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/19601/
> -----------------------------------------------------------
>
> (Updated March 25, 2014, 2:53 a.m.)
>
>
> Review request for accumulo.
>
>
> Bugs: ACCUMULO-2520
> https://issues.apache.org/jira/browse/ACCUMULO-2520
>
>
> Repository: accumulo
>
>
> Description
> -------
>
> These changes for 1.6 for ACCUMULO-2520 are very different than the changes made in 1.4 and 1.5. I merged the changes from 1.5 to 1.6 using -sours (as opposed to resolving conflicts in a merge in a radically different way). This patch makes the following changes.
>
> * adds test for bad delete markers
> * made changes do disallow a delete marker of hdfs://nn/, it used to pass even though "" and "/" did not in 1.6
> * logs a warning about invalid delete markers instead of derailing garbage collection
>
>
> Diffs
> -----
>
> server/gc/src/main/java/org/apache/accumulo/gc/GarbageCollectionAlgorithm.java 464d0d9
> server/gc/src/test/java/org/apache/accumulo/gc/GarbageCollectionTest.java 1e9c1dd
> test/src/test/java/org/apache/accumulo/test/functional/GarbageCollectorIT.java 3a9b404
>
> Diff: https://reviews.apache.org/r/19601/diff/
>
>
> Testing
> -------
>
> ran added/changed test
>
>
> Thanks,
>
> kturner
>
>