Mailing-List: contact common-commits-help@hadoop.apache.org; run by ezmlm
Precedence: bulk
Reply-To: common-dev@hadoop.apache.org
Content-Type: text/plain; charset="utf-8"
MIME-Version: 1.0
Content-Transfer-Encoding: 7bit
Subject: svn commit: r802971 - in /hadoop/common/branches/branch-0.20: ./
 src/hdfs/org/apache/hadoop/hdfs/
 src/hdfs/org/apache/hadoop/hdfs/server/common/
 src/hdfs/org/apache/hadoop/hdfs/server/namenode/
 src/test/org/apache/hadoop/hdfs/server/common/
Date: Tue, 11 Aug 2009 01:08:38 -0000
To: common-commits@hadoop.apache.org
From: szetszwo@apache.org
Message-Id: <20090811010838.5216C2388878@eris.apache.org>

Author: szetszwo
Date: Tue Aug 11 01:08:37 2009
New Revision: 802971

URL: http://svn.apache.org/viewvc?rev=802971&view=rev
Log:
HDFS-525. The SimpleDateFormat object in ListPathsServlet is not thread safe.  Contributed by Suresh Srinivas

Added:
    hadoop/common/branches/branch-0.20/src/hdfs/org/apache/hadoop/hdfs/server/common/ThreadLocalDateFormat.java
    hadoop/common/branches/branch-0.20/src/test/org/apache/hadoop/hdfs/server/common/TestThreadLocalDateFormat.java
Modified:
    hadoop/common/branches/branch-0.20/CHANGES.txt
    hadoop/common/branches/branch-0.20/src/hdfs/org/apache/hadoop/hdfs/HftpFileSystem.java
    hadoop/common/branches/branch-0.20/src/hdfs/org/apache/hadoop/hdfs/server/namenode/ListPathsServlet.java

Modified: hadoop/common/branches/branch-0.20/CHANGES.txt
URL: http://svn.apache.org/viewvc/hadoop/common/branches/branch-0.20/CHANGES.txt?rev=802971&r1=802970&r2=802971&view=diff
==============================================================================
--- hadoop/common/branches/branch-0.20/CHANGES.txt (original)
+++ hadoop/common/branches/branch-0.20/CHANGES.txt Tue Aug 11 01:08:37 2009
@@ -198,7 +198,10 @@
 
     MAPREDUCE-796. Fixes a ClassCastException in an exception log in
     MultiThreadedMapRunner. (Amar Kamat via ddas)
-   
+
+    HDFS-525. The SimpleDateFormat object in ListPathsServlet is not thread
+    safe.  (Suresh Srinivas via szetszwo)
+ 
 Release 0.20.0 - 2009-04-15
 
   INCOMPATIBLE CHANGES

Modified: hadoop/common/branches/branch-0.20/src/hdfs/org/apache/hadoop/hdfs/HftpFileSystem.java
URL: http://svn.apache.org/viewvc/hadoop/common/branches/branch-0.20/src/hdfs/org/apache/hadoop/hdfs/HftpFileSystem.java?rev=802971&r1=802970&r2=802971&view=diff
==============================================================================
--- hadoop/common/branches/branch-0.20/src/hdfs/org/apache/hadoop/hdfs/HftpFileSystem.java (original)
+++ hadoop/common/branches/branch-0.20/src/hdfs/org/apache/hadoop/hdfs/HftpFileSystem.java Tue Aug 11 01:08:37 2009
@@ -31,7 +31,6 @@
 import java.net.UnknownHostException;
 
 import java.text.ParseException;
-import java.text.SimpleDateFormat;
 
 import java.util.ArrayList;
 import java.util.Random;
@@ -56,6 +55,7 @@
 import org.apache.hadoop.fs.MD5MD5CRC32FileChecksum;
 import org.apache.hadoop.fs.Path;
 import org.apache.hadoop.fs.permission.FsPermission;
+import org.apache.hadoop.hdfs.server.common.ThreadLocalDateFormat;
 import org.apache.hadoop.hdfs.server.namenode.ListPathsServlet;
 import org.apache.hadoop.ipc.RemoteException;
 import org.apache.hadoop.security.*;
@@ -77,7 +77,7 @@
   protected UserGroupInformation ugi; 
   protected final Random ran = new Random();
 
