[jira] [Commented] (PHOENIX-6711) Add support of skipping the system tables existence check during connection initialisation and create new table result iterator which doesn't require fetch meta data of table

[ASF GitHub Bot (Jira)]

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

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

joshelser commented on code in PR #1437:
URL: https://github.com/apache/phoenix/pull/1437#discussion_r871649578


##########
phoenix-core/src/main/java/org/apache/phoenix/iterate/TableResultIterator.java:
##########
@@ -143,6 +143,20 @@ public TableResultIterator(MutationState mutationState, 
Scan scan, ScanMetricsHo
         ScanUtil.setScanAttributesForClient(scan, table, 
plan.getContext().getConnection());
     }
 
+    public TableResultIterator(MutationState mutationState, Scan scan, 
ScanMetricsHolder scanMetricsHolder,
+                               long renewLeaseThreshold, ParallelScanGrouper 
scanGrouper, byte[] tableName,
+                               boolean transactionalTable, boolean indexTable, 
boolean immutableRowsEnabled)
+            throws SQLException {
+        this.scan = scan;
+        this.plan = null;
+        this.scanMetricsHolder = scanMetricsHolder;
+        htable = mutationState.getHTable(tableName, transactionalTable, 
indexTable, immutableRowsEnabled);;

Review Comment:
   ```suggestion
           htable = mutationState.getHTable(tableName, transactionalTable, 
indexTable, immutableRowsEnabled);
   ```



##########
phoenix-core/src/main/java/org/apache/phoenix/iterate/TableResultIterator.java:
##########
@@ -179,6 +193,9 @@ public Tuple next() throws SQLException {
                 try {
                     throw ServerUtil.parseServerException(e);
                 } catch(HashJoinCacheNotFoundException e1) {
+                    if( plan == null) {
+                        throw e;

Review Comment:
   Need to have a `LOG.warn` message here indicating that, becuase we took the 
optimization to skip the system table lookup, we cannot do any recovery of the 
HashCache getting expired.
   
   This is a little worrisome as a batch job which runs for many hours (e.g. 
starvation on the YARN/Spark side) could result in the HashCache expiring. 
Normally, the code in this catch block would have then re-created the hashcache 
on the server and let the query continue.
   
   It's ok to make optimizations like this, but we need to make sure we clearly 
indicate when we are hitting some change in semantics as a result.



##########
phoenix-core/src/main/java/org/apache/phoenix/query/QueryServices.java:
##########
@@ -383,6 +383,11 @@ public interface QueryServices extends SQLCloseable {
      */
     String SOURCE_OPERATION_ATTRIB = "phoenix.source.operation";
 
+    /**
+     * Parameter to skip the system tables existence check to avoid 
unnecessary calls to Region server
+     * holding the SYSTEM.CATALOG table in batch oriented jobs.
+     */
+    String SKIP_SYSTEM_TABLES_EXISTENCE_CHECK = 
"phoenix.skip.system.tables.existence.check";

Review Comment:
   up in 
[phoenix-core/src/main/java/org/apache/phoenix/query/ConnectionQueryServicesImpl.java](https://github.com/apache/phoenix/pull/1437/files#diff-539e6818644fa837ebee4e4b61f4c570f75ac439437050c9a0459762ef8e385c)
 ?



##########
phoenix-core/src/main/java/org/apache/phoenix/iterate/TableResultIterator.java:
##########
@@ -143,6 +143,20 @@ public TableResultIterator(MutationState mutationState, 
Scan scan, ScanMetricsHo
         ScanUtil.setScanAttributesForClient(scan, table, 
plan.getContext().getConnection());
     }
 
+    public TableResultIterator(MutationState mutationState, Scan scan, 
ScanMetricsHolder scanMetricsHolder,

Review Comment:
   Would be good to consolidate with the existing constructors. OK to have 
multiple public constructors, but it would be better if they can be overloaded 
to reduce the amount of code duplication in the constructor bodies.





> Add support of skipping the system tables existence check during connection 
> initialisation and create new table result iterator which doesn't require 
> fetch meta data of table
> ------------------------------------------------------------------------------------------------------------------------------------------------------------------------------
>
>                 Key: PHOENIX-6711
>                 URL: https://issues.apache.org/jira/browse/PHOENIX-6711
>             Project: Phoenix
>          Issue Type: Sub-task
>            Reporter: Rajeshbabu Chintaguntla
>            Assignee: Rajeshbabu Chintaguntla
>            Priority: Major
>             Fix For: 5.1.2
>
>




--
This message was sent by Atlassian Jira
(v8.20.7#820007)