hbase-issues mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "takeshi.miao (JIRA)" <j...@apache.org>
Subject [jira] [Commented] (HBASE-10147) Canary additions
Date Sun, 29 Dec 2013 17:03:50 GMT

    [ 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)

Mime
View raw message