-  protected static final SimpleDateFormat df = ListPathsServlet.df;
+  protected static final ThreadLocalDateFormat df = ListPathsServlet.df;
 
   @Override
   public void initialize(URI name, Configuration conf) throws IOException {

Added: hadoop/common/branches/branch-0.20/src/hdfs/org/apache/hadoop/hdfs/server/common/ThreadLocalDateFormat.java
URL: http://svn.apache.org/viewvc/hadoop/common/branches/branch-0.20/src/hdfs/org/apache/hadoop/hdfs/server/common/ThreadLocalDateFormat.java?rev=802971&view=auto
==============================================================================
--- hadoop/common/branches/branch-0.20/src/hdfs/org/apache/hadoop/hdfs/server/common/ThreadLocalDateFormat.java (added)
+++ hadoop/common/branches/branch-0.20/src/hdfs/org/apache/hadoop/hdfs/server/common/ThreadLocalDateFormat.java Tue Aug 11 01:08:37 2009
@@ -0,0 +1,87 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.hadoop.hdfs.server.common;
+
+import java.text.ParseException;
+import java.text.SimpleDateFormat;
+import java.util.Date;
+import java.util.TimeZone;
+
+/**
+ * Thread safe implementation of {@link SimpleDateFormat} 
+ * TODO: This needs to be moved to hadoop common project.
+ */
+public class ThreadLocalDateFormat {
+  private final String format;
+
+  /**
+   * Constructs {@link ThreadLocalDateFormat} using given date format pattern
+   * @param format Date format pattern
+   */
+  public ThreadLocalDateFormat(String format) {
+    this.format = format;
+  }
+
+  /**
+   * ThreadLocal based {@link SimpleDateFormat}
+   */
+  private final ThreadLocal<SimpleDateFormat> dateFormat = 
+    new ThreadLocal<SimpleDateFormat>() {
+      @Override
+      protected SimpleDateFormat initialValue() {
+        SimpleDateFormat df = new SimpleDateFormat(format);
+        return df;
+      }
+    };
+
+  /**
+   * Format given <code>Date</code> into date/time string.
+   * @param date Date to be formatted.
+   * @return the formatted date-time string.
+   */
+  public String format(Date date) {
+    return dateFormat.get().format(date);
+  }
+
+  /**
+   * Parse the String to produce <code>Date</code>.
+   * @param source String to parse.
+   * @return Date parsed from the String.
+   * @throws ParseException
+   *           - if the beginning of the specified string cannot be parsed.
+   */
+  public Date parse(String source) throws ParseException {
+    return dateFormat.get().parse(source);
+  }
+
+  /**
+   * @param zone
+   */
+  public void setTimeZone(TimeZone zone) {
+    dateFormat.get().setTimeZone(zone);
+  }
+
+  /**
+   * Get access to underlying SimpleDateFormat.
+   * Note: Do not pass reference to this Date to other threads!
+   * @return the SimpleDateFormat for the thread.
+   */
+  SimpleDateFormat get() {
+    return dateFormat.get();
+  }
+}

Modified: hadoop/common/branches/branch-0.20/src/hdfs/org/apache/hadoop/hdfs/server/namenode/ListPathsServlet.java
URL: http://svn.apache.org/viewvc/hadoop/common/branches/branch-0.20/src/hdfs/org/apache/hadoop/hdfs/server/namenode/ListPathsServlet.java?rev=802971&r1=802970&r2=802971&view=diff
==============================================================================
--- hadoop/common/branches/branch-0.20/src/hdfs/org/apache/hadoop/hdfs/server/namenode/ListPathsServlet.java (original)
+++ hadoop/common/branches/branch-0.20/src/hdfs/org/apache/hadoop/hdfs/server/namenode/ListPathsServlet.java Tue Aug 11 01:08:37 2009
@@ -19,6 +19,7 @@
 
 import org.apache.hadoop.fs.FileStatus;
 import org.apache.hadoop.hdfs.protocol.ClientProtocol;
+import org.apache.hadoop.hdfs.server.common.ThreadLocalDateFormat;
 import org.apache.hadoop.ipc.RemoteException;
 import org.apache.hadoop.security.UnixUserGroupInformation;
 import org.apache.hadoop.util.VersionInfo;
@@ -27,14 +28,12 @@
 
 import java.io.IOException;
 import java.io.PrintWriter;
-import java.text.SimpleDateFormat;
 import java.util.Date;
 import java.util.HashMap;
 import java.util.Map;
 import java.util.Stack;
 import java.util.TimeZone;
 import java.util.regex.Pattern;
-import java.util.regex.PatternSyntaxException;
 import javax.servlet.ServletException;
 import javax.servlet.http.HttpServletRequest;
 import javax.servlet.http.HttpServletResponse;
@@ -46,9 +45,9 @@
 public class ListPathsServlet extends DfsServlet {
   /** For java.io.Serializable */
   private static final long serialVersionUID = 1L;
+  public static final ThreadLocalDateFormat df = 
+    new ThreadLocalDateFormat("yyyy-MM-dd'T'HH:mm:ssZ");
 
-  public static final SimpleDateFormat df =
-    new SimpleDateFormat("yyyy-MM-dd'T'HH:mm:ssZ");
   static {
     df.setTimeZone(TimeZone.getTimeZone("UTC"));
   }
@@ -163,13 +162,10 @@
         }
         catch(RemoteException re) {re.writeXml(p, doc);}
       }
