hbase-issues mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Umesh Agashe (JIRA)" <j...@apache.org>
Subject [jira] [Commented] (HBASE-17786) Create LoadBalancer perf-tests (test balancer algorithm decoupled from workload)
Date Thu, 04 May 2017 23:21:04 GMT

    [ https://issues.apache.org/jira/browse/HBASE-17786?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=15997579#comment-15997579

Umesh Agashe commented on HBASE-17786:

bq. nit: the file header should use "/*" instead of "/**" because it's not meant to be a javadoc
parsed comment. I think we make this mistake in lots of places already, so not a big deal
if you leave it.

Makes sense. Fixed. 2428 out of 3528 Java files use /**. In future, git add hook would be
useful to check this.

bq. We've been inconsistent on wether non-Test classes in the test packages need InterfaceAudience
annotations. Since this is a tool, best to be on the safe side and label it IA.LimitedPrivate(TOOL).


bq. So this is general cleanup while you're in the area?


bq. Comments like this would be good to have as output while running as DEBUG or INFO messages
(perhaps on stderr if we can't use a logger for some reason).

Fixed. Added a INFO log message for it.

bq. Why this variable rather than just using HConstants.DEFAULT_REGIONSERVER_PORT?


bq. Class description should include an example of how we expect folks to invoke this.


bq. What's the rationale for using System.out directly instead of a logger?

Thinking about this more, and looking at e.g. the existing PerformanceEvaluation tool, I think
we should update this to use commons-logger as most of the rest of our stuff does.

I referred to tests HFilePerformanceEvaluation and ProcedureWALPerformanceEvaluation which
have System.out.print statements. Most PerformanceEvaluation tools (9/12) have System.out.print
statements. Looking at the output I think the rational is: logs get printed to stderr while
explicit print statements print to stdout. stderr can be redirected to get user presentable
output with the tool.

We should only need the stochastic load balancer settings if the load balancer is the stochastic
load balancer, I think?
Also, we should not set these values if the incoming configuration had them set by the user
running the perf tool.

Removed. User can set these in conf, defaults will be used otherwise.

> Create LoadBalancer perf-tests (test balancer algorithm decoupled from workload)
> --------------------------------------------------------------------------------
>                 Key: HBASE-17786
>                 URL: https://issues.apache.org/jira/browse/HBASE-17786
>             Project: HBase
>          Issue Type: Sub-task
>          Components: Balancer, proc-v2
>            Reporter: stack
>            Assignee: Umesh Agashe
>              Labels: beginner
>             Fix For: 2.0.0
>         Attachments: HBASE-17786.001.patch
> (Below is a quote from [~mbertozzi] taken from an internal issue that I'm moving out
> Add perf tools and keep monitored balancer performance (a BalancerPE-type thing).
> Most of the balancers should be instantiable without requiring a mini-cluster, and it
easy to create tons of RegionInfo and ServerNames with a for loop.
> The balancer is just creating a map RegionInfo:ServerName.
> There are two methods to test roundRobinAssignment() and retainAssignment()
> {code}
> Map<ServerName, List<HRegionInfo>> roundRobinAssignment(
>     List<HRegionInfo> regions,
>     List<ServerName> servers
>   ) throws HBaseIOException; 
> Map<ServerName, List<HRegionInfo>> retainAssignment(
>     Map<HRegionInfo, ServerName> regions,
>     List<ServerName> servers
>   ) throws HBaseIOException;
> {code}
> There are a bunch of obvious optimization that everyone can see just by looking at the
code. (like replacing array with set when we do contains/remove operations). It will be nice
to have a baseline and start improving from there.

This message was sent by Atlassian JIRA

View raw message