hadoop-mapreduce-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From cdoug...@apache.org
Subject svn commit: r984695 - in /hadoop/mapreduce/trunk: CHANGES.txt src/test/mapred/org/apache/hadoop/tools/rumen/TestParsedLine.java src/tools/org/apache/hadoop/tools/rumen/ParsedLine.java
Date Thu, 12 Aug 2010 09:13:04 GMT
Author: cdouglas
Date: Thu Aug 12 09:13:03 2010
New Revision: 984695

URL: http://svn.apache.org/viewvc?rev=984695&view=rev
Log:
MAPREDUCE-2000. Fix parsing of JobHistory lines in Rumen when quotes are
escaped. Contributed by Hong Tang

Added:
    hadoop/mapreduce/trunk/src/test/mapred/org/apache/hadoop/tools/rumen/TestParsedLine.java
Modified:
    hadoop/mapreduce/trunk/CHANGES.txt
    hadoop/mapreduce/trunk/src/tools/org/apache/hadoop/tools/rumen/ParsedLine.java

Modified: hadoop/mapreduce/trunk/CHANGES.txt
URL: http://svn.apache.org/viewvc/hadoop/mapreduce/trunk/CHANGES.txt?rev=984695&r1=984694&r2=984695&view=diff
==============================================================================
--- hadoop/mapreduce/trunk/CHANGES.txt (original)
+++ hadoop/mapreduce/trunk/CHANGES.txt Thu Aug 12 09:13:03 2010
@@ -126,9 +126,6 @@ Trunk (unreleased changes)
     MAPREDUCE-1829. JobInProgress.findSpeculativeTask should use min() to
     find the candidate instead of sort(). (Scott Chen via vinodkv)
 
-    MAPREDUCE-1961. Fix ConcurrentModificationException in Gridmix during
-    shutdown. (Hong Tang via cdouglas)
-
   BUG FIXES
 
     MAPREDUCE-1845. FairScheduler.tasksToPreempt() can return negative number.
@@ -248,6 +245,12 @@ Trunk (unreleased changes)
     MAPREDUCE-1780. AccessControlList.toString() is used for serialization of
     ACL in JobStatus.java. (Ravi Gummadi via vinodkv)
 
+    MAPREDUCE-1961. Fix ConcurrentModificationException in Gridmix during
+    shutdown. (Hong Tang via cdouglas)
+
+    MAPREDUCE-2000. Fix parsing of JobHistory lines in Rumen when quotes are
+    escaped. (Hong Tang via cdouglas)
+
 Release 0.21.0 - Unreleased
 
   INCOMPATIBLE CHANGES

