[
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).
Fixed.
bq. So this is general cleanup while you're in the area?
Yes
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?
Fixed.
bq. Class description should include an example of how we expect folks to
invoke this.
Fixed.
{quote}
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.
{quote}
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.
{quote}
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.
{quote}
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 here)
> 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
(v6.3.15#6346)
