HDFS-13702. Remove HTrace hooks from DFSClient to reduce CPU usage. Contributed by Todd Lipcon.

(cherry picked from commit 5d748bd)

Change-Id: If054510f4b32203bba632ec1456110c8ef82bc8a

Author: umbrant

hadoop-hdfs-project/hadoop-hdfs-client/src/main/java/org/apache/hadoop/hdfs/DFSClient.java

@@ -2977,25 +2977,6 @@ TraceScope newSrcDstTraceScope(String description, String src, String dst) {
     return scope;
   }
 
-  /**
-   * Full detailed tracing for read requests: path, position in the file,
-   * and length.
-   *
-   * @param reqLen requested length
-   */
-  TraceScope newReaderTraceScope(String description, String path, long pos,
-      int reqLen) {
-    TraceScope scope = newPathTraceScope(description, path);
-    scope.addKVAnnotation("pos", Long.toString(pos));
-    scope.addKVAnnotation("reqLen", Integer.toString(reqLen));
-    return scope;
-  }
-
-  /** Add the returned length info to the scope. */
-  void addRetLenToReaderScope(TraceScope scope, int retLen) {
-    scope.addKVAnnotation("retLen", Integer.toString(retLen));
-  }
-
   /**
    * Get the erasure coding policy information for the specified path
    *

hadoop-hdfs-project/hadoop-hdfs-client/src/main/java/org/apache/hadoop/hdfs/DFSInputStream.java

@@ -84,8 +84,6 @@
 import org.apache.hadoop.util.StopWatch;
 import org.apache.hadoop.util.StringUtils;
 import org.apache.htrace.core.SpanId;
-import org.apache.htrace.core.TraceScope;
-import org.apache.htrace.core.Tracer;
 
 import com.google.common.annotations.VisibleForTesting;
 
@@ -640,7 +638,6 @@ protected BlockReader getBlockReader(LocatedBlock targetBlock,
         setClientCacheContext(dfsClient.getClientContext()).
         setUserGroupInformation(dfsClient.ugi).
         setConfiguration(dfsClient.getConfiguration()).
-        setTracer(dfsClient.getTracer()).
         build();
   }
 
@@ -820,31 +817,14 @@ public synchronized int read(@Nonnull final byte buf[], int off, int len)
     }
     ReaderStrategy byteArrayReader =
         new ByteArrayStrategy(buf, off, len, readStatistics, dfsClient);
-    try (TraceScope scope =
-             dfsClient.newReaderTraceScope("DFSInputStream#byteArrayRead",
-                 src, getPos(), len)) {
-      int retLen = readWithStrategy(byteArrayReader);
-      if (retLen < len) {
-        dfsClient.addRetLenToReaderScope(scope, retLen);
-      }
-      return retLen;
-    }
+    return readWithStrategy(byteArrayReader);
   }
 
   @Override
   public synchronized int read(final ByteBuffer buf) throws IOException {
     ReaderStrategy byteBufferReader =
         new ByteBufferStrategy(buf, readStatistics, dfsClient);
-    int reqLen = buf.remaining();
-    try (TraceScope scope =
-             dfsClient.newReaderTraceScope("DFSInputStream#byteBufferRead",
-                 src, getPos(), reqLen)){
-      int retLen = readWithStrategy(byteBufferReader);
-      if (retLen < reqLen) {
-        dfsClient.addRetLenToReaderScope(scope, retLen);
-      }
-      return retLen;
-    }
+    return readWithStrategy(byteBufferReader);
   }
 
   private DNAddrPair chooseDataNode(LocatedBlock block,
@@ -1025,16 +1005,12 @@ private Callable<ByteBuffer> getFromOneDataNode(final DNAddrPair datanode,
       final ByteBuffer bb,
       final CorruptedBlocks corruptedBlocks,
       final int hedgedReadId) {
-    final SpanId parentSpanId = Tracer.getCurrentSpanId();
     return new Callable<ByteBuffer>() {
       @Override
       public ByteBuffer call() throws Exception {
         DFSClientFaultInjector.get().sleepBeforeHedgedGet();
-        try (TraceScope ignored = dfsClient.getTracer().
-            newScope("hedgedRead" + hedgedReadId, parentSpanId)) {
-          actualGetFromOneDataNode(datanode, start, end, bb, corruptedBlocks);
-          return bb;
-        }
+        actualGetFromOneDataNode(datanode, start, end, bb, corruptedBlocks);
+        return bb;
       }
     };
   }
@@ -1332,16 +1308,8 @@ public int read(long position, byte[] buffer, int offset, int length)
     if (length == 0) {
       return 0;
     }
-    try (TraceScope scope = dfsClient.
-        newReaderTraceScope("DFSInputStream#byteArrayPread",
-            src, position, length)) {
-      ByteBuffer bb = ByteBuffer.wrap(buffer, offset, length);
-      int retLen = pread(position, bb);
-      if (retLen < length) {
-        dfsClient.addRetLenToReaderScope(scope, retLen);
-      }
-      return retLen;
-    }
+    ByteBuffer bb = ByteBuffer.wrap(buffer, offset, length);
+    return pread(position, bb);
   }
 
   private int pread(long position, ByteBuffer buffer)

hadoop-hdfs-project/hadoop-hdfs-client/src/main/java/org/apache/hadoop/hdfs/client/impl/BlockReaderFactory.java

@@ -75,7 +75,6 @@
 
 import com.google.common.annotations.VisibleForTesting;
 import com.google.common.base.Preconditions;
-import org.apache.htrace.core.Tracer;
 
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
@@ -189,11 +188,6 @@ public boolean getSupportsReceiptVerification() {
    */
   private Configuration configuration;
 
