[
https://issues.apache.org/jira/browse/HBASE-5045?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13259981#comment-13259981
]
Phabricator commented on HBASE-5045:
------------------------------------
stack has commented on the revision "[jira] [HBASE-5045] Annotation for Custom
Param formatting and next() RPC call info".
This addition, while I'm sure its sweet, is tough to follow. The model needs
tweaking IMO.
INLINE COMMENTS
src/main/java/org/apache/hadoop/hbase/ipc/HBaseRPC.java:51 Is there an actual
change here? If there is, the extra import doesn't do anything it seems.
src/main/java/org/apache/hadoop/hbase/ipc/HBaseServer.java:197 What does this
method do? Return a map keyed by what? What is the String? Where would I
plug this in or how would I use it? Does an inspecific method like this need
to be public?
src/main/java/org/apache/hadoop/hbase/ipc/WritableRpcEngine.java:319 Is there
reason for flipping order in which these methods are called or is it just
vagaries of patch (0.89fb vs trunk)?
src/main/java/org/apache/hadoop/hbase/monitoring/MonitoredRPCHandler.java:45
Whats a 'realMethod'?
src/main/java/org/apache/hadoop/hbase/regionserver/HRegion.java:86 gratuitous
change
src/main/java/org/apache/hadoop/hbase/regionserver/HRegion.java:3489 Does
this have to public?
Why is this 'originalScan' rather than just 'scan'?
src/main/java/org/apache/hadoop/hbase/regionserver/HRegionServer.java:2210
Does this inner class have to be in HRS? Its a massive class already.
src/main/java/org/apache/hadoop/hbase/regionserver/HRegionServer.java:2221 Do
we have to reach into an HRegion like this? Can we not use an accessor? This
can be brittle going forward.
src/main/java/org/apache/hadoop/hbase/util/ParamFormat.java:2 No need of this
and its wrong year anyways.
src/main/java/org/apache/hadoop/hbase/util/ParamFormat.java:27 Should we
start an annotations package? There are other annotations floating about hbase.
src/main/java/org/apache/hadoop/hbase/util/ParamFormat.java:41 This comment
is about the future? What is this method doing now?
src/main/java/org/apache/hadoop/hbase/util/ParamFormatHelper.java:2 ditto w/
above
src/main/java/org/apache/hadoop/hbase/util/ParamFormatHelper.java:63 A method
named getMap seems way too broad a target to find using introspection. I'd
think we'd make the target narrow so for sure we returned only when we had the
right method.
src/main/java/org/apache/hadoop/hbase/util/ParamFormatHelper.java:79 Ok to
ignore?
src/main/java/org/apache/hadoop/hbase/util/ParamFormatHelper.java:102 This
method is a pretty print of a method invocation? Should we call it that
instead instead of a getMap? getMap is generic. If it were called
prettyPrint, my guess is it'd be clear to most whats going on here.
src/main/java/org/apache/hadoop/hbase/util/ParamFormatter.java:2 ditto
src/main/java/org/apache/hadoop/hbase/util/ParamFormatter.java:24 Should it
be called ParamPrettyPrinter or MethodPrettyPrinter or
MethodInvocationPrettyPrinter instead?
src/main/java/org/apache/hadoop/hbase/util/ParamFormatter.java:32 See
comments above
REVISION DETAIL
https://reviews.facebook.net/D2913
BRANCH
add_the_table_name_and_cf_name_for_the_next_HBASE-5045_v3
> Add the table name and cf name for the next call int the task monitor
> ---------------------------------------------------------------------
>
> Key: HBASE-5045
> URL: https://issues.apache.org/jira/browse/HBASE-5045
> Project: HBase
> Issue Type: Improvement
> Reporter: Liyin Tang
> Assignee: Amir Shimoni
> Attachments: D2913.1.patch, D2913.2.patch
>
>
> In the task monitor, we don't have much information about the next call
> compared to other operations.
> It would be nice to add the table name and cf name for each next call in the
> task monitor.
--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators:
https://issues.apache.org/jira/secure/ContactAdministrators!default.jspa
For more information on JIRA, see: http://www.atlassian.com/software/jira