-    } catch (PatternSyntaxException e) {
-      out.println(e.toString());
-    } finally {
       if (doc != null) {
         doc.endDocument();
       }
-
+    } finally {
       if (out != null) {
         out.close();
       }

Added: hadoop/common/branches/branch-0.20/src/test/org/apache/hadoop/hdfs/server/common/TestThreadLocalDateFormat.java
URL: http://svn.apache.org/viewvc/hadoop/common/branches/branch-0.20/src/test/org/apache/hadoop/hdfs/server/common/TestThreadLocalDateFormat.java?rev=802971&view=auto
==============================================================================
--- hadoop/common/branches/branch-0.20/src/test/org/apache/hadoop/hdfs/server/common/TestThreadLocalDateFormat.java (added)
+++ hadoop/common/branches/branch-0.20/src/test/org/apache/hadoop/hdfs/server/common/TestThreadLocalDateFormat.java Tue Aug 11 01:08:37 2009
@@ -0,0 +1,107 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.hadoop.hdfs.server.common;
+
+import java.text.ParseException;
+import java.text.SimpleDateFormat;
+import java.util.Date;
+import java.util.Random;
+import java.util.TimeZone;
+
+import org.apache.commons.logging.Log;
+import org.apache.commons.logging.LogFactory;
+import junit.framework.TestCase;
+
+/**
+ * Test for {@link ThreadLocalDateFormat}
+ */
+public class TestThreadLocalDateFormat extends TestCase {
+  private static final int TOTAL_THREADS = 3;
+  private static final Log LOG = LogFactory.getLog(TestThreadLocalDateFormat.class);
+  private static final ThreadLocalDateFormat TDF = new ThreadLocalDateFormat(
+      "dd-MM-yyyy HH:mm:ss:S Z");
+  private static volatile boolean failed = false;
+  private final static Random rand = new Random();
+
+  private static synchronized void setFailed() {
+    failed = true;
+  }
+
+  /**
+   * Run formatting and parsing test and look for multi threaded access related
+   * failures
+   */
+  private void runTest(final SimpleDateFormat df) {
+    while (!failed) {
+      try {
+        df.setTimeZone(TimeZone.getTimeZone("UTC"));
+        Date date = new Date(rand.nextInt(Integer.MAX_VALUE));
+        String s1 = df.format(date);
+        Date parsedDate = df.parse(s1);
+        String s2 = df.format(parsedDate);
+        if (!s1.equals(s2)) {
+          LOG.warn("Parse failed, actual /" + s2 + "/ expected /" + s1 + "/");
+          setFailed();
+        }
+      } catch (ArrayIndexOutOfBoundsException e) {
+        setFailed();
+        LOG.warn("exception ", e);
+      } catch (ParseException e) {
+        LOG.warn("Parsing failed ", e);
+        setFailed();
+      } catch (Exception e) {
+        LOG.warn("Unknown exception", e);
+        setFailed();
+      }
+    }
+  }
+
+  /**
+   * {@link SimpleDateFormat} when using with multiple threads has following
+   * issues:
+   * <ul>
+   * <li>format method throws {@link ArrayIndexOutOfBoundsException}
+   * <li>parse method throws {@link ParseException} or returns invalid parse
+   * </ul>
+   * This test shows ThreadLocal based implementation of
+   * {@link SimpleDateFormat} does not have these issues.
+   * 
+   * @throws InterruptedException
+   */
+  public void testDateFormat() throws InterruptedException {
+    for (int i = 0; i < TOTAL_THREADS; i++) {
+      Thread thread = new Thread() {
+        public void run() {
+          runTest(TDF.get());
+        }
+      };
+      thread.start();
+    }
+
+    // Wait up to 30 seconds for failure to occur
+    long endTime = System.currentTimeMillis() + 30 * 1000;
+    while (!failed && endTime > System.currentTimeMillis()) {
+      try {
+        Thread.sleep(1000);
+      } catch (InterruptedException ie) {
+        LOG.debug("Exception", ie);
+      }
+    }
+    assertFalse(failed);
+  }
+}