-  /**
-   * The HTrace tracer to use.
-   */
-  private Tracer tracer;
-
   /**
    * Information about the domain socket path we should use to connect to the
    * local peer-- or null if we haven't examined the local domain socket.
@@ -298,11 +292,6 @@ public BlockReaderFactory setConfiguration(
     return this;
   }
 
-  public BlockReaderFactory setTracer(Tracer tracer) {
-    this.tracer = tracer;
-    return this;
-  }
-
   @VisibleForTesting
   public static void setFailureInjectorForTesting(FailureInjector injector) {
     failureInjector = injector;
@@ -447,7 +436,7 @@ private BlockReader getLegacyBlockReaderLocal() throws IOException {
     try {
       return BlockReaderLocalLegacy.newBlockReader(conf,
           userGroupInformation, configuration, fileName, block, token,
-          datanode, startOffset, length, storageType, tracer);
+          datanode, startOffset, length, storageType);
     } catch (RemoteException remoteException) {
       ioe = remoteException.unwrapRemoteException(
                 InvalidToken.class, AccessControlException.class);
@@ -505,7 +494,6 @@ private BlockReader getBlockReaderLocal() throws InvalidToken {
         setVerifyChecksum(verifyChecksum).
         setCachingStrategy(cachingStrategy).
         setStorageType(storageType).
-        setTracer(tracer).
         build();
   }
 
@@ -856,7 +844,7 @@ private BlockReader getRemoteBlockReader(Peer peer) throws IOException {
     return BlockReaderRemote.newBlockReader(
         fileName, block, token, startOffset, length,
         verifyChecksum, clientName, peer, datanode,
-        clientContext.getPeerCache(), cachingStrategy, tracer,
+        clientContext.getPeerCache(), cachingStrategy,
         networkDistance);
   }
 

hadoop-hdfs-project/hadoop-hdfs-client/src/main/java/org/apache/hadoop/hdfs/client/impl/BlockReaderLocal.java

@@ -35,8 +35,6 @@
 import org.apache.hadoop.util.DataChecksum;
 import org.apache.hadoop.util.DirectBufferPool;
 import org.apache.hadoop.util.Timer;
-import org.apache.htrace.core.TraceScope;
-import org.apache.htrace.core.Tracer;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
 
@@ -83,7 +81,6 @@ public static class Builder {
     private long dataPos;
     private ExtendedBlock block;
     private StorageType storageType;
-    private Tracer tracer;
     private ShortCircuitConf shortCircuitConf;
 
     public Builder(ShortCircuitConf conf) {
@@ -131,11 +128,6 @@ public Builder setStorageType(StorageType storageType) {
       return this;
     }
 
-    public Builder setTracer(Tracer tracer) {
-      this.tracer = tracer;
-      return this;
-    }
-
     public BlockReaderLocal build() {
       Preconditions.checkNotNull(replica);
       return new BlockReaderLocal(this);
@@ -244,11 +236,6 @@ public BlockReaderLocal build() {
    */
   private StorageType storageType;
 