Added: hadoop/mapreduce/trunk/src/test/mapred/org/apache/hadoop/tools/rumen/TestParsedLine.java
URL: http://svn.apache.org/viewvc/hadoop/mapreduce/trunk/src/test/mapred/org/apache/hadoop/tools/rumen/TestParsedLine.java?rev=984695&view=auto
==============================================================================
--- hadoop/mapreduce/trunk/src/test/mapred/org/apache/hadoop/tools/rumen/TestParsedLine.java
(added)
+++ hadoop/mapreduce/trunk/src/test/mapred/org/apache/hadoop/tools/rumen/TestParsedLine.java
Thu Aug 12 09:13:03 2010
@@ -0,0 +1,105 @@
+/**
+ * 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.tools.rumen;
+
+import org.apache.hadoop.util.StringUtils;
+import org.junit.Test;
+import static org.junit.Assert.*;
+
+public class TestParsedLine {
+  static final char[] CHARS_TO_ESCAPE = new char[]{'=', '"', '.'};
+  
+  String buildLine(String type, String[] kvseq) {
+    StringBuilder sb = new StringBuilder();
+    sb.append(type);
+    for (int i=0; i<kvseq.length; ++i) {
+      sb.append(" ");
+      if (kvseq[i].equals(".") || kvseq[i].equals("\n")) {
+        sb.append(kvseq[i]);
+        continue;
+      }
+      if (i == kvseq.length-1) {
+        fail("Incorrect input, expecting value.");
+      }
+      sb.append(kvseq[i++]);
+      sb.append("=\"");
+      sb.append(StringUtils.escapeString(kvseq[i], StringUtils.ESCAPE_CHAR,
+          CHARS_TO_ESCAPE));
+      sb.append("\"");
+    }
+    return sb.toString();
+  }
+  
+  void testOneLine(String type, String... kvseq) {
+    String line = buildLine(type, kvseq);
+    ParsedLine pl = new ParsedLine(line, Hadoop20JHParser.internalVersion);
+    assertEquals("Mismatching type", type, pl.getType().toString());
+    for (int i = 0; i < kvseq.length; ++i) {
+      if (kvseq[i].equals(".") || kvseq[i].equals("\n")) {
+        continue;
+      }
+
+      assertEquals("Key mismatching for " + kvseq[i], kvseq[i + 1], StringUtils
+          .unEscapeString(pl.get(kvseq[i]), StringUtils.ESCAPE_CHAR,
+              CHARS_TO_ESCAPE));
+      ++i;
+    }
+  }
+  
+  @Test
+  public void testEscapedQuote() {
+    testOneLine("REC", "A", "x", "B", "abc\"de", "C", "f");
+    testOneLine("REC", "B", "abcde\"", "C", "f");
+    testOneLine("REC", "A", "x", "B", "\"abcde");
+  }
+
+  @Test
+  public void testEqualSign() {
+    testOneLine("REC1", "A", "x", "B", "abc=de", "C", "f");
+    testOneLine("REC2", "B", "=abcde", "C", "f");
+    testOneLine("REC3", "A", "x", "B", "abcde=");
+  }
+
+  @Test
+  public void testSpace() {
+    testOneLine("REC1", "A", "x", "B", "abc de", "C", "f");
+    testOneLine("REC2", "B", " ab c de", "C", "f");
+    testOneLine("REC3", "A", "x", "B", "abc\t  de  ");
+  }
+
+  @Test
+  public void testBackSlash() {
+    testOneLine("REC1", "A", "x", "B", "abc\\de", "C", "f");
+    testOneLine("REC2", "B", "\\ab\\c\\de", "C", "f");
+    testOneLine("REC3", "A", "x", "B", "abc\\\\de\\");
+    testOneLine("REC4", "A", "x", "B", "abc\\\"de\\\"", "C", "f");
+  }
+
+  @Test
+  public void testLineDelimiter() {
+    testOneLine("REC1", "A", "x", "B", "abc.de", "C", "f");
+    testOneLine("REC2", "B", ".ab.de");
+    testOneLine("REC3", "A", "x", "B", "abc.de.");
+    testOneLine("REC4", "A", "x", "B", "abc.de", ".");
+  }
+  
+  @Test
+  public void testMultipleLines() {
+    testOneLine("REC1", "A", "x", "\n", "B", "abc.de", "\n", "C", "f", "\n", ".");
+  }
+}

Modified: hadoop/mapreduce/trunk/src/tools/org/apache/hadoop/tools/rumen/ParsedLine.java
URL: http://svn.apache.org/viewvc/hadoop/mapreduce/trunk/src/tools/org/apache/hadoop/tools/rumen/ParsedLine.java?rev=984695&r1=984694&r2=984695&view=diff
==============================================================================
--- hadoop/mapreduce/trunk/src/tools/org/apache/hadoop/tools/rumen/ParsedLine.java (original)
+++ hadoop/mapreduce/trunk/src/tools/org/apache/hadoop/tools/rumen/ParsedLine.java Thu Aug
12 09:13:03 2010
@@ -18,14 +18,33 @@
 package org.apache.hadoop.tools.rumen;
 
 import java.util.Properties;
+import java.util.regex.Matcher;
 import java.util.regex.Pattern;
 
 class ParsedLine {
   Properties content;
   LogRecordType type;
 
-  static final Pattern keyValPair =
-      Pattern.compile(" *([a-zA-Z0-9_]+)=\"((?:[^\"\\\\]|\\\\[ .\"\\\\])*)\"");
+  static final String KEY = "(\\w+)";
+  /**
+   * The value string is enclosed in double quotation marks ('"') and
+   * occurrences of '"' and '\' are escaped with a '\'. So the escaped value
+   * string is essentially a string of escaped sequence ('\' followed by any
+   * character) or any character other than '"' and '\'.
+   * 
+   * The straightforward REGEX to capture the above is "((?:[^\"\\\\]|\\\\.)*)".
+   * Unfortunately Java's REGEX implementation is "broken" that it does not
+   * perform the NFA-to-DFA conversion and such expressions would lead to
+   * backtracking and stack overflow when matching with long strings. The
+   * following is a manual "unfolding" of the REGEX to get rid of backtracking.
+   */
+  static final String VALUE = "([^\"\\\\]*+(?:\\\\.[^\"\\\\]*+)*+)";
+  /**
+   * REGEX to match the Key-Value pairs in an input line. Capture group 1
+   * matches the key and capture group 2 matches the value (without quotation
+   * marks).
+   */
+  static final Pattern keyValPair = Pattern.compile(KEY + "=" + "\"" + VALUE + "\"");
 
   @SuppressWarnings("unused")
   ParsedLine(String fullLine, int version) {
@@ -47,65 +66,12 @@ class ParsedLine {
 
     String propValPairs = fullLine.substring(firstSpace + 1);
 
-    int pvPairsFirstNonBlank = 0;
-    int pvPairsLength = propValPairs.length();
+    Matcher matcher = keyValPair.matcher(propValPairs);
 
-    while (pvPairsLength > pvPairsFirstNonBlank
-        && propValPairs.charAt(pvPairsFirstNonBlank) == ' ') {
-      ++pvPairsFirstNonBlank;
-    }
-
-    if (pvPairsFirstNonBlank != 0) {
-      propValPairs = propValPairs.substring(pvPairsFirstNonBlank);
-    }
-
-    int cursor = 0;
-
-    while (cursor < propValPairs.length()) {
-      int equals = propValPairs.indexOf('=', cursor);
-
-      if (equals < 0) {
-        // maybe we do some error processing
-        return;
-      }
-
-      int nextCursor;
-
-      int endValue;
-
-      if (propValPairs.charAt(equals + 1) == '\"') {
-        int closeQuote = propValPairs.indexOf('\"', equals + 2);
-
-        nextCursor = closeQuote + 1;
-
-        endValue = closeQuote;
-
-        if (closeQuote < 0) {
-          endValue = propValPairs.length();
-
-          nextCursor = endValue;
-        }
-      } else {
-        int closeSpace = propValPairs.indexOf(' ', equals + 1);
-
-        if (closeSpace < 0) {
-          closeSpace = propValPairs.length();
-        }
-
-        endValue = closeSpace;
-
-        nextCursor = endValue;
-      }
-
-      content.setProperty(propValPairs.substring(cursor, equals), propValPairs
-          .substring(equals + 2, endValue));
-
-      cursor = nextCursor;
-
-      while (cursor < propValPairs.length()
-          && propValPairs.charAt(cursor) == ' ') {
-        ++cursor;
-      }
+    while(matcher.find()){
+      String key = matcher.group(1);
+      String value = matcher.group(2);
+      content.setProperty(key, value);
     }
   }
 



Mime
View raw message