[ 
https://issues.apache.org/jira/browse/PHOENIX-5215?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17429838#comment-17429838
 ] 

ASF GitHub Bot commented on PHOENIX-5215:
-----------------------------------------

dbwong commented on a change in pull request #1282:
URL: https://github.com/apache/phoenix/pull/1282#discussion_r730601037



##########
File path: phoenix-core/src/it/java/org/apache/shell/TestFSShellCluster.java
##########
@@ -0,0 +1,40 @@
+package org.apache.shell;
+import org.apache.hadoop.conf.Configuration;
+import org.apache.hadoop.fs.FSDataInputStream;
+import org.apache.hadoop.fs.FSDataOutputStream;
+import org.apache.hadoop.fs.FileStatus;
+import org.apache.hadoop.fs.FileSystem;
+import org.apache.hadoop.fs.FsShell;
+import org.apache.hadoop.fs.LocatedFileStatus;
+import org.apache.hadoop.fs.Path;
+import org.apache.hadoop.fs.RemoteIterator;
+import org.apache.hadoop.fs.permission.FsPermission;
+import org.apache.hadoop.util.Progressable;
+
+import java.io.FileNotFoundException;
+import java.io.IOException;
+import java.net.URI;
+import java.nio.file.Paths;
+import java.util.List;
+
+public class TestFSShellCluster {

Review comment:
       Same with this class on testing and IT

##########
File path: 
phoenix-core/src/main/java/org/apache/phoenix/coprocessor/BaseScannerRegionObserver.java
##########
@@ -279,9 +281,7 @@ public void close() throws IOException {
                             try {
                                 delegate.close();
                             } finally {
-                                if (child != null) {
-                                    child.stop();
-                                }
+                                child.end();

Review comment:
       What did we change here that insure child is non-null? 

##########
File path: 
phoenix-core/src/main/java/org/apache/phoenix/iterate/ParallelIterators.java
##########
@@ -108,6 +108,7 @@ protected void submitWork(final List<List<Scan>> 
nestedScans, List<List<Pair<Sca
         context.getOverallQueryMetrics().updateNumParallelScans(numScans);
         GLOBAL_NUM_PARALLEL_SCANS.update(numScans);
         final long renewLeaseThreshold = 
context.getConnection().getQueryServices().getRenewLeaseThresholdMilliSeconds();
+        //TODO: Parent Span creation using Span.current() and create children 
with description  "Parallel scanner for table: " + 
tableRef.getTable().getPhysicalName().getString()

Review comment:
       Is this being handled in this JIRA or a followup?

##########
File path: 
phoenix-core/src/main/java/org/apache/phoenix/iterate/ParallelIterators.java
##########
@@ -118,10 +119,11 @@ protected void submitWork(final List<List<Scan>> 
nestedScans, List<List<Pair<Sca
                         mutationState, tableRef, scan, scanMetricsHolder, 
renewLeaseThreshold, plan,
                         scanGrouper, caches);
             context.getConnection().addIteratorForLeaseRenewal(tableResultItr);
-            Future<PeekingResultIterator> future = 
executor.submit(Tracing.wrap(new JobCallable<PeekingResultIterator>() {
+            Future<PeekingResultIterator> future = executor.submit(new 
JobCallable<PeekingResultIterator>() {
                 
                 @Override
                 public PeekingResultIterator call() throws Exception {
+                    //TODO: create child span for each call using parent

Review comment:
       This one we use to handle with Tracing.wrap right?

##########
File path: phoenix-core/src/main/java/org/apache/phoenix/trace/TraceReader.java
##########
@@ -47,6 +46,7 @@
 public class TraceReader {
 
     private static final Logger LOGGER = 
LoggerFactory.getLogger(TraceReader.class);
+    public static final long ROOT_SPAN_ID = 0x74ace;

Review comment:
       Is this the recommended approach for a top level span?

##########
File path: 
phoenix-core/src/it/java/org/apache/phoenix/end2end/LocalConnectionTest.java
##########
@@ -0,0 +1,80 @@
+package org.apache.phoenix.end2end;
+
+import io.opentelemetry.api.trace.Span;
+import io.opentelemetry.context.Scope;
+import org.apache.phoenix.trace.TraceUtil;
+
+import java.sql.*;
+
+public class LocalConnectionTest {

Review comment:
       Is this class assuming a local instance is running?  Can this be made an 
actual IT?

##########
File path: 
phoenix-core/src/it/java/org/apache/phoenix/end2end/LocalConnectionTest.java
##########
@@ -0,0 +1,80 @@
+package org.apache.phoenix.end2end;
+
+import io.opentelemetry.api.trace.Span;
+import io.opentelemetry.context.Scope;
+import org.apache.phoenix.trace.TraceUtil;
+
+import java.sql.*;

Review comment:
       nit: Phoenix style guidelines does not want to use import *




-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]


> Remove and replace HTrace
> -------------------------
>
>                 Key: PHOENIX-5215
>                 URL: https://issues.apache.org/jira/browse/PHOENIX-5215
>             Project: Phoenix
>          Issue Type: Bug
>            Reporter: Andrew Kyle Purtell
>            Assignee: Kiran Kumar Maturi
>            Priority: Major
>
> HTrace is dead.
> Hadoop is discussing a replacement of HTrace with OpenTracing, see 
> HADOOP-15566 
> HBase is having the same discussion on HBASE-22120



--
This message was sent by Atlassian Jira
(v8.3.4#803005)

Reply via email to