-  /**
-   * The Tracer to use.
-   */
-  private final Tracer tracer;
-
   private BlockReaderLocal(Builder builder) {
     this.replica = builder.replica;
     this.dataIn = replica.getDataStream().getChannel();
@@ -278,7 +265,6 @@ private BlockReaderLocal(Builder builder) {
     }
     this.maxReadaheadLength = maxReadaheadChunks * bytesPerChecksum;
     this.storageType = builder.storageType;
-    this.tracer = builder.tracer;
 
     if (builder.shortCircuitConf.isScrMetricsEnabled()) {
       metricsInitializationLock.lock();
@@ -360,52 +346,49 @@ private synchronized int drainDataBuf(ByteBuffer buf) {
    */
   private synchronized int fillBuffer(ByteBuffer buf, boolean canSkipChecksum)
       throws IOException {
-    try (TraceScope ignored = tracer.newScope(
-        "BlockReaderLocal#fillBuffer(" + block.getBlockId() + ")")) {
-      int total = 0;
-      long startDataPos = dataPos;
-      int startBufPos = buf.position();
-      while (buf.hasRemaining()) {
-        int nRead = blockReaderIoProvider.read(dataIn, buf, dataPos);
-        if (nRead < 0) {
-          break;
-        }
-        dataPos += nRead;
-        total += nRead;
-      }
-      if (canSkipChecksum) {
-        freeChecksumBufIfExists();
-        return total;
+    int total = 0;
+    long startDataPos = dataPos;
+    int startBufPos = buf.position();
+    while (buf.hasRemaining()) {
+      int nRead = blockReaderIoProvider.read(dataIn, buf, dataPos);
+      if (nRead < 0) {
+        break;
       }
-      if (total > 0) {
-        try {
-          buf.limit(buf.position());
-          buf.position(startBufPos);
-          createChecksumBufIfNeeded();
-          int checksumsNeeded = (total + bytesPerChecksum - 1) /
-              bytesPerChecksum;
-          checksumBuf.clear();
-          checksumBuf.limit(checksumsNeeded * checksumSize);
-          long checksumPos = BlockMetadataHeader.getHeaderSize()
-              + ((startDataPos / bytesPerChecksum) * checksumSize);
-          while (checksumBuf.hasRemaining()) {
-            int nRead = checksumIn.read(checksumBuf, checksumPos);
-            if (nRead < 0) {
-              throw new IOException("Got unexpected checksum file EOF at " +
-                  checksumPos + ", block file position " + startDataPos +
-                  " for block " + block + " of file " + filename);
-            }
-            checksumPos += nRead;
+      dataPos += nRead;
+      total += nRead;
+    }
+    if (canSkipChecksum) {
+      freeChecksumBufIfExists();
+      return total;
+    }
+    if (total > 0) {
+      try {
+        buf.limit(buf.position());
+        buf.position(startBufPos);
+        createChecksumBufIfNeeded();
+        int checksumsNeeded = (total + bytesPerChecksum - 1) /
+            bytesPerChecksum;
+        checksumBuf.clear();
+        checksumBuf.limit(checksumsNeeded * checksumSize);
+        long checksumPos = BlockMetadataHeader.getHeaderSize()
+            + ((startDataPos / bytesPerChecksum) * checksumSize);
+        while (checksumBuf.hasRemaining()) {
+          int nRead = checksumIn.read(checksumBuf, checksumPos);
+          if (nRead < 0) {
+            throw new IOException("Got unexpected checksum file EOF at " +
+                checksumPos + ", block file position " + startDataPos +
+                " for block " + block + " of file " + filename);
           }
-          checksumBuf.flip();
-
-          checksum.verifyChunkedSums(buf, checksumBuf, filename, startDataPos);
-        } finally {
-          buf.position(buf.limit());
+          checksumPos += nRead;
         }
+        checksumBuf.flip();
+
+        checksum.verifyChunkedSums(buf, checksumBuf, filename, startDataPos);
+      } finally {
+        buf.position(buf.limit());
       }
-      return total;
     }
+    return total;
   }
 
   private boolean createNoChecksumContext() {