[
https://issues.apache.org/jira/browse/HBASE-10147?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13858374#comment-13858374
]
takeshi.miao commented on HBASE-10147:
--------------------------------------
[~stack] & [~gustavoanatoly]
This patch also looks good to me, but still has some questions need to verify,
are...
1. The public API is broken, since the _Sink interface_ is changed
{code}
@@ -66,7 +66,8 @@
public interface Sink {
public void publishReadFailure(HRegionInfo region, Exception e);
public void publishReadFailure(HRegionInfo region, HColumnDescriptor
column, Exception e);
- public void publishReadTiming(HRegionInfo region, HColumnDescriptor
column, long msTime);
+ public void publishInfo(HRegionInfo region, ServerName serverName);
+ public void publishInfoTimeout(HRegionInfo region, ServerName serverName);
}
{code}
I not sure whether it is acceptable code change, that is ok if stack says ok,
due to I think currently that not many users really extend this _interface_ ...
?
2. I think that the _errorCode_ property would be better to be _volatile_. due
to it would be manipulated by the _monitor_ thread, and read by _main_ thread
in the same time.
{code}
#140 private static int errorCode;
// changed to
#140 private static volatile int errorCode;
{code}
And other changed static properties will be ok due to their value manipulation
are not intervened between two threads in the same time.
{code}
#136 private static long timeout = DEFAULT_TIMEOUT;
#137 private static boolean failOnError = true;
//...
#139 private static long startTimeCanary;
{code}
3. Just a little reminder that the _this_ reference shall be removed due to
_failOnError_ is changed to _static_
{code}
#224 this.failOnError = Boolean.parseBoolean(args[i]);
//...
#249 if (this.failOnError && monitor.hasError()) {
//...
#255 if (this.failOnError && monitor.hasError()) {
{code}
FYR~
> Canary additions
> ----------------
>
> Key: HBASE-10147
> URL: https://issues.apache.org/jira/browse/HBASE-10147
> Project: HBase
> Issue Type: Improvement
> Reporter: stack
> Assignee: Gustavo Anatoly
> Attachments: HBASE-10147.patch, HBASE-10147.patch, HBASE-10147.patch,
> HBASE-10147.patch
>
>
> I've been using the canary to quickly identify the dodgy machine in my
> cluster. It is useful for this. What would make it better would be:
> + Rather than saying how long it took to get a region after you have gotten
> the region, it'd be sweet to log BEFORE you went to get the region the
> regionname and the server it is on. I ask for this because as is, I have to
> wait for the canary to timeout which can be a while.
> + Second ask is that when I pass the -t, that when it fails, it says what it
> failed against -- what region and hopefully what server location (might be
> hard).
--
This message was sent by Atlassian JIRA
(v6.1.5#6160)
