You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@accumulo.apache.org by Sean Busbey <se...@manvsbeard.com> on 2014/04/21 20:15:07 UTC
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/
-----------------------------------------------------------
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
-----
src/server/pom.xml dbe4fb4
src/server/src/main/java/org/apache/accumulo/server/master/Master.java fb7be51
src/server/src/main/java/org/apache/accumulo/server/master/balancer/ChaoticLoadBalancer.java 02a4e89
src/server/src/main/java/org/apache/accumulo/server/master/balancer/DefaultLoadBalancer.java 4826097
src/server/src/main/java/org/apache/accumulo/server/master/balancer/TabletBalancer.java ad62360
test/system/auto/stress/migrations.py d07d7a8
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 Mike Drob <md...@mdrob.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/20525/#review41352
-----------------------------------------------------------
src/server/src/main/java/org/apache/accumulo/server/master/Master.java
<https://reviews.apache.org/r/20525/#comment74773>
Testability - accept connector as method argument.
- Mike Drob
On April 24, 2014, 3:43 p.m., Sean Busbey wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/20525/
> -----------------------------------------------------------
>
> (Updated April 24, 2014, 3:43 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
> -----
>
> src/server/pom.xml dbe4fb4
> src/server/src/main/java/org/apache/accumulo/server/master/Master.java fb7be51
> src/server/src/main/java/org/apache/accumulo/server/master/balancer/ChaoticLoadBalancer.java 02a4e89
> src/server/src/main/java/org/apache/accumulo/server/master/balancer/DefaultLoadBalancer.java 4826097
> src/server/src/main/java/org/apache/accumulo/server/master/balancer/TabletBalancer.java ad62360
> test/system/auto/stress/migrations.py d07d7a8
>
> 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 April 24, 2014, 4:31 p.m., Christopher Tubbs wrote:
> > src/server/pom.xml, line 94
> > <https://reviews.apache.org/r/20525/diff/3/?file=567770#file567770line94>
> >
> > default scope is compile. omit this line, and make sure to run "mvn clean -P sortpom" to ensure pom changes are sorted.
>
> Sean Busbey wrote:
> we have a parent pom dependency management section that makes the scope test for guava.
the sortpom profile doesn't exist in 1.4.6-SNAPSHOT, but I'll keep this in mind when merging forward.
- Sean
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/20525/#review41319
-----------------------------------------------------------
On April 24, 2014, 3:43 p.m., Sean Busbey wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/20525/
> -----------------------------------------------------------
>
> (Updated April 24, 2014, 3:43 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
> -----
>
> src/server/pom.xml dbe4fb4
> src/server/src/main/java/org/apache/accumulo/server/master/Master.java fb7be51
> src/server/src/main/java/org/apache/accumulo/server/master/balancer/ChaoticLoadBalancer.java 02a4e89
> src/server/src/main/java/org/apache/accumulo/server/master/balancer/DefaultLoadBalancer.java 4826097
> src/server/src/main/java/org/apache/accumulo/server/master/balancer/TabletBalancer.java ad62360
> test/system/auto/stress/migrations.py d07d7a8
>
> 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 Christopher Tubbs <ct...@apache.org>.
> On April 24, 2014, 12:31 p.m., Christopher Tubbs wrote:
> > src/server/pom.xml, line 94
> > <https://reviews.apache.org/r/20525/diff/3/?file=567770#file567770line94>
> >
> > default scope is compile. omit this line, and make sure to run "mvn clean -P sortpom" to ensure pom changes are sorted.
>
> Sean Busbey wrote:
> we have a parent pom dependency management section that makes the scope test for guava.
>
> Sean Busbey wrote:
> the sortpom profile doesn't exist in 1.4.6-SNAPSHOT, but I'll keep this in mind when merging forward.
FYI, it's bad practice to put scopes in the dependencyManagement sections. It's best to use the default scope there, and specify a more specific scope in the child POMs, where it's actually used.
- Christopher
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/20525/#review41319
-----------------------------------------------------------
On April 24, 2014, 11:43 a.m., Sean Busbey wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/20525/
> -----------------------------------------------------------
>
> (Updated April 24, 2014, 11:43 a.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
> -----
>
> src/server/pom.xml dbe4fb4
> src/server/src/main/java/org/apache/accumulo/server/master/Master.java fb7be51
> src/server/src/main/java/org/apache/accumulo/server/master/balancer/ChaoticLoadBalancer.java 02a4e89
> src/server/src/main/java/org/apache/accumulo/server/master/balancer/DefaultLoadBalancer.java 4826097
> src/server/src/main/java/org/apache/accumulo/server/master/balancer/TabletBalancer.java ad62360
> test/system/auto/stress/migrations.py d07d7a8
>
> 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 April 24, 2014, 4:31 p.m., Christopher Tubbs wrote:
> > src/server/pom.xml, line 94
> > <https://reviews.apache.org/r/20525/diff/3/?file=567770#file567770line94>
> >
> > default scope is compile. omit this line, and make sure to run "mvn clean -P sortpom" to ensure pom changes are sorted.
we have a parent pom dependency management section that makes the scope test for guava.
- Sean
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/20525/#review41319
-----------------------------------------------------------
On April 24, 2014, 3:43 p.m., Sean Busbey wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/20525/
> -----------------------------------------------------------
>
> (Updated April 24, 2014, 3:43 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
> -----
>
> src/server/pom.xml dbe4fb4
> src/server/src/main/java/org/apache/accumulo/server/master/Master.java fb7be51
> src/server/src/main/java/org/apache/accumulo/server/master/balancer/ChaoticLoadBalancer.java 02a4e89
> src/server/src/main/java/org/apache/accumulo/server/master/balancer/DefaultLoadBalancer.java 4826097
> src/server/src/main/java/org/apache/accumulo/server/master/balancer/TabletBalancer.java ad62360
> test/system/auto/stress/migrations.py d07d7a8
>
> 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 Christopher Tubbs <ct...@apache.org>.
> On April 24, 2014, 12:31 p.m., Christopher Tubbs wrote:
> > src/server/pom.xml, line 94
> > <https://reviews.apache.org/r/20525/diff/3/?file=567770#file567770line94>
> >
> > default scope is compile. omit this line, and make sure to run "mvn clean -P sortpom" to ensure pom changes are sorted.
>
> Sean Busbey wrote:
> we have a parent pom dependency management section that makes the scope test for guava.
>
> Sean Busbey wrote:
> the sortpom profile doesn't exist in 1.4.6-SNAPSHOT, but I'll keep this in mind when merging forward.
>
> Christopher Tubbs wrote:
> FYI, it's bad practice to put scopes in the dependencyManagement sections. It's best to use the default scope there, and specify a more specific scope in the child POMs, where it's actually used.
>
> Sean Busbey wrote:
> I agree. I'm just trying to limit the scope for this ticket.
Cool. As long as you're cognizent for the merge to 1.5 and later, I see no problem with this in 1.4
- Christopher
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/20525/#review41319
-----------------------------------------------------------
On April 24, 2014, 11:43 a.m., Sean Busbey wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/20525/
> -----------------------------------------------------------
>
> (Updated April 24, 2014, 11:43 a.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
> -----
>
> src/server/pom.xml dbe4fb4
> src/server/src/main/java/org/apache/accumulo/server/master/Master.java fb7be51
> src/server/src/main/java/org/apache/accumulo/server/master/balancer/ChaoticLoadBalancer.java 02a4e89
> src/server/src/main/java/org/apache/accumulo/server/master/balancer/DefaultLoadBalancer.java 4826097
> src/server/src/main/java/org/apache/accumulo/server/master/balancer/TabletBalancer.java ad62360
> test/system/auto/stress/migrations.py d07d7a8
>
> 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 April 24, 2014, 4:31 p.m., Christopher Tubbs wrote:
> > src/server/pom.xml, line 94
> > <https://reviews.apache.org/r/20525/diff/3/?file=567770#file567770line94>
> >
> > default scope is compile. omit this line, and make sure to run "mvn clean -P sortpom" to ensure pom changes are sorted.
>
> Sean Busbey wrote:
> we have a parent pom dependency management section that makes the scope test for guava.
>
> Sean Busbey wrote:
> the sortpom profile doesn't exist in 1.4.6-SNAPSHOT, but I'll keep this in mind when merging forward.
>
> Christopher Tubbs wrote:
> FYI, it's bad practice to put scopes in the dependencyManagement sections. It's best to use the default scope there, and specify a more specific scope in the child POMs, where it's actually used.
I agree. I'm just trying to limit the scope for this ticket.
- Sean
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/20525/#review41319
-----------------------------------------------------------
On April 24, 2014, 3:43 p.m., Sean Busbey wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/20525/
> -----------------------------------------------------------
>
> (Updated April 24, 2014, 3:43 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
> -----
>
> src/server/pom.xml dbe4fb4
> src/server/src/main/java/org/apache/accumulo/server/master/Master.java fb7be51
> src/server/src/main/java/org/apache/accumulo/server/master/balancer/ChaoticLoadBalancer.java 02a4e89
> src/server/src/main/java/org/apache/accumulo/server/master/balancer/DefaultLoadBalancer.java 4826097
> src/server/src/main/java/org/apache/accumulo/server/master/balancer/TabletBalancer.java ad62360
> test/system/auto/stress/migrations.py d07d7a8
>
> 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 Christopher Tubbs <ct...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/20525/#review41319
-----------------------------------------------------------
src/server/pom.xml
<https://reviews.apache.org/r/20525/#comment74744>
default scope is compile. omit this line, and make sure to run "mvn clean -P sortpom" to ensure pom changes are sorted.
- Christopher Tubbs
On April 24, 2014, 11:43 a.m., Sean Busbey wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/20525/
> -----------------------------------------------------------
>
> (Updated April 24, 2014, 11:43 a.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
> -----
>
> src/server/pom.xml dbe4fb4
> src/server/src/main/java/org/apache/accumulo/server/master/Master.java fb7be51
> src/server/src/main/java/org/apache/accumulo/server/master/balancer/ChaoticLoadBalancer.java 02a4e89
> src/server/src/main/java/org/apache/accumulo/server/master/balancer/DefaultLoadBalancer.java 4826097
> src/server/src/main/java/org/apache/accumulo/server/master/balancer/TabletBalancer.java ad62360
> test/system/auto/stress/migrations.py d07d7a8
>
> 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
>
>
Re: Review Request 20525: ACCUMULO-2694 Fix handling of tablet migrations
for offline tables.
Posted by Josh Elser <jo...@gmail.com>.
-----------------------------------------------------------
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, 2:01 p.m., Eric Newton wrote:
> > server/src/main/java/org/apache/accumulo/server/master/balancer/TabletBalancer.java, line 98
> > <https://reviews.apache.org/r/20525/diff/4/?file=583985#file583985line98>
> >
> > Nit: spelling: deferred, can't
> >
fixed.
- Sean
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/20525/#review44494
-----------------------------------------------------------
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 Eric Newton <er...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/20525/#review44494
-----------------------------------------------------------
server/src/main/java/org/apache/accumulo/server/master/balancer/TabletBalancer.java
<https://reviews.apache.org/r/20525/#comment78896>
Nit: spelling: deferred, can't
- Eric Newton
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 23, 2014, 9:43 p.m., Bill Havanki wrote:
> > server/src/main/java/org/apache/accumulo/server/master/balancer/TabletBalancer.java, lines 157-158
> > <https://reviews.apache.org/r/20525/diff/6/?file=615223#file615223line157>
> >
> > When constraintNotMet() transitions to stuck = true, it won't run the cause to log it; i.e., the first cause in a series won't get logged.
Correct, it's not supposed to log until the problem has persisted for the threshold period.
- Sean
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/20525/#review46455
-----------------------------------------------------------
On June 23, 2014, 9:18 p.m., Sean Busbey wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/20525/
> -----------------------------------------------------------
>
> (Updated June 23, 2014, 9:18 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 Bill Havanki <bh...@clouderagovt.com>.
> On June 23, 2014, 5:43 p.m., Bill Havanki wrote:
> > server/src/main/java/org/apache/accumulo/server/master/balancer/TabletBalancer.java, lines 157-158
> > <https://reviews.apache.org/r/20525/diff/6/?file=615223#file615223line157>
> >
> > When constraintNotMet() transitions to stuck = true, it won't run the cause to log it; i.e., the first cause in a series won't get logged.
>
> Sean Busbey wrote:
> Correct, it's not supposed to log until the problem has persisted for the threshold period.
OK. I was concerned that this would lead to under-reporting - for example, a 59-second long problem would never be logged. But, if the nature of this is that it's fine to not mention more transient problems (say, a 30-second long problem that flip-flops), I'm good.
- Bill
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/20525/#review46455
-----------------------------------------------------------
On June 23, 2014, 5:18 p.m., Sean Busbey wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/20525/
> -----------------------------------------------------------
>
> (Updated June 23, 2014, 5:18 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 Bill Havanki <bh...@clouderagovt.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/20525/#review46455
-----------------------------------------------------------
server/src/main/java/org/apache/accumulo/server/master/balancer/TabletBalancer.java
<https://reviews.apache.org/r/20525/#comment81847>
When constraintNotMet() transitions to stuck = true, it won't run the cause to log it; i.e., the first cause in a series won't get logged.
- Bill Havanki
On June 23, 2014, 5:18 p.m., Sean Busbey wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/20525/
> -----------------------------------------------------------
>
> (Updated June 23, 2014, 5:18 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>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/20525/#review46799
-----------------------------------------------------------
Ship it!
Ship it w/ a friendly reminder to convert the python test into an IT after the merge.
- Josh Elser
On June 23, 2014, 9:18 p.m., Sean Busbey wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/20525/
> -----------------------------------------------------------
>
> (Updated June 23, 2014, 9:18 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>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/20525/
-----------------------------------------------------------
(Updated June 23, 2014, 9:18 p.m.)
Review request for accumulo, Eric Newton and Mike Drob.
Changes
-------
more typos
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 (updated)
-----
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 Mike Drob <md...@mdrob.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/20525/#review46448
-----------------------------------------------------------
server/src/main/java/org/apache/accumulo/server/master/balancer/TabletBalancer.java
<https://reviews.apache.org/r/20525/#comment81842>
spelling: there
- Mike Drob
On June 23, 2014, 9:02 p.m., Sean Busbey wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/20525/
> -----------------------------------------------------------
>
> (Updated June 23, 2014, 9:02 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>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/20525/
-----------------------------------------------------------
(Updated June 23, 2014, 9:02 p.m.)
Review request for accumulo, Eric Newton and Mike Drob.
Changes
-------
rebased to current. fixed typos. added TODO for ACCUMULO-2938.
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 (updated)
-----
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>.
-----------------------------------------------------------
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.
Changes
-------
Rebased to 1.5 dev line. Incorporated feedback from Christopher and Mike.
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 (updated)
-----
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>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/20525/
-----------------------------------------------------------
(Updated April 24, 2014, 3:43 p.m.)
Review request for accumulo, Eric Newton and Mike Drob.
Changes
-------
incorporated feedback from ecn.
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 (updated)
-----
src/server/pom.xml dbe4fb4
src/server/src/main/java/org/apache/accumulo/server/master/Master.java fb7be51
src/server/src/main/java/org/apache/accumulo/server/master/balancer/ChaoticLoadBalancer.java 02a4e89
src/server/src/main/java/org/apache/accumulo/server/master/balancer/DefaultLoadBalancer.java 4826097
src/server/src/main/java/org/apache/accumulo/server/master/balancer/TabletBalancer.java ad62360
test/system/auto/stress/migrations.py d07d7a8
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 April 22, 2014, 2:27 p.m., Eric Newton wrote:
> > src/server/src/main/java/org/apache/accumulo/server/master/balancer/DefaultLoadBalancer.java, line 317
> > <https://reviews.apache.org/r/20525/diff/2/?file=563456#file563456line317>
> >
> > Duplicate code. Create a public class, add a setter for migrations (or provide in the ctor).
I had started to do this, as helper classes in TabletBalancer. The issue I ran into was then the wrong logger is in scope. That is, I want to know the DefaultLoadBalancer or the ChaoticBalancer has a problem, not just that the top level TabletBalancer.
I could pass a Logger into the constructor, but it looked super awkward in comparison to the duplicated code. preferable?
- Sean
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/20525/#review41013
-----------------------------------------------------------
On April 21, 2014, 9:32 p.m., Sean Busbey wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/20525/
> -----------------------------------------------------------
>
> (Updated April 21, 2014, 9:32 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
> -----
>
> src/server/pom.xml dbe4fb4
> src/server/src/main/java/org/apache/accumulo/server/master/Master.java fb7be51
> src/server/src/main/java/org/apache/accumulo/server/master/balancer/ChaoticLoadBalancer.java 02a4e89
> src/server/src/main/java/org/apache/accumulo/server/master/balancer/DefaultLoadBalancer.java 4826097
> src/server/src/main/java/org/apache/accumulo/server/master/balancer/TabletBalancer.java ad62360
> test/system/auto/stress/migrations.py d07d7a8
>
> 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 Bill Havanki <bh...@clouderagovt.com>.
> On April 22, 2014, 10:27 a.m., Eric Newton wrote:
> > src/server/src/main/java/org/apache/accumulo/server/master/balancer/DefaultLoadBalancer.java, line 317
> > <https://reviews.apache.org/r/20525/diff/2/?file=563456#file563456line317>
> >
> > Duplicate code. Create a public class, add a setter for migrations (or provide in the ctor).
>
> Sean Busbey wrote:
> I had started to do this, as helper classes in TabletBalancer. The issue I ran into was then the wrong logger is in scope. That is, I want to know the DefaultLoadBalancer or the ChaoticBalancer has a problem, not just that the top level TabletBalancer.
>
> I could pass a Logger into the constructor, but it looked super awkward in comparison to the duplicated code. preferable?
Since the point of running a BalancerProblem is to log, I think passing a logger would be OK. You could also pass the balancer to it and have the balancer allow logging through it, that's sorta complicated though.
- Bill
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/20525/#review41013
-----------------------------------------------------------
On April 21, 2014, 5:32 p.m., Sean Busbey wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/20525/
> -----------------------------------------------------------
>
> (Updated April 21, 2014, 5:32 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
> -----
>
> src/server/pom.xml dbe4fb4
> src/server/src/main/java/org/apache/accumulo/server/master/Master.java fb7be51
> src/server/src/main/java/org/apache/accumulo/server/master/balancer/ChaoticLoadBalancer.java 02a4e89
> src/server/src/main/java/org/apache/accumulo/server/master/balancer/DefaultLoadBalancer.java 4826097
> src/server/src/main/java/org/apache/accumulo/server/master/balancer/TabletBalancer.java ad62360
> test/system/auto/stress/migrations.py d07d7a8
>
> 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 April 22, 2014, 2:27 p.m., Eric Newton wrote:
> > src/server/src/main/java/org/apache/accumulo/server/master/balancer/DefaultLoadBalancer.java, line 315
> > <https://reviews.apache.org/r/20525/diff/2/?file=563456#file563456line315>
> >
> > noservers is constant, should be NO_SERVERS.
fixed
> On April 22, 2014, 2:27 p.m., Eric Newton wrote:
> > src/server/src/main/java/org/apache/accumulo/server/master/balancer/DefaultLoadBalancer.java, line 317
> > <https://reviews.apache.org/r/20525/diff/2/?file=563456#file563456line317>
> >
> > Duplicate code. Create a public class, add a setter for migrations (or provide in the ctor).
>
> Sean Busbey wrote:
> I had started to do this, as helper classes in TabletBalancer. The issue I ran into was then the wrong logger is in scope. That is, I want to know the DefaultLoadBalancer or the ChaoticBalancer has a problem, not just that the top level TabletBalancer.
>
> I could pass a Logger into the constructor, but it looked super awkward in comparison to the duplicated code. preferable?
>
> Bill Havanki wrote:
> Since the point of running a BalancerProblem is to log, I think passing a logger would be OK. You could also pass the balancer to it and have the balancer allow logging through it, that's sorta complicated though.
moved to TabletBalancer and passed Logger. kept it protected, since these are only needed by subclasses of TabletBalancer.
> On April 22, 2014, 2:27 p.m., Eric Newton wrote:
> > src/server/src/main/java/org/apache/accumulo/server/master/balancer/DefaultLoadBalancer.java, line 333
> > <https://reviews.apache.org/r/20525/diff/2/?file=563456#file563456line333>
> >
> > At this point balancing has not been performed, it could throw an error and balanceSuccessful doesn't really reflect a successful balance call. Rename to resetBalanceError, or move to post-balance.
renamed to resetBalancerErrors
- Sean
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/20525/#review41013
-----------------------------------------------------------
On April 21, 2014, 9:32 p.m., Sean Busbey wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/20525/
> -----------------------------------------------------------
>
> (Updated April 21, 2014, 9:32 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
> -----
>
> src/server/pom.xml dbe4fb4
> src/server/src/main/java/org/apache/accumulo/server/master/Master.java fb7be51
> src/server/src/main/java/org/apache/accumulo/server/master/balancer/ChaoticLoadBalancer.java 02a4e89
> src/server/src/main/java/org/apache/accumulo/server/master/balancer/DefaultLoadBalancer.java 4826097
> src/server/src/main/java/org/apache/accumulo/server/master/balancer/TabletBalancer.java ad62360
> test/system/auto/stress/migrations.py d07d7a8
>
> 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 Eric Newton <er...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/20525/#review41013
-----------------------------------------------------------
src/server/src/main/java/org/apache/accumulo/server/master/balancer/DefaultLoadBalancer.java
<https://reviews.apache.org/r/20525/#comment74372>
noservers is constant, should be NO_SERVERS.
src/server/src/main/java/org/apache/accumulo/server/master/balancer/DefaultLoadBalancer.java
<https://reviews.apache.org/r/20525/#comment74370>
Duplicate code. Create a public class, add a setter for migrations (or provide in the ctor).
src/server/src/main/java/org/apache/accumulo/server/master/balancer/DefaultLoadBalancer.java
<https://reviews.apache.org/r/20525/#comment74371>
At this point balancing has not been performed, it could throw an error and balanceSuccessful doesn't really reflect a successful balance call. Rename to resetBalanceError, or move to post-balance.
- Eric Newton
On April 21, 2014, 9:32 p.m., Sean Busbey wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/20525/
> -----------------------------------------------------------
>
> (Updated April 21, 2014, 9:32 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
> -----
>
> src/server/pom.xml dbe4fb4
> src/server/src/main/java/org/apache/accumulo/server/master/Master.java fb7be51
> src/server/src/main/java/org/apache/accumulo/server/master/balancer/ChaoticLoadBalancer.java 02a4e89
> src/server/src/main/java/org/apache/accumulo/server/master/balancer/DefaultLoadBalancer.java 4826097
> src/server/src/main/java/org/apache/accumulo/server/master/balancer/TabletBalancer.java ad62360
> test/system/auto/stress/migrations.py d07d7a8
>
> 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>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/20525/
-----------------------------------------------------------
(Updated April 21, 2014, 9:32 p.m.)
Review request for accumulo, Eric Newton and Mike Drob.
Changes
-------
Limit the frequency of warnings to avoid transient blockage and flooding messages.
Previous patch:
$ grep -i "WARN : not balancing" logs/host1-9873/master_host1.example.com.log | grep -i "outstanding migrations" | wc -l
816
And now:
$ grep -i "WARN : not balancing" logs/host1-19121/master_host1.example.com.log | grep -i "outstanding migrations" | wc -l
12
The test is set to use per-table balancers and has 3 tables, so that 12 count represents four minutes with a set of 3 messages per minute.
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 (updated)
-----
src/server/pom.xml dbe4fb4
src/server/src/main/java/org/apache/accumulo/server/master/Master.java fb7be51
src/server/src/main/java/org/apache/accumulo/server/master/balancer/ChaoticLoadBalancer.java 02a4e89
src/server/src/main/java/org/apache/accumulo/server/master/balancer/DefaultLoadBalancer.java 4826097
src/server/src/main/java/org/apache/accumulo/server/master/balancer/TabletBalancer.java ad62360
test/system/auto/stress/migrations.py d07d7a8
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 April 21, 2014, 6:20 p.m., Mike Drob wrote:
> > src/server/src/main/java/org/apache/accumulo/server/master/balancer/DefaultLoadBalancer.java, line 317
> > <https://reviews.apache.org/r/20525/diff/1/?file=563229#file563229line317>
> >
> > Can this cause a lot of unnecessary logs?
>
> Sean Busbey wrote:
> When migrations are moving slowly for some reason, it would produce a message every ~5 seconds. the monitor would roll them up into a single message, but the log4j would have no such dampening. I could put in a time threshold, if you'd prefer to keep that down.
in the case of ACCUMULO-2694, that's a max of 60 messages before self correction. That seems excessive. once a minute sound better?
- Sean
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/20525/#review40907
-----------------------------------------------------------
On April 21, 2014, 6:15 p.m., Sean Busbey wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/20525/
> -----------------------------------------------------------
>
> (Updated April 21, 2014, 6:15 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
> -----
>
> src/server/pom.xml dbe4fb4
> src/server/src/main/java/org/apache/accumulo/server/master/Master.java fb7be51
> src/server/src/main/java/org/apache/accumulo/server/master/balancer/ChaoticLoadBalancer.java 02a4e89
> src/server/src/main/java/org/apache/accumulo/server/master/balancer/DefaultLoadBalancer.java 4826097
> src/server/src/main/java/org/apache/accumulo/server/master/balancer/TabletBalancer.java ad62360
> test/system/auto/stress/migrations.py d07d7a8
>
> 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 April 21, 2014, 6:20 p.m., Mike Drob wrote:
> > src/server/src/main/java/org/apache/accumulo/server/master/balancer/DefaultLoadBalancer.java, line 317
> > <https://reviews.apache.org/r/20525/diff/1/?file=563229#file563229line317>
> >
> > Can this cause a lot of unnecessary logs?
>
> Sean Busbey wrote:
> When migrations are moving slowly for some reason, it would produce a message every ~5 seconds. the monitor would roll them up into a single message, but the log4j would have no such dampening. I could put in a time threshold, if you'd prefer to keep that down.
>
> Sean Busbey wrote:
> in the case of ACCUMULO-2694, that's a max of 60 messages before self correction. That seems excessive. once a minute sound better?
Take a look at the change and let me know if this sounds sufficiently fixed? It'll only message if balance calls fail for one contiguous minute, and only repeat the message once per minute.
- Sean
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/20525/#review40907
-----------------------------------------------------------
On April 21, 2014, 9:32 p.m., Sean Busbey wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/20525/
> -----------------------------------------------------------
>
> (Updated April 21, 2014, 9:32 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
> -----
>
> src/server/pom.xml dbe4fb4
> src/server/src/main/java/org/apache/accumulo/server/master/Master.java fb7be51
> src/server/src/main/java/org/apache/accumulo/server/master/balancer/ChaoticLoadBalancer.java 02a4e89
> src/server/src/main/java/org/apache/accumulo/server/master/balancer/DefaultLoadBalancer.java 4826097
> src/server/src/main/java/org/apache/accumulo/server/master/balancer/TabletBalancer.java ad62360
> test/system/auto/stress/migrations.py d07d7a8
>
> 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 April 21, 2014, 6:20 p.m., Mike Drob wrote:
> > src/server/src/main/java/org/apache/accumulo/server/master/balancer/DefaultLoadBalancer.java, line 317
> > <https://reviews.apache.org/r/20525/diff/1/?file=563229#file563229line317>
> >
> > Can this cause a lot of unnecessary logs?
When migrations are moving slowly for some reason, it would produce a message every ~5 seconds. the monitor would roll them up into a single message, but the log4j would have no such dampening. I could put in a time threshold, if you'd prefer to keep that down.
- Sean
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/20525/#review40907
-----------------------------------------------------------
On April 21, 2014, 6:15 p.m., Sean Busbey wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/20525/
> -----------------------------------------------------------
>
> (Updated April 21, 2014, 6:15 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
> -----
>
> src/server/pom.xml dbe4fb4
> src/server/src/main/java/org/apache/accumulo/server/master/Master.java fb7be51
> src/server/src/main/java/org/apache/accumulo/server/master/balancer/ChaoticLoadBalancer.java 02a4e89
> src/server/src/main/java/org/apache/accumulo/server/master/balancer/DefaultLoadBalancer.java 4826097
> src/server/src/main/java/org/apache/accumulo/server/master/balancer/TabletBalancer.java ad62360
> test/system/auto/stress/migrations.py d07d7a8
>
> 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 Mike Drob <md...@mdrob.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/20525/#review40907
-----------------------------------------------------------
src/server/src/main/java/org/apache/accumulo/server/master/balancer/DefaultLoadBalancer.java
<https://reviews.apache.org/r/20525/#comment74178>
Can this cause a lot of unnecessary logs?
- Mike Drob
On April 21, 2014, 6:15 p.m., Sean Busbey wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/20525/
> -----------------------------------------------------------
>
> (Updated April 21, 2014, 6:15 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
> -----
>
> src/server/pom.xml dbe4fb4
> src/server/src/main/java/org/apache/accumulo/server/master/Master.java fb7be51
> src/server/src/main/java/org/apache/accumulo/server/master/balancer/ChaoticLoadBalancer.java 02a4e89
> src/server/src/main/java/org/apache/accumulo/server/master/balancer/DefaultLoadBalancer.java 4826097
> src/server/src/main/java/org/apache/accumulo/server/master/balancer/TabletBalancer.java ad62360
> test/system/auto/stress/migrations.py d07d7a8
>
> 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 April 21, 2014, 9:22 p.m., Josh Elser wrote:
> > src/server/src/main/java/org/apache/accumulo/server/master/balancer/DefaultLoadBalancer.java, line 321
> > <https://reviews.apache.org/r/20525/diff/1/?file=563229#file563229line321>
> >
> > Seems like this could get rather spammy. If we have no tservers, we *know* that things are already bad, probably don't need to be re-WARN'ed.
a balancer's notion of "there are online tservers" might differ from other points in the system, so I think it's still worth calling out if a balancer thinks there are none.
Take a look at the change and let me know if this sounds sufficiently fixed? It'll only message if balance calls fail for one contiguous minute, and only repeat the message once per minute.
- Sean
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/20525/#review40950
-----------------------------------------------------------
On April 21, 2014, 9:32 p.m., Sean Busbey wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/20525/
> -----------------------------------------------------------
>
> (Updated April 21, 2014, 9:32 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
> -----
>
> src/server/pom.xml dbe4fb4
> src/server/src/main/java/org/apache/accumulo/server/master/Master.java fb7be51
> src/server/src/main/java/org/apache/accumulo/server/master/balancer/ChaoticLoadBalancer.java 02a4e89
> src/server/src/main/java/org/apache/accumulo/server/master/balancer/DefaultLoadBalancer.java 4826097
> src/server/src/main/java/org/apache/accumulo/server/master/balancer/TabletBalancer.java ad62360
> test/system/auto/stress/migrations.py d07d7a8
>
> 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>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/20525/#review40950
-----------------------------------------------------------
src/server/src/main/java/org/apache/accumulo/server/master/balancer/DefaultLoadBalancer.java
<https://reviews.apache.org/r/20525/#comment74269>
Seems like this could get rather spammy. If we have no tservers, we *know* that things are already bad, probably don't need to be re-WARN'ed.
- Josh Elser
On April 21, 2014, 6:15 p.m., Sean Busbey wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/20525/
> -----------------------------------------------------------
>
> (Updated April 21, 2014, 6:15 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
> -----
>
> src/server/pom.xml dbe4fb4
> src/server/src/main/java/org/apache/accumulo/server/master/Master.java fb7be51
> src/server/src/main/java/org/apache/accumulo/server/master/balancer/ChaoticLoadBalancer.java 02a4e89
> src/server/src/main/java/org/apache/accumulo/server/master/balancer/DefaultLoadBalancer.java 4826097
> src/server/src/main/java/org/apache/accumulo/server/master/balancer/TabletBalancer.java ad62360
> test/system/auto/stress/migrations.py d07d7a8
>
> 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
>
>