[GitHub] accumulo pull request #272: ACCUMULO-4654 Added the ability to continue bala...

classic Classic list List threaded Threaded
33 messages Options
12
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

[GitHub] accumulo pull request #272: ACCUMULO-4654 Added the ability to continue bala...

joshelser
GitHub user ivakegg opened a pull request:

    https://github.com/apache/accumulo/pull/272

    ACCUMULO-4654 Added the ability to continue balancing even if there a…

    …re pending migrations.  Also added detection for tables that have been continuously migrating for over an hour.

You can merge this pull request into a Git repository by running:

    $ git pull https://github.com/ivakegg/accumulo ACCUMULO-4654

Alternatively you can review and apply these changes as the patch at:

    https://github.com/apache/accumulo/pull/272.patch

To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:

    This closes #272
   
----
commit 224becfb010a9d30a52a604145fe83215a18279e
Author: Ivan Bella <[hidden email]>
Date:   2017-06-19T17:57:44Z

    ACCUMULO-4654 Added the ability to continue balancing even if there are pending migrations.  Also added detection for tables that have been continuously migrating for over an hour.

----


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at [hidden email] or file a JIRA ticket
with INFRA.
---
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

[GitHub] accumulo pull request #272: ACCUMULO-4654 Added the ability to continue bala...

joshelser
Github user keith-turner commented on a diff in the pull request:

    https://github.com/apache/accumulo/pull/272#discussion_r122819594
 
    --- Diff: server/base/src/main/java/org/apache/accumulo/server/master/balancer/HostRegexTableLoadBalancer.java ---
    @@ -81,15 +86,24 @@
           + "balancer.host.regex.concurrent.migrations";
       private static final int HOST_BALANCER_REGEX_MAX_MIGRATIONS_DEFAULT = 250;
       protected static final String DEFAULT_POOL = "HostTableLoadBalancer.ALL";
    +  private static final int DEFAULT_OUTSTANDING_MIGRATIONS = 500;
    +  public static final String HOST_BALANCER_OUTSTANDING_MIGRATIONS_KEY = Property.TABLE_ARBITRARY_PROP_PREFIX.getKey()
    +      + "balancer.host.regex.max.outstanding.migrations";
     
       protected long oobCheckMillis = AccumuloConfiguration.getTimeInMillis(HOST_BALANCER_OOB_DEFAULT);
     
    +  private static final long ONE_HOUR = 60 * 60 * 1000;
    +  private static final Set<KeyExtent> EMPTY_MIGRATIONS = Collections.EMPTY_SET;
    +
       private Map<String,String> tableIdToTableName = null;
       private Map<String,Pattern> poolNameToRegexPattern = null;
       private volatile long lastOOBCheck = System.currentTimeMillis();
       private boolean isIpBasedRegex = false;
       private Map<String,SortedMap<TServerInstance,TabletServerStatus>> pools = new HashMap<>();
       private int maxTServerMigrations = HOST_BALANCER_REGEX_MAX_MIGRATIONS_DEFAULT;
    +  private int maxOutstandingMigrations = DEFAULT_OUTSTANDING_MIGRATIONS;
    +  private Map<KeyExtent,TabletMigration> migrationsFromLastPass = new HashMap<KeyExtent,TabletMigration>();
    --- End diff --
   
    For the case where many tables use this balancer, will this increase memory usage significantly?    For example if 100 tables use this balancer and all keeping track of their last migrations.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at [hidden email] or file a JIRA ticket
with INFRA.
---
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

[GitHub] accumulo pull request #272: ACCUMULO-4654 Added the ability to continue bala...

