Return-Path: 
 <issues-return-176931-apmail-hbase-issues-archive=hbase.apache.org@hbase.apache.org>
X-Original-To: apmail-hbase-issues-archive@www.apache.org
Delivered-To: apmail-hbase-issues-archive@www.apache.org
Received: from mail.apache.org (hermes.apache.org [140.211.11.3])
	by minotaur.apache.org (Postfix) with SMTP id 90D2B10C0D
	for <apmail-hbase-issues-archive@www.apache.org>;
 Wed, 31 Dec 2014 22:38:20 +0000 (UTC)
Received: (qmail 28578 invoked by uid 500); 31 Dec 2014 22:38:14 -0000
Delivered-To: apmail-hbase-issues-archive@hbase.apache.org
Received: (qmail 28530 invoked by uid 500); 31 Dec 2014 22:38:14 -0000
Mailing-List: contact issues-help@hbase.apache.org; run by ezmlm
Precedence: bulk
List-Help: <mailto:issues-help@hbase.apache.org>
List-Unsubscribe: <mailto:issues-unsubscribe@hbase.apache.org>
List-Post: <mailto:issues@hbase.apache.org>
List-Id: <issues.hbase.apache.org>
Delivered-To: mailing list issues@hbase.apache.org
Received: (qmail 28519 invoked by uid 99); 31 Dec 2014 22:38:14 -0000
Received: from arcas.apache.org (HELO arcas.apache.org) (140.211.11.28)
    by apache.org (qpsmtpd/0.29) with ESMTP; Wed, 31 Dec 2014 22:38:14 +0000
Date: Wed, 31 Dec 2014 22:38:13 +0000 (UTC)
From: "Sean Busbey (JIRA)" <jira@apache.org>
To: issues@hbase.apache.org
Message-ID: <JIRA.12764228.1419961613000.123355.1420065493957@Atlassian.JIRA>
In-Reply-To: <JIRA.12764228.1419961613000@Atlassian.JIRA>
References: <JIRA.12764228.1419961613000@Atlassian.JIRA>
 <JIRA.12764228.1419961613801@arcas>
Subject: [jira] [Commented] (HBASE-12785) Use FutureTask to timeout the
 attempt to get the lock for hbck
MIME-Version: 1.0
Content-Type: text/plain; charset=utf-8
Content-Transfer-Encoding: 7bit
X-JIRA-FingerPrint: 30527f35849b9dde25b450d4833f0394


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