You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@accumulo.apache.org by Josh Elser <jo...@gmail.com> on 2014/06/02 05:53:22 UTC
Re: Review Request 20525: ACCUMULO-2694 Fix handling of tablet migrations
for offline tables.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/20525/#review44473
-----------------------------------------------------------
server/src/main/java/org/apache/accumulo/server/master/balancer/TabletBalancer.java
<https://reviews.apache.org/r/20525/#comment78890>
Logging the KeyExtents has the potential to leak sensitive data (think inverted index storing terms in the row). Maybe you can log just the tableId for the first 10 migrations instead.
test/system/auto/stress/migrations.py
<https://reviews.apache.org/r/20525/#comment78891>
Just noting (to make sure it doesn't go missing in the merge) that this will need to be migrated to an IT.
- Josh Elser
On May 17, 2014, 9:25 p.m., Sean Busbey wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/20525/
> -----------------------------------------------------------
>
> (Updated May 17, 2014, 9:25 p.m.)
>
>
> Review request for accumulo, Eric Newton and Mike Drob.
>
>
> Bugs: ACCUMULO-2694
> https://issues.apache.org/jira/browse/ACCUMULO-2694
>
>
> Repository: accumulo
>
>
> Description
> -------
>
> ACCUMULO-2694 Fix handling of tablet migrations for offline tables.
>
> * Adds a funtional test that fails due to not rebalancing
> * Fix master to clear migrations when it learns that a table has gone offline
> * Update master to periodically clean up migrations for offline tables
> * Fix balancers to make sure they log if they can't balance.
>
>
> Diffs
> -----
>
> server/pom.xml bd61fe6d870449247cde10ae6ed88c98a31657a2
> server/src/main/java/org/apache/accumulo/server/master/Master.java a2ad2e65e5766d5760199a2c477679f5616a0710
> server/src/main/java/org/apache/accumulo/server/master/balancer/ChaoticLoadBalancer.java e14008a8e4caf72133dd9a08b9aab566bb03d862
> server/src/main/java/org/apache/accumulo/server/master/balancer/DefaultLoadBalancer.java 1fcab4681c32b20d04d8ab9e72d6a61eb6bdddf0
> server/src/main/java/org/apache/accumulo/server/master/balancer/TabletBalancer.java 69387d365e7207b2faaa91805ca2fe8b432e0d49
> test/system/auto/stress/migrations.py d07d7a89ddc930f538f736883d1a3ec43d020d00
>
> Diff: https://reviews.apache.org/r/20525/diff/
>
>
> Testing
> -------
>
> Ran functional test without other changes -> failed. After full patch functional test passes.
>
>
> Thanks,
>
> Sean Busbey
>
>
Re: Review Request 20525: ACCUMULO-2694 Fix handling of tablet migrations
for offline tables.
Posted by Sean Busbey <se...@manvsbeard.com>.
> On June 2, 2014, 3:53 a.m., Josh Elser wrote:
> > server/src/main/java/org/apache/accumulo/server/master/balancer/TabletBalancer.java, line 146
> > <https://reviews.apache.org/r/20525/diff/4/?file=583985#file583985line146>
> >
> > Logging the KeyExtents has the potential to leak sensitive data (think inverted index storing terms in the row). Maybe you can log just the tableId for the first 10 migrations instead.
Good point. I'll include a note about why we're limiting the data as well.
- Sean
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/20525/#review44473
-----------------------------------------------------------
On May 17, 2014, 9:25 p.m., Sean Busbey wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/20525/
> -----------------------------------------------------------
>
> (Updated May 17, 2014, 9:25 p.m.)
>
>
> Review request for accumulo, Eric Newton and Mike Drob.
>
>
> Bugs: ACCUMULO-2694
> https://issues.apache.org/jira/browse/ACCUMULO-2694
>
>
> Repository: accumulo
>
>
> Description
> -------
>
> ACCUMULO-2694 Fix handling of tablet migrations for offline tables.
>
> * Adds a funtional test that fails due to not rebalancing
> * Fix master to clear migrations when it learns that a table has gone offline
> * Update master to periodically clean up migrations for offline tables
> * Fix balancers to make sure they log if they can't balance.
>
>
> Diffs
> -----
>
> server/pom.xml bd61fe6d870449247cde10ae6ed88c98a31657a2
> server/src/main/java/org/apache/accumulo/server/master/Master.java a2ad2e65e5766d5760199a2c477679f5616a0710
> server/src/main/java/org/apache/accumulo/server/master/balancer/ChaoticLoadBalancer.java e14008a8e4caf72133dd9a08b9aab566bb03d862
> server/src/main/java/org/apache/accumulo/server/master/balancer/DefaultLoadBalancer.java 1fcab4681c32b20d04d8ab9e72d6a61eb6bdddf0
> server/src/main/java/org/apache/accumulo/server/master/balancer/TabletBalancer.java 69387d365e7207b2faaa91805ca2fe8b432e0d49
> test/system/auto/stress/migrations.py d07d7a89ddc930f538f736883d1a3ec43d020d00
>
> Diff: https://reviews.apache.org/r/20525/diff/
>
>
> Testing
> -------
>
> Ran functional test without other changes -> failed. After full patch functional test passes.
>
>
> Thanks,
>
> Sean Busbey
>
>
Re: Review Request 20525: ACCUMULO-2694 Fix handling of tablet migrations
for offline tables.
Posted by Josh Elser <jo...@gmail.com>.
> On June 2, 2014, 3:53 a.m., Josh Elser wrote:
> > server/src/main/java/org/apache/accumulo/server/master/balancer/TabletBalancer.java, line 146
> > <https://reviews.apache.org/r/20525/diff/4/?file=583985#file583985line146>
> >
> > Logging the KeyExtents has the potential to leak sensitive data (think inverted index storing terms in the row). Maybe you can log just the tableId for the first 10 migrations instead.
>
> Sean Busbey wrote:
> Good point. I'll include a note about why we're limiting the data as well.
Excellent.
- Josh
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/20525/#review44473
-----------------------------------------------------------
On May 17, 2014, 9:25 p.m., Sean Busbey wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/20525/
> -----------------------------------------------------------
>
> (Updated May 17, 2014, 9:25 p.m.)
>
>
> Review request for accumulo, Eric Newton and Mike Drob.
>
>
> Bugs: ACCUMULO-2694
> https://issues.apache.org/jira/browse/ACCUMULO-2694
>
>
> Repository: accumulo
>
>
> Description
> -------
>
> ACCUMULO-2694 Fix handling of tablet migrations for offline tables.
>
> * Adds a funtional test that fails due to not rebalancing
> * Fix master to clear migrations when it learns that a table has gone offline
> * Update master to periodically clean up migrations for offline tables
> * Fix balancers to make sure they log if they can't balance.
>
>
> Diffs
> -----
>
> server/pom.xml bd61fe6d870449247cde10ae6ed88c98a31657a2
> server/src/main/java/org/apache/accumulo/server/master/Master.java a2ad2e65e5766d5760199a2c477679f5616a0710
> server/src/main/java/org/apache/accumulo/server/master/balancer/ChaoticLoadBalancer.java e14008a8e4caf72133dd9a08b9aab566bb03d862
> server/src/main/java/org/apache/accumulo/server/master/balancer/DefaultLoadBalancer.java 1fcab4681c32b20d04d8ab9e72d6a61eb6bdddf0
> server/src/main/java/org/apache/accumulo/server/master/balancer/TabletBalancer.java 69387d365e7207b2faaa91805ca2fe8b432e0d49
> test/system/auto/stress/migrations.py d07d7a89ddc930f538f736883d1a3ec43d020d00
>
> Diff: https://reviews.apache.org/r/20525/diff/
>
>
> Testing
> -------
>
> Ran functional test without other changes -> failed. After full patch functional test passes.
>
>
> Thanks,
>
> Sean Busbey
>
>
Re: Review Request 20525: ACCUMULO-2694 Fix handling of tablet migrations
for offline tables.
Posted by Josh Elser <jo...@gmail.com>.
> On June 2, 2014, 3:53 a.m., Josh Elser wrote:
> > server/src/main/java/org/apache/accumulo/server/master/balancer/TabletBalancer.java, line 146
> > <https://reviews.apache.org/r/20525/diff/4/?file=583985#file583985line146>
> >
> > Logging the KeyExtents has the potential to leak sensitive data (think inverted index storing terms in the row). Maybe you can log just the tableId for the first 10 migrations instead.
>
> Sean Busbey wrote:
> Good point. I'll include a note about why we're limiting the data as well.
>
> Josh Elser wrote:
> Excellent.
>
> Sean Busbey wrote:
> FYI, we currently output key extents in tons of other places :/
Blech. I'll file a ticket to investigate this.
- Josh
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/20525/#review44473
-----------------------------------------------------------
On May 17, 2014, 9:25 p.m., Sean Busbey wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/20525/
> -----------------------------------------------------------
>
> (Updated May 17, 2014, 9:25 p.m.)
>
>
> Review request for accumulo, Eric Newton and Mike Drob.
>
>
> Bugs: ACCUMULO-2694
> https://issues.apache.org/jira/browse/ACCUMULO-2694
>
>
> Repository: accumulo
>
>
> Description
> -------
>
> ACCUMULO-2694 Fix handling of tablet migrations for offline tables.
>
> * Adds a funtional test that fails due to not rebalancing
> * Fix master to clear migrations when it learns that a table has gone offline
> * Update master to periodically clean up migrations for offline tables
> * Fix balancers to make sure they log if they can't balance.
>
>
> Diffs
> -----
>
> server/pom.xml bd61fe6d870449247cde10ae6ed88c98a31657a2
> server/src/main/java/org/apache/accumulo/server/master/Master.java a2ad2e65e5766d5760199a2c477679f5616a0710
> server/src/main/java/org/apache/accumulo/server/master/balancer/ChaoticLoadBalancer.java e14008a8e4caf72133dd9a08b9aab566bb03d862
> server/src/main/java/org/apache/accumulo/server/master/balancer/DefaultLoadBalancer.java 1fcab4681c32b20d04d8ab9e72d6a61eb6bdddf0
> server/src/main/java/org/apache/accumulo/server/master/balancer/TabletBalancer.java 69387d365e7207b2faaa91805ca2fe8b432e0d49
> test/system/auto/stress/migrations.py d07d7a89ddc930f538f736883d1a3ec43d020d00
>
> Diff: https://reviews.apache.org/r/20525/diff/
>
>
> Testing
> -------
>
> Ran functional test without other changes -> failed. After full patch functional test passes.
>
>
> Thanks,
>
> Sean Busbey
>
>
Re: Review Request 20525: ACCUMULO-2694 Fix handling of tablet migrations
for offline tables.
Posted by Sean Busbey <se...@manvsbeard.com>.
> On June 2, 2014, 3:53 a.m., Josh Elser wrote:
> > server/src/main/java/org/apache/accumulo/server/master/balancer/TabletBalancer.java, line 146
> > <https://reviews.apache.org/r/20525/diff/4/?file=583985#file583985line146>
> >
> > Logging the KeyExtents has the potential to leak sensitive data (think inverted index storing terms in the row). Maybe you can log just the tableId for the first 10 migrations instead.
>
> Sean Busbey wrote:
> Good point. I'll include a note about why we're limiting the data as well.
>
> Josh Elser wrote:
> Excellent.
>
> Sean Busbey wrote:
> FYI, we currently output key extents in tons of other places :/
>
> Josh Elser wrote:
> Blech. I'll file a ticket to investigate this.
Does that mean I can leave this log message in place and have it dealt with as a part of the follow on? :)
- Sean
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/20525/#review44473
-----------------------------------------------------------
On May 17, 2014, 9:25 p.m., Sean Busbey wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/20525/
> -----------------------------------------------------------
>
> (Updated May 17, 2014, 9:25 p.m.)
>
>
> Review request for accumulo, Eric Newton and Mike Drob.
>
>
> Bugs: ACCUMULO-2694
> https://issues.apache.org/jira/browse/ACCUMULO-2694
>
>
> Repository: accumulo
>
>
> Description
> -------
>
> ACCUMULO-2694 Fix handling of tablet migrations for offline tables.
>
> * Adds a funtional test that fails due to not rebalancing
> * Fix master to clear migrations when it learns that a table has gone offline
> * Update master to periodically clean up migrations for offline tables
> * Fix balancers to make sure they log if they can't balance.
>
>
> Diffs
> -----
>
> server/pom.xml bd61fe6d870449247cde10ae6ed88c98a31657a2
> server/src/main/java/org/apache/accumulo/server/master/Master.java a2ad2e65e5766d5760199a2c477679f5616a0710
> server/src/main/java/org/apache/accumulo/server/master/balancer/ChaoticLoadBalancer.java e14008a8e4caf72133dd9a08b9aab566bb03d862
> server/src/main/java/org/apache/accumulo/server/master/balancer/DefaultLoadBalancer.java 1fcab4681c32b20d04d8ab9e72d6a61eb6bdddf0
> server/src/main/java/org/apache/accumulo/server/master/balancer/TabletBalancer.java 69387d365e7207b2faaa91805ca2fe8b432e0d49
> test/system/auto/stress/migrations.py d07d7a89ddc930f538f736883d1a3ec43d020d00
>
> Diff: https://reviews.apache.org/r/20525/diff/
>
>
> Testing
> -------
>
> Ran functional test without other changes -> failed. After full patch functional test passes.
>
>
> Thanks,
>
> Sean Busbey
>
>
Re: Review Request 20525: ACCUMULO-2694 Fix handling of tablet migrations
for offline tables.
Posted by Josh Elser <jo...@gmail.com>.
> On June 2, 2014, 3:53 a.m., Josh Elser wrote:
> > server/src/main/java/org/apache/accumulo/server/master/balancer/TabletBalancer.java, line 146
> > <https://reviews.apache.org/r/20525/diff/4/?file=583985#file583985line146>
> >
> > Logging the KeyExtents has the potential to leak sensitive data (think inverted index storing terms in the row). Maybe you can log just the tableId for the first 10 migrations instead.
>
> Sean Busbey wrote:
> Good point. I'll include a note about why we're limiting the data as well.
>
> Josh Elser wrote:
> Excellent.
>
> Sean Busbey wrote:
> FYI, we currently output key extents in tons of other places :/
>
> Josh Elser wrote:
> Blech. I'll file a ticket to investigate this.
>
> Sean Busbey wrote:
> Does that mean I can leave this log message in place and have it dealt with as a part of the follow on? :)
I'd rather it just be trounced now, but I won't -1 you if you don't.
- Josh
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/20525/#review44473
-----------------------------------------------------------
On May 17, 2014, 9:25 p.m., Sean Busbey wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/20525/
> -----------------------------------------------------------
>
> (Updated May 17, 2014, 9:25 p.m.)
>
>
> Review request for accumulo, Eric Newton and Mike Drob.
>
>
> Bugs: ACCUMULO-2694
> https://issues.apache.org/jira/browse/ACCUMULO-2694
>
>
> Repository: accumulo
>
>
> Description
> -------
>
> ACCUMULO-2694 Fix handling of tablet migrations for offline tables.
>
> * Adds a funtional test that fails due to not rebalancing
> * Fix master to clear migrations when it learns that a table has gone offline
> * Update master to periodically clean up migrations for offline tables
> * Fix balancers to make sure they log if they can't balance.
>
>
> Diffs
> -----
>
> server/pom.xml bd61fe6d870449247cde10ae6ed88c98a31657a2
> server/src/main/java/org/apache/accumulo/server/master/Master.java a2ad2e65e5766d5760199a2c477679f5616a0710
> server/src/main/java/org/apache/accumulo/server/master/balancer/ChaoticLoadBalancer.java e14008a8e4caf72133dd9a08b9aab566bb03d862
> server/src/main/java/org/apache/accumulo/server/master/balancer/DefaultLoadBalancer.java 1fcab4681c32b20d04d8ab9e72d6a61eb6bdddf0
> server/src/main/java/org/apache/accumulo/server/master/balancer/TabletBalancer.java 69387d365e7207b2faaa91805ca2fe8b432e0d49
> test/system/auto/stress/migrations.py d07d7a89ddc930f538f736883d1a3ec43d020d00
>
> Diff: https://reviews.apache.org/r/20525/diff/
>
>
> Testing
> -------
>
> Ran functional test without other changes -> failed. After full patch functional test passes.
>
>
> Thanks,
>
> Sean Busbey
>
>
Re: Review Request 20525: ACCUMULO-2694 Fix handling of tablet migrations
for offline tables.
Posted by Sean Busbey <se...@manvsbeard.com>.
> On June 2, 2014, 3:53 a.m., Josh Elser wrote:
> > server/src/main/java/org/apache/accumulo/server/master/balancer/TabletBalancer.java, line 146
> > <https://reviews.apache.org/r/20525/diff/4/?file=583985#file583985line146>
> >
> > Logging the KeyExtents has the potential to leak sensitive data (think inverted index storing terms in the row). Maybe you can log just the tableId for the first 10 migrations instead.
>
> Sean Busbey wrote:
> Good point. I'll include a note about why we're limiting the data as well.
>
> Josh Elser wrote:
> Excellent.
>
> Sean Busbey wrote:
> FYI, we currently output key extents in tons of other places :/
>
> Josh Elser wrote:
> Blech. I'll file a ticket to investigate this.
>
> Sean Busbey wrote:
> Does that mean I can leave this log message in place and have it dealt with as a part of the follow on? :)
>
> Josh Elser wrote:
> I'd rather it just be trounced now, but I won't -1 you if you don't.
I'd rather increase the priority and fixVersion for ACCUMULO-2938 than hack something in for just this one location and then undo it for that ticket.
- Sean
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/20525/#review44473
-----------------------------------------------------------
On May 17, 2014, 9:25 p.m., Sean Busbey wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/20525/
> -----------------------------------------------------------
>
> (Updated May 17, 2014, 9:25 p.m.)
>
>
> Review request for accumulo, Eric Newton and Mike Drob.
>
>
> Bugs: ACCUMULO-2694
> https://issues.apache.org/jira/browse/ACCUMULO-2694
>
>
> Repository: accumulo
>
>
> Description
> -------
>
> ACCUMULO-2694 Fix handling of tablet migrations for offline tables.
>
> * Adds a funtional test that fails due to not rebalancing
> * Fix master to clear migrations when it learns that a table has gone offline
> * Update master to periodically clean up migrations for offline tables
> * Fix balancers to make sure they log if they can't balance.
>
>
> Diffs
> -----
>
> server/pom.xml bd61fe6d870449247cde10ae6ed88c98a31657a2
> server/src/main/java/org/apache/accumulo/server/master/Master.java a2ad2e65e5766d5760199a2c477679f5616a0710
> server/src/main/java/org/apache/accumulo/server/master/balancer/ChaoticLoadBalancer.java e14008a8e4caf72133dd9a08b9aab566bb03d862
> server/src/main/java/org/apache/accumulo/server/master/balancer/DefaultLoadBalancer.java 1fcab4681c32b20d04d8ab9e72d6a61eb6bdddf0
> server/src/main/java/org/apache/accumulo/server/master/balancer/TabletBalancer.java 69387d365e7207b2faaa91805ca2fe8b432e0d49
> test/system/auto/stress/migrations.py d07d7a89ddc930f538f736883d1a3ec43d020d00
>
> Diff: https://reviews.apache.org/r/20525/diff/
>
>
> Testing
> -------
>
> Ran functional test without other changes -> failed. After full patch functional test passes.
>
>
> Thanks,
>
> Sean Busbey
>
>
Re: Review Request 20525: ACCUMULO-2694 Fix handling of tablet migrations
for offline tables.
Posted by Sean Busbey <se...@manvsbeard.com>.
> On June 2, 2014, 3:53 a.m., Josh Elser wrote:
> > server/src/main/java/org/apache/accumulo/server/master/balancer/TabletBalancer.java, line 146
> > <https://reviews.apache.org/r/20525/diff/4/?file=583985#file583985line146>
> >
> > Logging the KeyExtents has the potential to leak sensitive data (think inverted index storing terms in the row). Maybe you can log just the tableId for the first 10 migrations instead.
>
> Sean Busbey wrote:
> Good point. I'll include a note about why we're limiting the data as well.
>
> Josh Elser wrote:
> Excellent.
FYI, we currently output key extents in tons of other places :/
- Sean
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/20525/#review44473
-----------------------------------------------------------
On May 17, 2014, 9:25 p.m., Sean Busbey wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/20525/
> -----------------------------------------------------------
>
> (Updated May 17, 2014, 9:25 p.m.)
>
>
> Review request for accumulo, Eric Newton and Mike Drob.
>
>
> Bugs: ACCUMULO-2694
> https://issues.apache.org/jira/browse/ACCUMULO-2694
>
>
> Repository: accumulo
>
>
> Description
> -------
>
> ACCUMULO-2694 Fix handling of tablet migrations for offline tables.
>
> * Adds a funtional test that fails due to not rebalancing
> * Fix master to clear migrations when it learns that a table has gone offline
> * Update master to periodically clean up migrations for offline tables
> * Fix balancers to make sure they log if they can't balance.
>
>
> Diffs
> -----
>
> server/pom.xml bd61fe6d870449247cde10ae6ed88c98a31657a2
> server/src/main/java/org/apache/accumulo/server/master/Master.java a2ad2e65e5766d5760199a2c477679f5616a0710
> server/src/main/java/org/apache/accumulo/server/master/balancer/ChaoticLoadBalancer.java e14008a8e4caf72133dd9a08b9aab566bb03d862
> server/src/main/java/org/apache/accumulo/server/master/balancer/DefaultLoadBalancer.java 1fcab4681c32b20d04d8ab9e72d6a61eb6bdddf0
> server/src/main/java/org/apache/accumulo/server/master/balancer/TabletBalancer.java 69387d365e7207b2faaa91805ca2fe8b432e0d49
> test/system/auto/stress/migrations.py d07d7a89ddc930f538f736883d1a3ec43d020d00
>
> Diff: https://reviews.apache.org/r/20525/diff/
>
>
> Testing
> -------
>
> Ran functional test without other changes -> failed. After full patch functional test passes.
>
>
> Thanks,
>
> Sean Busbey
>
>