joshelser
In reply to this post by joshelser
Github user keith-turner commented on a diff in the pull request:

    https://github.com/apache/accumulo/pull/272#discussion_r122982930
 
    --- Diff: server/base/src/main/java/org/apache/accumulo/server/master/balancer/HostRegexTableLoadBalancer.java ---
    @@ -81,15 +86,24 @@
           + "balancer.host.regex.concurrent.migrations";
       private static final int HOST_BALANCER_REGEX_MAX_MIGRATIONS_DEFAULT = 250;
       protected static final String DEFAULT_POOL = "HostTableLoadBalancer.ALL";
    +  private static final int DEFAULT_OUTSTANDING_MIGRATIONS = 500;
    +  public static final String HOST_BALANCER_OUTSTANDING_MIGRATIONS_KEY = Property.TABLE_ARBITRARY_PROP_PREFIX.getKey()
    +      + "balancer.host.regex.max.outstanding.migrations";
     
       protected long oobCheckMillis = AccumuloConfiguration.getTimeInMillis(HOST_BALANCER_OOB_DEFAULT);
     
    +  private static final long ONE_HOUR = 60 * 60 * 1000;
    +  private static final Set<KeyExtent> EMPTY_MIGRATIONS = Collections.EMPTY_SET;
    +
       private Map<String,String> tableIdToTableName = null;
       private Map<String,Pattern> poolNameToRegexPattern = null;
       private volatile long lastOOBCheck = System.currentTimeMillis();
       private boolean isIpBasedRegex = false;
       private Map<String,SortedMap<TServerInstance,TabletServerStatus>> pools = new HashMap<>();
       private int maxTServerMigrations = HOST_BALANCER_REGEX_MAX_MIGRATIONS_DEFAULT;
    +  private int maxOutstandingMigrations = DEFAULT_OUTSTANDING_MIGRATIONS;
    +  private Map<KeyExtent,TabletMigration> migrationsFromLastPass = new HashMap<KeyExtent,TabletMigration>();
    --- End diff --
   
    I spoke w/ @ivakegg and found this is not a per table balancer, so not an issue.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at [hidden email] or file a JIRA ticket
with INFRA.
---
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

[GitHub] accumulo pull request #272: ACCUMULO-4654 Added the ability to continue bala...

joshelser
In reply to this post by joshelser
Github user dlmarion commented on a diff in the pull request:

    https://github.com/apache/accumulo/pull/272#discussion_r123021060
 
    --- Diff: server/base/src/main/java/org/apache/accumulo/server/master/balancer/HostRegexTableLoadBalancer.java ---
    @@ -81,15 +86,24 @@
           + "balancer.host.regex.concurrent.migrations";
       private static final int HOST_BALANCER_REGEX_MAX_MIGRATIONS_DEFAULT = 250;
       protected static final String DEFAULT_POOL = "HostTableLoadBalancer.ALL";
    +  private static final int DEFAULT_OUTSTANDING_MIGRATIONS = 500;
    +  public static final String HOST_BALANCER_OUTSTANDING_MIGRATIONS_KEY = Property.TABLE_ARBITRARY_PROP_PREFIX.getKey()
    --- End diff --
   
    Can you add some text in the class javadoc for this property?


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at [hidden email] or file a JIRA ticket
with INFRA.
---
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

[GitHub] accumulo pull request #272: ACCUMULO-4654 Added the ability to continue bala...

joshelser
In reply to this post by joshelser
Github user dlmarion commented on a diff in the pull request:

    https://github.com/apache/accumulo/pull/272#discussion_r123022077
 
    --- Diff: server/base/src/main/java/org/apache/accumulo/server/master/balancer/HostRegexTableLoadBalancer.java ---
    @@ -81,15 +86,24 @@
           + "balancer.host.regex.concurrent.migrations";
       private static final int HOST_BALANCER_REGEX_MAX_MIGRATIONS_DEFAULT = 250;
       protected static final String DEFAULT_POOL = "HostTableLoadBalancer.ALL";
    +  private static final int DEFAULT_OUTSTANDING_MIGRATIONS = 500;
    +  public static final String HOST_BALANCER_OUTSTANDING_MIGRATIONS_KEY = Property.TABLE_ARBITRARY_PROP_PREFIX.getKey()
    +      + "balancer.host.regex.max.outstanding.migrations";
     
       protected long oobCheckMillis = AccumuloConfiguration.getTimeInMillis(HOST_BALANCER_OOB_DEFAULT);
     
    +  private static final long ONE_HOUR = 60 * 60 * 1000;
    +  private static final Set<KeyExtent> EMPTY_MIGRATIONS = Collections.EMPTY_SET;
    +
       private Map<String,String> tableIdToTableName = null;
       private Map<String,Pattern> poolNameToRegexPattern = null;
       private volatile long lastOOBCheck = System.currentTimeMillis();
       private boolean isIpBasedRegex = false;
       private Map<String,SortedMap<TServerInstance,TabletServerStatus>> pools = new HashMap<>();
       private int maxTServerMigrations = HOST_BALANCER_REGEX_MAX_MIGRATIONS_DEFAULT;
    +  private int maxOutstandingMigrations = DEFAULT_OUTSTANDING_MIGRATIONS;
    +  private Map<KeyExtent,TabletMigration> migrationsFromLastPass = new HashMap<KeyExtent,TabletMigration>();
    --- End diff --
   
    can these maps be made final?


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at [hidden email] or file a JIRA ticket
with INFRA.
---
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

