You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@pinot.apache.org by ri...@apache.org on 2022/04/05 12:40:29 UTC

[pinot] branch tracing-spi created (now 4aaa19eef2)

This is an automated email from the ASF dual-hosted git repository.

richardstartin pushed a change to branch tracing-spi
in repository https://gitbox.apache.org/repos/asf/pinot.git


      at 4aaa19eef2 add tracing SPI

This branch includes the following new commits:

     new 4aaa19eef2 add tracing SPI

The 1 revisions listed above as "new" are entirely new to this
repository and will be described in separate emails.  The revisions
listed as "add" were already present in the repository and have only
been added to this reference.



---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org


[pinot] 01/01: add tracing SPI

Posted by ri...@apache.org.
This is an automated email from the ASF dual-hosted git repository.

richardstartin pushed a commit to branch tracing-spi
in repository https://gitbox.apache.org/repos/asf/pinot.git

commit 4aaa19eef2a7016e4e2b725a7aa357fc51f491d5
Author: richardstartin <ri...@startree.ai>
AuthorDate: Fri Apr 1 16:21:43 2022 +0100

    add tracing SPI
---
 .../apache/pinot/core/operator/BaseOperator.java   |  23 ++---
 .../query/executor/ServerQueryExecutorV1Impl.java  |   3 +-
 .../pinot/core/util/trace/DefaultTracer.java       | 100 +++++++++++++++++++++
 .../pinot/core/util/trace/TraceContextTest.java    |   3 +-
 .../apache/pinot/spi/trace/ExecutionRecording.java |  71 +++++++++++++++
 .../org/apache/pinot/spi/trace/FilterType.java     |  24 +++++
 .../apache/pinot/spi/trace/OperatorExecution.java  |  22 +++++
 .../java/org/apache/pinot/spi/trace/Phase.java     |  27 ++++++
 .../java/org/apache/pinot/spi/trace/Scope.java     |  25 ++++++
 .../java/org/apache/pinot/spi/trace/Tracer.java    |  30 +++++++
 .../java/org/apache/pinot/spi/trace/Tracing.java   |  70 +++++++++++++++
 11 files changed, 382 insertions(+), 16 deletions(-)

diff --git a/pinot-core/src/main/java/org/apache/pinot/core/operator/BaseOperator.java b/pinot-core/src/main/java/org/apache/pinot/core/operator/BaseOperator.java
index e99d5c59a1..369f936e95 100644
--- a/pinot-core/src/main/java/org/apache/pinot/core/operator/BaseOperator.java
+++ b/pinot-core/src/main/java/org/apache/pinot/core/operator/BaseOperator.java
@@ -20,36 +20,31 @@ package org.apache.pinot.core.operator;
 
 import org.apache.pinot.core.common.Block;
 import org.apache.pinot.core.common.Operator;
-import org.apache.pinot.core.util.trace.TraceContext;
 import org.apache.pinot.spi.exception.EarlyTerminationException;
