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 2015/01/29 16:59:50 UTC
Review Request 30417: ACCUMULO-3462 ACCUMULO-3541 fixed compaction
logging and state bug
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/30417/
-----------------------------------------------------------
Review request for accumulo and Eric Newton.
Bugs: ACCUMULO-3462 and ACCUMULO-3541
https://issues.apache.org/jira/browse/ACCUMULO-3462
https://issues.apache.org/jira/browse/ACCUMULO-3541
Repository: accumulo
Description
-------
Fixes compaction logging bug and bug with compaction state not being reset. Patch builds on work already committed for ACCUMULO-3462.
I am slightly nervous about the changes w/ control flow, but I feel this is the best solution to avoid duplicate logging.
Diffs
-----
server/tserver/src/main/java/org/apache/accumulo/tserver/Tablet.java 7420ec4
Diff: https://reviews.apache.org/r/30417/diff/
Testing
-------
Still running ITs. Getting this patch out for review so we can move forward for 1.6.2.
Thanks,
kturner
Re: Review Request 30417: ACCUMULO-3462 ACCUMULO-3541 fixed compaction
logging and state bug
Posted by Eric Newton <er...@gmail.com>.
> On Jan. 29, 2015, 5:54 p.m., kturner wrote:
> > server/tserver/src/main/java/org/apache/accumulo/tserver/Tablet.java, line 3347
> > <https://reviews.apache.org/r/30417/diff/1/?file=840136#file840136line3347>
> >
> > While trying apply this patch to 1.5 I noticed that 1.5 is different. 1.5 does not keep a reference to what `Trace.on()` returns.
> >
> > If this reference does not need to be kept, then the fix may be simpler. Eric do you know if we need to keep the reference, or can we just call `curr.stop()` later insread of `span.stop()`.
They should be equivalent.
- Eric
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/30417/#review70234
-----------------------------------------------------------
On Jan. 29, 2015, 3:59 p.m., kturner wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/30417/
> -----------------------------------------------------------
>
> (Updated Jan. 29, 2015, 3:59 p.m.)
>
>
> Review request for accumulo and Eric Newton.
>
>
> Bugs: ACCUMULO-3462 and ACCUMULO-3541
> https://issues.apache.org/jira/browse/ACCUMULO-3462
> https://issues.apache.org/jira/browse/ACCUMULO-3541
>
>
> Repository: accumulo
>
>
> Description
> -------
>
> Fixes compaction logging bug and bug with compaction state not being reset. Patch builds on work already committed for ACCUMULO-3462.
>
> I am slightly nervous about the changes w/ control flow, but I feel this is the best solution to avoid duplicate logging.
>
>
> Diffs
> -----
>
> server/tserver/src/main/java/org/apache/accumulo/tserver/Tablet.java 7420ec4
>
> Diff: https://reviews.apache.org/r/30417/diff/
>
>
> Testing
> -------
>
> Still running ITs. Getting this patch out for review so we can move forward for 1.6.2.
>
>
> Thanks,
>
> kturner
>
>
Re: Review Request 30417: ACCUMULO-3462 ACCUMULO-3541 fixed compaction
logging and state bug
Posted by ke...@deenlo.com.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/30417/#review70234
-----------------------------------------------------------
server/tserver/src/main/java/org/apache/accumulo/tserver/Tablet.java
<https://reviews.apache.org/r/30417/#comment115308>
While trying apply this patch to 1.5 I noticed that 1.5 is different. 1.5 does not keep a reference to what `Trace.on()` returns.
If this reference does not need to be kept, then the fix may be simpler. Eric do you know if we need to keep the reference, or can we just call `curr.stop()` later insread of `span.stop()`.
- kturner
On Jan. 29, 2015, 3:59 p.m., kturner wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/30417/
> -----------------------------------------------------------
>
> (Updated Jan. 29, 2015, 3:59 p.m.)
>
>
> Review request for accumulo and Eric Newton.
>
>
> Bugs: ACCUMULO-3462 and ACCUMULO-3541
> https://issues.apache.org/jira/browse/ACCUMULO-3462
> https://issues.apache.org/jira/browse/ACCUMULO-3541
>
>
> Repository: accumulo
>
>
> Description
> -------
>
> Fixes compaction logging bug and bug with compaction state not being reset. Patch builds on work already committed for ACCUMULO-3462.
>
> I am slightly nervous about the changes w/ control flow, but I feel this is the best solution to avoid duplicate logging.
>
>
> Diffs
> -----
>
> server/tserver/src/main/java/org/apache/accumulo/tserver/Tablet.java 7420ec4
>
> Diff: https://reviews.apache.org/r/30417/diff/
>
>
> Testing
> -------
>
> Still running ITs. Getting this patch out for review so we can move forward for 1.6.2.
>
>
> Thanks,
>
> kturner
>
>
Re: Review Request 30417: ACCUMULO-3462 ACCUMULO-3541 fixed compaction
logging and state bug
Posted by Eric Newton <er...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/30417/#review70211
-----------------------------------------------------------
Ship it!
Ship It!
- Eric Newton
On Jan. 29, 2015, 3:59 p.m., kturner wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/30417/
> -----------------------------------------------------------
>
> (Updated Jan. 29, 2015, 3:59 p.m.)
>
>
> Review request for accumulo and Eric Newton.
>
>
> Bugs: ACCUMULO-3462 and ACCUMULO-3541
> https://issues.apache.org/jira/browse/ACCUMULO-3462
> https://issues.apache.org/jira/browse/ACCUMULO-3541
>
>
> Repository: accumulo
>
>
> Description
> -------
>
> Fixes compaction logging bug and bug with compaction state not being reset. Patch builds on work already committed for ACCUMULO-3462.
>
> I am slightly nervous about the changes w/ control flow, but I feel this is the best solution to avoid duplicate logging.
>
>
> Diffs
> -----
>
> server/tserver/src/main/java/org/apache/accumulo/tserver/Tablet.java 7420ec4
>
> Diff: https://reviews.apache.org/r/30417/diff/
>
>
> Testing
> -------
>
> Still running ITs. Getting this patch out for review so we can move forward for 1.6.2.
>
>
> Thanks,
>
> kturner
>
>
Re: Review Request 30417: ACCUMULO-3462 ACCUMULO-3541 fixed compaction
logging and state bug
Posted by ke...@deenlo.com.
> On Jan. 29, 2015, 4:25 p.m., kturner wrote:
> > `mvn verify -Psunny` passed... going to look for some compaction specific ITs to run
`mvn verify -Dtest=foo -Dit.test=*ompac* -Dfindbugs.skip=true -DfailIfNoTests=false` also passed
- kturner
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/30417/#review70216
-----------------------------------------------------------
On Jan. 29, 2015, 3:59 p.m., kturner wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/30417/
> -----------------------------------------------------------
>
> (Updated Jan. 29, 2015, 3:59 p.m.)
>
>
> Review request for accumulo and Eric Newton.
>
>
> Bugs: ACCUMULO-3462 and ACCUMULO-3541
> https://issues.apache.org/jira/browse/ACCUMULO-3462
> https://issues.apache.org/jira/browse/ACCUMULO-3541
>
>
> Repository: accumulo
>
>
> Description
> -------
>
> Fixes compaction logging bug and bug with compaction state not being reset. Patch builds on work already committed for ACCUMULO-3462.
>
> I am slightly nervous about the changes w/ control flow, but I feel this is the best solution to avoid duplicate logging.
>
>
> Diffs
> -----
>
> server/tserver/src/main/java/org/apache/accumulo/tserver/Tablet.java 7420ec4
>
> Diff: https://reviews.apache.org/r/30417/diff/
>
>
> Testing
> -------
>
> Still running ITs. Getting this patch out for review so we can move forward for 1.6.2.
>
>
> Thanks,
>
> kturner
>
>
Re: Review Request 30417: ACCUMULO-3462 ACCUMULO-3541 fixed compaction
logging and state bug
Posted by ke...@deenlo.com.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/30417/#review70216
-----------------------------------------------------------
`mvn verify -Psunny` passed... going to look for some compaction specific ITs to run
- kturner
On Jan. 29, 2015, 3:59 p.m., kturner wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/30417/
> -----------------------------------------------------------
>
> (Updated Jan. 29, 2015, 3:59 p.m.)
>
>
> Review request for accumulo and Eric Newton.
>
>
> Bugs: ACCUMULO-3462 and ACCUMULO-3541
> https://issues.apache.org/jira/browse/ACCUMULO-3462
> https://issues.apache.org/jira/browse/ACCUMULO-3541
>
>
> Repository: accumulo
>
>
> Description
> -------
>
> Fixes compaction logging bug and bug with compaction state not being reset. Patch builds on work already committed for ACCUMULO-3462.
>
> I am slightly nervous about the changes w/ control flow, but I feel this is the best solution to avoid duplicate logging.
>
>
> Diffs
> -----
>
> server/tserver/src/main/java/org/apache/accumulo/tserver/Tablet.java 7420ec4
>
> Diff: https://reviews.apache.org/r/30417/diff/
>
>
> Testing
> -------
>
> Still running ITs. Getting this patch out for review so we can move forward for 1.6.2.
>
>
> Thanks,
>
> kturner
>
>
Re: Review Request 30417: ACCUMULO-3462 ACCUMULO-3541 fixed compaction
logging and state bug
Posted by Josh Elser <jo...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/30417/#review70214
-----------------------------------------------------------
Ship it!
Ship It!
- Josh Elser
On Jan. 29, 2015, 3:59 p.m., kturner wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/30417/
> -----------------------------------------------------------
>
> (Updated Jan. 29, 2015, 3:59 p.m.)
>
>
> Review request for accumulo and Eric Newton.
>
>
> Bugs: ACCUMULO-3462 and ACCUMULO-3541
> https://issues.apache.org/jira/browse/ACCUMULO-3462
> https://issues.apache.org/jira/browse/ACCUMULO-3541
>
>
> Repository: accumulo
>
>
> Description
> -------
>
> Fixes compaction logging bug and bug with compaction state not being reset. Patch builds on work already committed for ACCUMULO-3462.
>
> I am slightly nervous about the changes w/ control flow, but I feel this is the best solution to avoid duplicate logging.
>
>
> Diffs
> -----
>
> server/tserver/src/main/java/org/apache/accumulo/tserver/Tablet.java 7420ec4
>
> Diff: https://reviews.apache.org/r/30417/diff/
>
>
> Testing
> -------
>
> Still running ITs. Getting this patch out for review so we can move forward for 1.6.2.
>
>
> Thanks,
>
> kturner
>
>
Re: Review Request 30417: ACCUMULO-3462 ACCUMULO-3541 fixed compaction
logging and state bug
Posted by ke...@deenlo.com.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/30417/
-----------------------------------------------------------
(Updated Jan. 29, 2015, 10:47 p.m.)
Review request for accumulo and Eric Newton.
Changes
-------
Changed code to do nothing w/ span Trace.on fails
Bugs: ACCUMULO-3462 and ACCUMULO-3541
https://issues.apache.org/jira/browse/ACCUMULO-3462
https://issues.apache.org/jira/browse/ACCUMULO-3541
Repository: accumulo
Description
-------
Fixes compaction logging bug and bug with compaction state not being reset. Patch builds on work already committed for ACCUMULO-3462.
I am slightly nervous about the changes w/ control flow, but I feel this is the best solution to avoid duplicate logging.
Diffs (updated)
-----
server/tserver/src/main/java/org/apache/accumulo/tserver/Tablet.java 7420ec4
Diff: https://reviews.apache.org/r/30417/diff/
Testing
-------
Still running ITs. Getting this patch out for review so we can move forward for 1.6.2.
Thanks,
kturner
Re: Review Request 30417: ACCUMULO-3462 ACCUMULO-3541 fixed compaction
logging and state bug
Posted by ke...@deenlo.com.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/30417/
-----------------------------------------------------------
(Updated Jan. 29, 2015, 7:57 p.m.)
Review request for accumulo and Eric Newton.
Changes
-------
Go from 2 to 1 `try{}finally{}` blocks in `majorCompact()`.
Bugs: ACCUMULO-3462 and ACCUMULO-3541
https://issues.apache.org/jira/browse/ACCUMULO-3462
https://issues.apache.org/jira/browse/ACCUMULO-3541
Repository: accumulo
Description
-------
Fixes compaction logging bug and bug with compaction state not being reset. Patch builds on work already committed for ACCUMULO-3462.
I am slightly nervous about the changes w/ control flow, but I feel this is the best solution to avoid duplicate logging.
Diffs (updated)
-----
server/tserver/src/main/java/org/apache/accumulo/tserver/Tablet.java 7420ec4
Diff: https://reviews.apache.org/r/30417/diff/
Testing
-------
Still running ITs. Getting this patch out for review so we can move forward for 1.6.2.
Thanks,
kturner