[GitHub] accumulo pull request #272: ACCUMULO-4654 Added the ability to continue bala...

joshelser
In reply to this post by joshelser
Github user joshelser commented on a diff in the pull request:

    https://github.com/apache/accumulo/pull/272#discussion_r123026632
 
    --- Diff: server/base/src/main/java/org/apache/accumulo/server/master/balancer/HostRegexTableLoadBalancer.java ---
    @@ -301,8 +327,10 @@ public long balance(SortedMap<TServerInstance,TabletServerStatus> current, Set<K
           return minBalanceTime;
     
         Map<String,String> tableIdMap = t.tableIdMap();
    +    long now = System.currentTimeMillis();
    --- End diff --
   
    It doesn't look like this is being used more than once? Why pull it out to a variable in that case?


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at [hidden email] or file a JIRA ticket
with INFRA.
---
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

[GitHub] accumulo pull request #272: ACCUMULO-4654 Added the ability to continue bala...

joshelser
In reply to this post by joshelser
Github user joshelser commented on a diff in the pull request:

    https://github.com/apache/accumulo/pull/272#discussion_r123026846
 
    --- Diff: server/base/src/main/java/org/apache/accumulo/server/master/balancer/HostRegexTableLoadBalancer.java ---
    @@ -367,8 +396,30 @@ public long balance(SortedMap<TServerInstance,TabletServerStatus> current, Set<K
         }
     
         if (migrations != null && migrations.size() > 0) {
    -      LOG.warn("Not balancing tables due to {} outstanding migrations", migrations.size());
    -      return minBalanceTime;
    +      if (migrations.size() >= maxOutstandingMigrations) {
    +        LOG.warn("Not balancing tables due to {} outstanding migrations", migrations.size());
    +        LOG.trace("Sample up to 10 outstanding migrations: {}", Iterables.limit(migrations, 10));
    --- End diff --
   
    I'd probably wrap this line in a `ifTraceEnabled()` since string-ifying the migrations could get a little costly.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at [hidden email] or file a JIRA ticket
with INFRA.
---
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

[GitHub] accumulo pull request #272: ACCUMULO-4654 Added the ability to continue bala...

joshelser
In reply to this post by joshelser
Github user joshelser commented on a diff in the pull request:

    https://github.com/apache/accumulo/pull/272#discussion_r123027005
 
    --- Diff: server/base/src/main/java/org/apache/accumulo/server/master/balancer/HostRegexTableLoadBalancer.java ---
    @@ -367,8 +396,30 @@ public long balance(SortedMap<TServerInstance,TabletServerStatus> current, Set<K
         }
     
         if (migrations != null && migrations.size() > 0) {
    -      LOG.warn("Not balancing tables due to {} outstanding migrations", migrations.size());
    -      return minBalanceTime;
    +      if (migrations.size() >= maxOutstandingMigrations) {
    +        LOG.warn("Not balancing tables due to {} outstanding migrations", migrations.size());
    +        LOG.trace("Sample up to 10 outstanding migrations: {}", Iterables.limit(migrations, 10));
    +        return minBalanceTime;
    +      }
    +
    +      LOG.debug("Current outstanding migrations of " + migrations.size() + " being applied");
    +      LOG.trace("Sample up to 10 outstanding migrations: {}", Iterables.limit(migrations, 10));
    --- End diff --
   
    Same here.
   
    Also, forgot to use the `{}` syntax on the debug above.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at [hidden email] or file a JIRA ticket
with INFRA.
---
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

[GitHub] accumulo pull request #272: ACCUMULO-4654 Added the ability to continue bala...

joshelser
In reply to this post by joshelser
Github user joshelser commented on a diff in the pull request:

    https://github.com/apache/accumulo/pull/272#discussion_r123027160
 
    --- Diff: server/base/src/main/java/org/apache/accumulo/server/master/balancer/HostRegexTableLoadBalancer.java ---
    @@ -301,8 +327,10 @@ public long balance(SortedMap<TServerInstance,TabletServerStatus> current, Set<K
           return minBalanceTime;
     
         Map<String,String> tableIdMap = t.tableIdMap();
    +    long now = System.currentTimeMillis();
    --- End diff --
   
    Oops, sorry, my bad. GH folding was misleading me.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at [hidden email] or file a JIRA ticket
with INFRA.
---
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

[GitHub] accumulo pull request #272: ACCUMULO-4654 Added the ability to continue bala...

joshelser
In reply to this post by joshelser
Github user joshelser commented on a diff in the pull request:

    https://github.com/apache/accumulo/pull/272#discussion_r123027579
 
    --- Diff: server/base/src/test/java/org/apache/accumulo/server/master/balancer/BaseHostRegexTableLoadBalancerTest.java ---
    @@ -138,9 +142,12 @@ public String getId() {
     
       protected static final HashMap<String,String> DEFAULT_TABLE_PROPERTIES = new HashMap<>();
       {
    -    DEFAULT_TABLE_PROPERTIES.put(HostRegexTableLoadBalancer.HOST_BALANCER_OOB_CHECK_KEY, "2s");
    +    DEFAULT_TABLE_PROPERTIES.put(HostRegexTableLoadBalancer.HOST_BALANCER_OOB_CHECK_KEY, "7s");
    --- End diff --
   
    How important of a change is this? Something that needs to be called out in release notes?


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at [hidden email] or file a JIRA ticket
with INFRA.
---
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

[GitHub] accumulo issue #272: ACCUMULO-4654 Added the ability to continue balancing e...

joshelser
In reply to this post by joshelser
Github user joshelser commented on the issue:

    https://github.com/apache/accumulo/pull/272
 
    The description of this change makes me wonder if this is intended as a band-aid. It reads as those the root issue is that the Master orphans migrations (which causes problems in the balancer). Are we solving the right problem?


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at [hidden email] or file a JIRA ticket
with INFRA.
---
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

[GitHub] accumulo pull request #272: ACCUMULO-4654 Added the ability to continue bala...

joshelser
In reply to this post by joshelser
Github user ivakegg commented on a diff in the pull request:

    https://github.com/apache/accumulo/pull/272#discussion_r123028922
 
    --- Diff: server/base/src/main/java/org/apache/accumulo/server/master/balancer/HostRegexTableLoadBalancer.java ---
    @@ -367,8 +396,30 @@ public long balance(SortedMap<TServerInstance,TabletServerStatus> current, Set<K
         }
     
         if (migrations != null && migrations.size() > 0) {
    -      LOG.warn("Not balancing tables due to {} outstanding migrations", migrations.size());
    -      return minBalanceTime;
    +      if (migrations.size() >= maxOutstandingMigrations) {
    +        LOG.warn("Not balancing tables due to {} outstanding migrations", migrations.size());
    +        LOG.trace("Sample up to 10 outstanding migrations: {}", Iterables.limit(migrations, 10));
    --- End diff --
   
    In this form of calling LOG.trace, I thought the object passed in does not actually get string-ifyed if trace is not enabled.....


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at [hidden email] or file a JIRA ticket
with INFRA.
---
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

[GitHub] accumulo pull request #272: ACCUMULO-4654 Added the ability to continue bala...

joshelser
In reply to this post by joshelser
Github user ivakegg commented on a diff in the pull request:

    https://github.com/apache/accumulo/pull/272#discussion_r123029015
 
    --- Diff: server/base/src/main/java/org/apache/accumulo/server/master/balancer/HostRegexTableLoadBalancer.java ---
    @@ -367,8 +396,30 @@ public long balance(SortedMap<TServerInstance,TabletServerStatus> current, Set<K
         }
     
         if (migrations != null && migrations.size() > 0) {
    -      LOG.warn("Not balancing tables due to {} outstanding migrations", migrations.size());
    -      return minBalanceTime;
    +      if (migrations.size() >= maxOutstandingMigrations) {
    +        LOG.warn("Not balancing tables due to {} outstanding migrations", migrations.size());
    +        LOG.trace("Sample up to 10 outstanding migrations: {}", Iterables.limit(migrations, 10));
    +        return minBalanceTime;
    +      }
    +
    +      LOG.debug("Current outstanding migrations of " + migrations.size() + " being applied");
    +      LOG.trace("Sample up to 10 outstanding migrations: {}", Iterables.limit(migrations, 10));
    --- End diff --
   
    that is fair



---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at [hidden email] or file a JIRA ticket
with INFRA.
---
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

[GitHub] accumulo pull request #272: ACCUMULO-4654 Added the ability to continue bala...

joshelser
In reply to this post by joshelser
Github user ivakegg commented on a diff in the pull request:

    https://github.com/apache/accumulo/pull/272#discussion_r123029366
 
    --- Diff: server/base/src/test/java/org/apache/accumulo/server/master/balancer/BaseHostRegexTableLoadBalancerTest.java ---
    @@ -138,9 +142,12 @@ public String getId() {
     
       protected static final HashMap<String,String> DEFAULT_TABLE_PROPERTIES = new HashMap<>();
       {
    -    DEFAULT_TABLE_PROPERTIES.put(HostRegexTableLoadBalancer.HOST_BALANCER_OOB_CHECK_KEY, "2s");
    +    DEFAULT_TABLE_PROPERTIES.put(HostRegexTableLoadBalancer.HOST_BALANCER_OOB_CHECK_KEY, "7s");
    --- End diff --
   
    in the test, that change is important because some of the new tests take a little more time to do the balancing and I need that to happen before the out-of-bounds check.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at [hidden email] or file a JIRA ticket
with INFRA.
---
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

[GitHub] accumulo pull request #272: ACCUMULO-4654 Added the ability to continue bala...

joshelser
In reply to this post by joshelser
Github user joshelser commented on a diff in the pull request:

    https://github.com/apache/accumulo/pull/272#discussion_r123029523
 
    --- Diff: server/base/src/main/java/org/apache/accumulo/server/master/balancer/HostRegexTableLoadBalancer.java ---
    @@ -367,8 +396,30 @@ public long balance(SortedMap<TServerInstance,TabletServerStatus> current, Set<K
         }
     
         if (migrations != null && migrations.size() > 0) {
    -      LOG.warn("Not balancing tables due to {} outstanding migrations", migrations.size());
    -      return minBalanceTime;
    +      if (migrations.size() >= maxOutstandingMigrations) {
    +        LOG.warn("Not balancing tables due to {} outstanding migrations", migrations.size());
    +        LOG.trace("Sample up to 10 outstanding migrations: {}", Iterables.limit(migrations, 10));
    --- End diff --
   
    Nope :) Think about calling a method in java. You still have to cast/convert the object into the correct parameters (e.g. the Iterable<String> into a String) regardless of whether or not it gets logged.
   
    The thing that SLF4j loggers get you is that you don't build the log message if it's not going to be logged.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at [hidden email] or file a JIRA ticket
with INFRA.
---
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

[GitHub] accumulo pull request #272: ACCUMULO-4654 Added the ability to continue bala...

joshelser
In reply to this post by joshelser
Github user joshelser commented on a diff in the pull request:

    https://github.com/apache/accumulo/pull/272#discussion_r123029714
 
    --- Diff: server/base/src/test/java/org/apache/accumulo/server/master/balancer/BaseHostRegexTableLoadBalancerTest.java ---
    @@ -138,9 +142,12 @@ public String getId() {
     
       protected static final HashMap<String,String> DEFAULT_TABLE_PROPERTIES = new HashMap<>();
       {
    -    DEFAULT_TABLE_PROPERTIES.put(HostRegexTableLoadBalancer.HOST_BALANCER_OOB_CHECK_KEY, "2s");
    +    DEFAULT_TABLE_PROPERTIES.put(HostRegexTableLoadBalancer.HOST_BALANCER_OOB_CHECK_KEY, "7s");
    --- End diff --
   
    Sorry, I didn't realize that this was in a test. I thought this was changing things for users.
   
    /me goes back to coffee cup


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at [hidden email] or file a JIRA ticket
with INFRA.
---
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

[GitHub] accumulo issue #272: ACCUMULO-4654 Added the ability to continue balancing e...

joshelser
In reply to this post by joshelser
Github user ivakegg commented on the issue:

    https://github.com/apache/accumulo/pull/272
 
    I do not believe we have orphaned migrations (that would be a bug).  Our problem is migrations that hang or should I say take a very long time (perhaps hours) because the tablet being migrated is in the process of being compacted.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at [hidden email] or file a JIRA ticket
with INFRA.
---
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

[GitHub] accumulo issue #272: ACCUMULO-4654 Added the ability to continue balancing e...

joshelser
In reply to this post by joshelser
Github user joshelser commented on the issue:

    https://github.com/apache/accumulo/pull/272
 
    > Our problem is migrations that hang or should I say take a very long time (perhaps hours) because the tablet being migrated is in the process of being compacted.
   
    Ahh, ok. Thanks for clarifying. I was assuming that migrations sitting around for order-hours was a bug :)


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at [hidden email] or file a JIRA ticket
with INFRA.
---
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

[GitHub] accumulo pull request #272: ACCUMULO-4654 Added the ability to continue bala...

joshelser
In reply to this post by joshelser
Github user ivakegg commented on a diff in the pull request:

    https://github.com/apache/accumulo/pull/272#discussion_r123045767
 
    --- Diff: server/base/src/main/java/org/apache/accumulo/server/master/balancer/HostRegexTableLoadBalancer.java ---
    @@ -81,15 +86,24 @@
           + "balancer.host.regex.concurrent.migrations";
       private static final int HOST_BALANCER_REGEX_MAX_MIGRATIONS_DEFAULT = 250;
       protected static final String DEFAULT_POOL = "HostTableLoadBalancer.ALL";
    +  private static final int DEFAULT_OUTSTANDING_MIGRATIONS = 500;
    +  public static final String HOST_BALANCER_OUTSTANDING_MIGRATIONS_KEY = Property.TABLE_ARBITRARY_PROP_PREFIX.getKey()
    --- End diff --
   
    yes


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at [hidden email] or file a JIRA ticket
with INFRA.
---
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

[GitHub] accumulo pull request #272: ACCUMULO-4654 Added the ability to continue bala...

joshelser
In reply to this post by joshelser
Github user ivakegg commented on a diff in the pull request:

    https://github.com/apache/accumulo/pull/272#discussion_r123045826
 
    --- Diff: server/base/src/main/java/org/apache/accumulo/server/master/balancer/HostRegexTableLoadBalancer.java ---
    @@ -81,15 +86,24 @@
           + "balancer.host.regex.concurrent.migrations";
       private static final int HOST_BALANCER_REGEX_MAX_MIGRATIONS_DEFAULT = 250;
       protected static final String DEFAULT_POOL = "HostTableLoadBalancer.ALL";
    +  private static final int DEFAULT_OUTSTANDING_MIGRATIONS = 500;
    +  public static final String HOST_BALANCER_OUTSTANDING_MIGRATIONS_KEY = Property.TABLE_ARBITRARY_PROP_PREFIX.getKey()
    +      + "balancer.host.regex.max.outstanding.migrations";
     
       protected long oobCheckMillis = AccumuloConfiguration.getTimeInMillis(HOST_BALANCER_OOB_DEFAULT);
     
    +  private static final long ONE_HOUR = 60 * 60 * 1000;
    +  private static final Set<KeyExtent> EMPTY_MIGRATIONS = Collections.EMPTY_SET;
    +
       private Map<String,String> tableIdToTableName = null;
       private Map<String,Pattern> poolNameToRegexPattern = null;
       private volatile long lastOOBCheck = System.currentTimeMillis();
       private boolean isIpBasedRegex = false;
       private Map<String,SortedMap<TServerInstance,TabletServerStatus>> pools = new HashMap<>();
       private int maxTServerMigrations = HOST_BALANCER_REGEX_MAX_MIGRATIONS_DEFAULT;
    +  private int maxOutstandingMigrations = DEFAULT_OUTSTANDING_MIGRATIONS;
    +  private Map<KeyExtent,TabletMigration> migrationsFromLastPass = new HashMap<KeyExtent,TabletMigration>();
    --- End diff --
   
    yes


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at [hidden email] or file a JIRA ticket
with INFRA.
---
12
Loading...