-import org.slf4j.Logger;
-import org.slf4j.LoggerFactory;
+import org.apache.pinot.spi.trace.ExecutionRecording;
+import org.apache.pinot.spi.trace.OperatorExecution;
+import org.apache.pinot.spi.trace.Tracing;
 
 
 /**
  * Any other Pinot Operators should extend BaseOperator
  */
 public abstract class BaseOperator<T extends Block> implements Operator<T> {
-  private static final Logger LOGGER = LoggerFactory.getLogger(BaseOperator.class);
 
   @Override
   public final T nextBlock() {
     if (Thread.interrupted()) {
       throw new EarlyTerminationException();
     }
-    if (TraceContext.traceEnabled()) {
-      long start = System.currentTimeMillis();
-      T nextBlock = getNextBlock();
-      long end = System.currentTimeMillis();
-      String operatorName = getOperatorName();
-      LOGGER.trace("Time spent in {}: {}", operatorName, (end - start));
-      TraceContext.logTime(operatorName, (end - start));
-      return nextBlock;
-    } else {
-      return getNextBlock();
+    try (OperatorExecution execution = Tracing.getTracer().startOperatorExecution(getClass())) {
+      return getNextBlock(execution);
     }
   }
 
   // Make it protected because we should always call nextBlock()
   protected abstract T getNextBlock();
+
+  protected T getNextBlock(ExecutionRecording recording) {
+    return getNextBlock();
+  }
 }
diff --git a/pinot-core/src/main/java/org/apache/pinot/core/query/executor/ServerQueryExecutorV1Impl.java b/pinot-core/src/main/java/org/apache/pinot/core/query/executor/ServerQueryExecutorV1Impl.java
index 6dcf6a9d95..af5465d199 100644
--- a/pinot-core/src/main/java/org/apache/pinot/core/query/executor/ServerQueryExecutorV1Impl.java
+++ b/pinot-core/src/main/java/org/apache/pinot/core/query/executor/ServerQueryExecutorV1Impl.java
@@ -66,6 +66,7 @@ import org.apache.pinot.segment.spi.MutableSegment;
 import org.apache.pinot.segment.spi.SegmentMetadata;
 import org.apache.pinot.spi.env.PinotConfiguration;
 import org.apache.pinot.spi.exception.BadQueryRequestException;
+import org.apache.pinot.spi.trace.Tracing;
 import org.apache.pinot.spi.utils.CommonConstants;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
@@ -195,7 +196,7 @@ public class ServerQueryExecutorV1Impl implements QueryExecutor {
 
     boolean enableTrace = queryRequest.isEnableTrace();
     if (enableTrace) {
-      TraceContext.register(requestId);
+      Tracing.getTracer().register(requestId);
     }
 
     DataTable dataTable = null;
diff --git a/pinot-core/src/main/java/org/apache/pinot/core/util/trace/DefaultTracer.java b/pinot-core/src/main/java/org/apache/pinot/core/util/trace/DefaultTracer.java
new file mode 100644
index 0000000000..91522f2591
--- /dev/null
+++ b/pinot-core/src/main/java/org/apache/pinot/core/util/trace/DefaultTracer.java
@@ -0,0 +1,100 @@
+/**
+ * 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.pinot.core.util.trace;
+
+import org.apache.pinot.spi.data.FieldSpec;
+import org.apache.pinot.spi.trace.FilterType;
+import org.apache.pinot.spi.trace.OperatorExecution;
+import org.apache.pinot.spi.trace.Phase;
+import org.apache.pinot.spi.trace.Tracer;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+
+public class DefaultTracer implements Tracer {
+
+  private static final Logger LOGGER = LoggerFactory.getLogger(DefaultTracer.class);
+
+  private static class NoOpExecution implements OperatorExecution {
+
+    @Override
+    public void setDocsScanned(long docsScanned) {
+    }
+
+    @Override
+    public void setBytesProcessed(long bytesScanned) {
+    }
+
+    @Override
+    public void setFilterType(FilterType filterType, String predicateType) {
+    }
+
+    @Override
+    public void setPhase(Phase phase) {
+    }
+
+    @Override
+    public void setDataTypes(FieldSpec.DataType inputDataType, FieldSpec.DataType outputDataType) {
+    }
+
+    @Override
+    public void setDocIdRange(int firstDocId, int lastDocId) {
+    }
+
+    @Override
+    public void setColumnCardinality(int cardinality) {
+    }
+
+    @Override
+    public void close() {
+    }
+  }
+
+  private static final NoOpExecution NO_OP_SPAN = new NoOpExecution();
+
+  private static final class MillisExecution extends NoOpExecution {
+
+    private final long _startTimeMillis = System.currentTimeMillis();
+    private final Class<?> _operator;
+
+    public MillisExecution(Class<?> operator) {
+      _operator = operator;
+    }
+
+    @Override
+    public void close() {
+      String operatorName = _operator.getSimpleName();
+      long duration = System.currentTimeMillis() - _startTimeMillis;
+      if (LOGGER.isTraceEnabled()) {
+        LOGGER.trace("Time spent in {}: {}", operatorName, duration);
+      }
+      TraceContext.logTime(operatorName, duration);
+    }
+  }
+
+  @Override
+  public void register(long requestId) {
+    TraceContext.register(requestId);
+  }
+
+  @Override
+  public OperatorExecution startOperatorExecution(Class<?> operatorClass) {
+    return TraceContext.traceEnabled() ? new MillisExecution(operatorClass) : NO_OP_SPAN;
+  }
+}
diff --git a/pinot-core/src/test/java/org/apache/pinot/core/util/trace/TraceContextTest.java b/pinot-core/src/test/java/org/apache/pinot/core/util/trace/TraceContextTest.java
index 5ebdf7c7de..ba4a4acb0c 100644
--- a/pinot-core/src/test/java/org/apache/pinot/core/util/trace/TraceContextTest.java
+++ b/pinot-core/src/test/java/org/apache/pinot/core/util/trace/TraceContextTest.java
@@ -26,6 +26,7 @@ import java.util.Set;
 import java.util.concurrent.ExecutorService;
 import java.util.concurrent.Executors;
 import java.util.concurrent.Future;
+import org.apache.pinot.spi.trace.Tracing;
 import org.apache.pinot.spi.utils.JsonUtils;
 import org.testng.Assert;
 import org.testng.annotations.Test;
@@ -73,7 +74,7 @@ public class TraceContextTest {
   private void testSingleRequest(ExecutorService executorService, final long requestId)
       throws Exception {
     Set<String> expectedTraces = new HashSet<>(NUM_CHILDREN_PER_REQUEST + 1);
-    TraceContext.register(requestId);
+    Tracing.getTracer().register(requestId);
     String key = Integer.toString(RANDOM.nextInt());
     int value = RANDOM.nextInt();
     expectedTraces.add(getTraceString(key, value));
diff --git a/pinot-spi/src/main/java/org/apache/pinot/spi/trace/ExecutionRecording.java b/pinot-spi/src/main/java/org/apache/pinot/spi/trace/ExecutionRecording.java
new file mode 100644
index 0000000000..dd08aaa1ed
--- /dev/null
+++ b/pinot-spi/src/main/java/org/apache/pinot/spi/trace/ExecutionRecording.java
@@ -0,0 +1,71 @@
+/**
+ * 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.pinot.spi.trace;
+
+import org.apache.pinot.spi.data.FieldSpec;
+
+
+public interface ExecutionRecording {
+
+  /**
+   * Sets the number of docs scanned by the operator.
+   * @param docsScanned how many docs were scanned.
+   */
+  void setDocsScanned(long docsScanned);
+
+  /**
+   * Sets the number of bytes scanned by the operator if this is possible to compute.
+   * @param bytesScanned the number of bytes scanned
+   */
+  void setBytesProcessed(long bytesScanned);
+
+  /**
+   * If the operator is a filter, determines the filter type (scan or index) and the predicate type
+   * @param filterType SCAN or INDEX
+   * @param predicateType e.g. BETWEEN, REGEXP_LIKE
+   */
+  void setFilterType(FilterType filterType, String predicateType);
+
+  /**
+   * The phase of the query
+   * @param phase the phase
+   */
+  void setPhase(Phase phase);
+
+  /**
+   * Records whether type transformation took place during the operator's invocation and what the types were
+   * @param inputDataType the input data type
+   * @param outputDataType the output data type
+   */
+  void setDataTypes(FieldSpec.DataType inputDataType, FieldSpec.DataType outputDataType);
+
+  /**
+   * Records the range of docIds during the operator invocation. This is useful for implicating a range of records
+   * in a slow operator invocation.
+   * @param firstDocId the first docId in the block
+   * @param lastDocId the last docId in the block
+   */
+  void setDocIdRange(int firstDocId, int lastDocId);
+
+  /**
+   * If known, record the cardinality of the column within the segment this operator invoked on
+   * @param cardinality the number of distinct values
+   */
+  void setColumnCardinality(int cardinality);
+}
diff --git a/pinot-spi/src/main/java/org/apache/pinot/spi/trace/FilterType.java b/pinot-spi/src/main/java/org/apache/pinot/spi/trace/FilterType.java
new file mode 100644
index 0000000000..89eaee8611
--- /dev/null
+++ b/pinot-spi/src/main/java/org/apache/pinot/spi/trace/FilterType.java
@@ -0,0 +1,24 @@
+/**
+ * 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.pinot.spi.trace;
+
+public enum FilterType {
+  INDEX,
+  SCAN
+}
diff --git a/pinot-spi/src/main/java/org/apache/pinot/spi/trace/OperatorExecution.java b/pinot-spi/src/main/java/org/apache/pinot/spi/trace/OperatorExecution.java
new file mode 100644
index 0000000000..ec2eafd135
--- /dev/null
+++ b/pinot-spi/src/main/java/org/apache/pinot/spi/trace/OperatorExecution.java
@@ -0,0 +1,22 @@
+/**
+ * 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.pinot.spi.trace;
+
+public interface OperatorExecution extends Scope, ExecutionRecording {
+}
diff --git a/pinot-spi/src/main/java/org/apache/pinot/spi/trace/Phase.java b/pinot-spi/src/main/java/org/apache/pinot/spi/trace/Phase.java
new file mode 100644
index 0000000000..c7f3d6808c
--- /dev/null
+++ b/pinot-spi/src/main/java/org/apache/pinot/spi/trace/Phase.java
@@ -0,0 +1,27 @@
+/**
+ * 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.pinot.spi.trace;
+
+public enum Phase {
+  EXTRACT,
+  FILTER,
+  TRANSFORM,
+  PROJECT,
+  AGGREGATE
+}
diff --git a/pinot-spi/src/main/java/org/apache/pinot/spi/trace/Scope.java b/pinot-spi/src/main/java/org/apache/pinot/spi/trace/Scope.java
new file mode 100644
index 0000000000..4ff56d0b6c
--- /dev/null
+++ b/pinot-spi/src/main/java/org/apache/pinot/spi/trace/Scope.java
@@ -0,0 +1,25 @@
+/**
+ * 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.pinot.spi.trace;
+
+public interface Scope extends AutoCloseable {
+
+  @Override
+  void close();
+}
diff --git a/pinot-spi/src/main/java/org/apache/pinot/spi/trace/Tracer.java b/pinot-spi/src/main/java/org/apache/pinot/spi/trace/Tracer.java
new file mode 100644
index 0000000000..92a77b4985
--- /dev/null
+++ b/pinot-spi/src/main/java/org/apache/pinot/spi/trace/Tracer.java
@@ -0,0 +1,30 @@
+/**
+ * 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.pinot.spi.trace;
+
+public interface Tracer {
+
+    /**
+     * Registers the requestId on the current thread. This means the request will be traced.
+     * @param requestId the requestId
+     */
+    void register(long requestId);
+
+    OperatorExecution startOperatorExecution(Class<?> clazz);
+}
diff --git a/pinot-spi/src/main/java/org/apache/pinot/spi/trace/Tracing.java b/pinot-spi/src/main/java/org/apache/pinot/spi/trace/Tracing.java
new file mode 100644
index 0000000000..9ae8b30708
--- /dev/null
+++ b/pinot-spi/src/main/java/org/apache/pinot/spi/trace/Tracing.java
@@ -0,0 +1,70 @@
+/**
+ * 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.pinot.spi.trace;
+
+import java.lang.invoke.MethodHandles;
+import java.lang.invoke.MethodType;
+import java.util.concurrent.atomic.AtomicReference;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+
+public class Tracing {
+
+  private Tracing() {
+  }
+
+  private static final Logger LOGGER = LoggerFactory.getLogger(Tracing.class);
+
+  private static final AtomicReference<Tracer> REGISTRATION = new AtomicReference<>();
+
+  /**
+   * User once registration point to allow customization of tracing behaviour. Registration will be successful
+   * if this was the first attempt to register and registration happened before first use of the tracer.
+   * @param tracer the tracer implementation
+   * @return true if the registration was successful.
+   */
+  public static boolean register(Tracer tracer) {
+    return REGISTRATION.compareAndSet(null, tracer);
+  }
+
+  private static final class Holder {
+    static final Tracer TRACER = REGISTRATION.get() == null ? createDefaultTracer() : REGISTRATION.get();
+  }
+
+  /**
+   * @return the registered tracer.
+   */
+  public static Tracer getTracer() {
+    return Holder.TRACER;
+  }
+
+  private static Tracer createDefaultTracer() {
+    // create the default tracer via method handles if no override is registered
+    String defaultImplementationClassName = "org.apache.pinot.core.util.trace.DefaultTracer";
+    try {
+      Class<?> clazz = Class.forName(defaultImplementationClassName, false, Tracing.class.getClassLoader());
+      return (Tracer) MethodHandles.publicLookup()
+          .findConstructor(clazz, MethodType.methodType(void.class)).invoke();
+    } catch (Throwable missing) {
+      LOGGER.error("could not construct MethodHandle for {}", defaultImplementationClassName, missing);
+      return null;
+    }
+  }
+}


---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org