hbase-issues mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Sean Busbey (JIRA)" <j...@apache.org>
Subject [jira] [Commented] (HBASE-12785) Use FutureTask to timeout the attempt to get the lock for hbck
Date Wed, 31 Dec 2014 22:38:13 GMT

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

Sean Busbey commented on HBASE-12785:
-------------------------------------

Good start!

-----
{code}
+    executor.execute(futureTask);
+    long duration = 0;
+    while (!futureTask.isDone() && duration <= 30000) {
+      Threads.sleep(10);
+      duration = EnvironmentEdgeManager.currentTime() - start;
+    }
+    if (!futureTask.isDone()) {
+      // took too long to obtain lock
+      LOG.warn("Took " + duration + " milliseconds to obtain lock");
+      futureTask.cancel(true);
+      return null;
+    }
+    executor.shutdownNow();
+    FSDataOutputStream stream = null;
     try {
<snip>
+      stream = futureTask.get();
{code}

The above can be simplified to using [FutureTask.get with timeout|http://docs.oracle.com/javase/6/docs/api/java/util/concurrent/FutureTask.html#get(long,
java.util.concurrent.TimeUnit)]. It would basically look like

{code}
+    executor.execute(futureTask);
+    FSDataOutputStream stream = null;
     try {
<snip>
+      stream = futureTask.get(30, TimeUnit.SECONDS);
+    } catch (TimeoutException exception) {
+      executor.shutdownNow();
{code}

-----
{code}
+    } catch (ExecutionException ee) {
+      LOG.warn("Encountered exception when opening lock file", ee);
{code}

This is a change in behavior, so I want to make sure it's intentional. Previously, if there
was a remote exception (that wasn't about the file already being created) or a RTE, we'd throw
that out of this method instead of returning a lock failure. With this change either of those
failure types happening in the Callable will result in us logging and returning a lock failure.

Since we exit upon lock failure, I think this is all fine?

-----
{code}
+    } catch (InterruptedException ie) {
+      LOG.warn("Encountered exception when opening lock file", ie);
{code}

The message here should specify that we got interrupted. Also since we aren't re-throwing,
we should set the thread's interrupted status. Something like:
{code}
+    } catch (InterruptedException ie) {
+      LOG.warn("Interrupted while opening lock file", ie);
+      Thread.currentThread().interrupt();
{code}

> Use FutureTask to timeout the attempt to get the lock for hbck
> --------------------------------------------------------------
>
>                 Key: HBASE-12785
>                 URL: https://issues.apache.org/jira/browse/HBASE-12785
>             Project: HBase
>          Issue Type: Task
>            Reporter: Ted Yu
>            Assignee: Ted Yu
>            Priority: Minor
>         Attachments: 12785-001.patch
>
>
> In reviewing HBASE-12607, Sean pointed out:
> It would be nice if we used a [FutureTask|http://docs.oracle.com/javase/7/docs/api/java/util/concurrent/FutureTask.html]
to timeout the attempt to get the lock rather than wait the whole period and then fail.
> This issue is to address Sean's review comment.



--
This message was sent by Atlassian JIRA
(v6.3.4#6332)

Mime
View raw message