Repository: trafodion
Updated Branches:
  refs/heads/master 0551efb0b -> 58bd5757a


[TRAFODION-3009] Streamline error handling in Executor utility commands

get region stats command were populating the errors in the
queue entry directly, but still used handleErrors() to populate the
errors again in the HANDLE_ERROR_ state.  In some cases, the diagnostics
area wasn't populated too.

In CDH5.4, when the table is purged, the region info wasn't returned


Project: http://git-wip-us.apache.org/repos/asf/trafodion/repo
Commit: http://git-wip-us.apache.org/repos/asf/trafodion/commit/4297cac9
Tree: http://git-wip-us.apache.org/repos/asf/trafodion/tree/4297cac9
Diff: http://git-wip-us.apache.org/repos/asf/trafodion/diff/4297cac9

Branch: refs/heads/master
Commit: 4297cac937309138023d9dcc2612d6fcdc27c366
Parents: 6ca1d0f
Author: selvaganesang <selva.govindara...@esgyn.com>
Authored: Mon Apr 9 21:07:22 2018 +0000
Committer: selvaganesang <selva.govindara...@esgyn.com>
Committed: Mon Apr 9 21:31:43 2018 +0000

----------------------------------------------------------------------
 core/sql/bin/SqlciErrors.txt                    |  2 +-
 core/sql/executor/ExExeUtilGet.cpp              | 90 ++++----------------
 core/sql/executor/HBaseClient_JNI.cpp           | 25 ------
 core/sql/executor/HBaseClient_JNI.h             |  2 -
 core/sql/exp/ExpHbaseInterface.cpp              |  4 +-
 .../java/org/trafodion/sql/HBaseClient.java     | 47 +++++-----
 .../java/org/trafodion/sql/TrafRegionStats.java | 13 ++-
 7 files changed, 54 insertions(+), 129 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/trafodion/blob/4297cac9/core/sql/bin/SqlciErrors.txt
----------------------------------------------------------------------
diff --git a/core/sql/bin/SqlciErrors.txt b/core/sql/bin/SqlciErrors.txt
index cef2b8e..86f4c06 100644
--- a/core/sql/bin/SqlciErrors.txt
+++ b/core/sql/bin/SqlciErrors.txt
@@ -1587,7 +1587,7 @@ $1~String1 --------------------------------
 8448 ZZZZZ 99999 BEGINNER MAJOR DBADMIN Unable to access Hbase interface. Call 
to $0~string0 returned error $1~string1($0~int0). Cause: $2~string2.
 8449 ZZZZZ 99999 BEGINNER MAJOR DBADMIN An invalid HBase table option or 
option value was specified: ($0~string0 = $1~string1)
 8450 ZZZZZ 99999 BEGINNER MINOR LOGONLY ESP number ($0~Int0) has processed 
($1~Int1) transactions of the LRU operation on table $2~String0.
-8451 ZZZZZ 99999 BEGINNER MINOR LOGONLY Error returned while retrieving region 
stats from hbase.
+8451 ZZZZZ 99999 BEGINNER MINOR LOGONLY Error $0~String0  returned while 
retrieving region stats from hbase.
 8452 ZZZZZ 99999 BEGINNER MINOR LOGONLY The regular expression is invalid. 
Cause: $0~String0
 8550 ZZZZZ 99999 ADVANCED MAJOR DBADMIN Error $0~NSKCode was returned by the 
Data Access Manager.
 8551 ZZZZZ 99999 ADVANCED MAJOR DBADMIN Error $0~NSKCode was returned by the 
file system on $0~string0$1~string1.

http://git-wip-us.apache.org/repos/asf/trafodion/blob/4297cac9/core/sql/executor/ExExeUtilGet.cpp
----------------------------------------------------------------------
diff --git a/core/sql/executor/ExExeUtilGet.cpp 
b/core/sql/executor/ExExeUtilGet.cpp
index bf938c3..10dd05f 100644
--- a/core/sql/executor/ExExeUtilGet.cpp
+++ b/core/sql/executor/ExExeUtilGet.cpp
@@ -2583,15 +2583,9 @@ short ExExeUtilGetMetadataInfoTcb::work()
 
              default:
                {
-                 ExHandleErrors(qparent_,
-                                pentry_down,
-                                0,
-                                getGlobals(),
-                                NULL,
-                                (ExeErrorCode)-4218,
-                                NULL,
-                                "GET"
-                                );
+                  ExRaiseSqlError(getHeap(), &diagsArea_, -4218, 
+                       NULL, NULL, NULL,
+                       "GET");
                  step_ = HANDLE_ERROR_;
                }
                break;
@@ -3116,15 +3110,8 @@ short ExExeUtilGetMetadataInfoComplexTcb::work()
 
              default:
                {
-                 ExHandleErrors(qparent_,
-                                pentry_down,
-                                0,
-                                getGlobals(),
-                                NULL,
-                                (ExeErrorCode)-4218,
-                                NULL,
-                                "GET"
-                                );
+                  ExRaiseSqlError(getHeap(), &diagsArea_, -4298, 
+                            NULL, NULL, NULL, "GET");
                  step_ = HANDLE_ERROR_;
                }
              break;
@@ -3930,15 +3917,8 @@ short ExExeUtilGetMetadataInfoVersionTcb::work()
 
                  default:
                    {
-                     ExHandleErrors(qparent_,
-                                    pentry_down,
-                                    0,
-                                    getGlobals(),
-                                    NULL,
-                                    (ExeErrorCode)-4218,
-                                    NULL,
-                                    "GET"
-                                    );
+                      ExRaiseSqlError(getHeap(), &diagsArea_, -4218, 
+                         NULL, NULL, NULL, "GET");
                      step_ = HANDLE_ERROR_;
                    }
                  break;
@@ -4580,26 +4560,7 @@ short ExExeUtilGetUIDTcb::work()
            step_ = DONE_;
          }
        break;
-
-       case ERROR_:
-         {
-           if (qparent_.up->isFull())
-             return WORK_OK;
-
-           ExHandleErrors(qparent_,
-                          pentry_down,
-                          0,
-                          getGlobals(),
-                          NULL,
-                          (ExeErrorCode)cliRC,
-                          NULL,
-                          NULL
-                          );
-           step_ = DONE_;
-         }
-       break;
-
-       case DONE_:
+      case DONE_:
          {
            if (qparent_.up->isFull())
              return WORK_OK;
@@ -4748,15 +4709,7 @@ short ExExeUtilGetQIDTcb::work()
             /* stmt must exist */
             if (!stmt)
               {
-                ExHandleErrors(qparent_,
-                               pentry_down,
-                               0,
-                               getGlobals(),
-                               NULL,
-                               (ExeErrorCode)-CLI_STMT_NOT_EXISTS,
-                               NULL,
-                               NULL
-                               );
+                ExRaiseSqlError(getHeap(), &diagsArea_, -CLI_STMT_NOT_EXISTS);
                 step_ = ERROR_;
                 break;
               }
@@ -4946,24 +4899,6 @@ short ExExeUtilGetErrorInfoTcb::work()
          }
          break;
 
-       case ERROR_:
-         {
-           if (qparent_.up->isFull())
-             return WORK_OK;
-
-           ExHandleErrors(qparent_,
-                          pentry_down,
-                          0,
-                          getGlobals(),
-                          NULL,
-                          (ExeErrorCode)cliRC,
-                          NULL,
-                          NULL
-                          );
-           step_ = DONE_;
-         }
-       break;
-
        case DONE_:
          {
            if (qparent_.up->isFull())
@@ -6353,6 +6288,9 @@ short ExExeUtilRegionStatsTcb::work()
           {
             if (collectStats(tableName_))
               {
+                ExRaiseSqlError(getHeap(), &diagsArea_, -8451,
+                     NULL, NULL, NULL,
+                     getSqlJniErrorStr());
                 step_ = HANDLE_ERROR_;
                 break;
               }
@@ -7097,7 +7035,9 @@ short ExExeUtilClusterStatsTcb::work()
               }
             else if (retcode < 0)
               {
-                ExRaiseSqlError(getHeap(), &diagsArea_, -8451);
+                ExRaiseSqlError(getHeap(), &diagsArea_, -8451,
+                     NULL, NULL, NULL,
+                     getSqlJniErrorStr());
                 step_ = HANDLE_ERROR_;
                 break;
               }

http://git-wip-us.apache.org/repos/asf/trafodion/blob/4297cac9/core/sql/executor/HBaseClient_JNI.cpp
----------------------------------------------------------------------
diff --git a/core/sql/executor/HBaseClient_JNI.cpp 
b/core/sql/executor/HBaseClient_JNI.cpp
index 178043b..8726fb8 100644
--- a/core/sql/executor/HBaseClient_JNI.cpp
+++ b/core/sql/executor/HBaseClient_JNI.cpp
@@ -235,8 +235,6 @@ HBC_RetCode HBaseClient_JNI::init()
     JavaMethods_[JM_LIST_ALL       ].jm_signature = "(Ljava/lang/String;)[[B";
     JavaMethods_[JM_GET_REGION_STATS       ].jm_name      = "getRegionStats";
     JavaMethods_[JM_GET_REGION_STATS       ].jm_signature = 
"(Ljava/lang/String;)[[B";
-    JavaMethods_[JM_GET_REGION_STATS_ENTRIES       ].jm_name      = 
"getRegionStatsEntries";
-    JavaMethods_[JM_GET_REGION_STATS_ENTRIES       ].jm_signature = "()I";
     JavaMethods_[JM_COPY       ].jm_name      = "copy";
     JavaMethods_[JM_COPY       ].jm_signature = 
"(Ljava/lang/String;Ljava/lang/String;Z)Z";
     JavaMethods_[JM_EXISTS     ].jm_name      = "exists";
@@ -1058,29 +1056,6 @@ NAArray<HbaseStr>* HBaseClient_JNI::listAll(NAHeap 
*heap, const char* pattern)
      return hbaseTables;
 }
 
-Int32 HBaseClient_JNI::getRegionStatsEntries()
-{
-  QRLogger::log(CAT_SQL_HBASE, LL_DEBUG, 
"HBaseClient_JNI::getRegionStatsEntries() called.");
-
-  if (initJNIEnv() != JOI_OK)
-     return 0;
-
-  tsRecentJMFromJNI = JavaMethods_[JM_GET_REGION_STATS_ENTRIES].jm_full_name;
-  jint numEntries = 
-    (jint)jenv_->CallIntMethod(javaObj_, 
JavaMethods_[JM_GET_REGION_STATS_ENTRIES].methodID);
-
-  if (jenv_->ExceptionCheck())
-  {
-    getExceptionDetails(__FILE__, __LINE__, 
"HBaseClient_JNI::getRegionStatsEntries()");
-    jenv_->PopLocalFrame(NULL);
-    return 0;
-  }
-
-  jenv_->PopLocalFrame(NULL);
-
-  return numEntries;
-}
-
 //////////////////////////////////////////////////////////////////////////////
 // 
 //////////////////////////////////////////////////////////////////////////////

http://git-wip-us.apache.org/repos/asf/trafodion/blob/4297cac9/core/sql/executor/HBaseClient_JNI.h
----------------------------------------------------------------------
diff --git a/core/sql/executor/HBaseClient_JNI.h 
b/core/sql/executor/HBaseClient_JNI.h
index 4b869c6..c89146a 100644
--- a/core/sql/executor/HBaseClient_JNI.h
+++ b/core/sql/executor/HBaseClient_JNI.h
@@ -460,7 +460,6 @@ public:
                    NABoolean force);
   NAArray<HbaseStr>* listAll(NAHeap *heap, const char* pattern);
   NAArray<HbaseStr>* getRegionStats(NAHeap *heap, const char* tblName);
-  Int32 getRegionStatsEntries();
 
   HBC_RetCode exists(const char* fileName, Int64 transID);
   HBC_RetCode grant(const Text& user, const Text& tableName, const TextVec& 
actionCodes); 
@@ -556,7 +555,6 @@ private:
    ,JM_DROP_ALL
    ,JM_LIST_ALL
    ,JM_GET_REGION_STATS
-   ,JM_GET_REGION_STATS_ENTRIES
    ,JM_COPY
    ,JM_EXISTS
    ,JM_GRANT

http://git-wip-us.apache.org/repos/asf/trafodion/blob/4297cac9/core/sql/exp/ExpHbaseInterface.cpp
----------------------------------------------------------------------
diff --git a/core/sql/exp/ExpHbaseInterface.cpp 
b/core/sql/exp/ExpHbaseInterface.cpp
index 8a1d426..b824a9d 100644
--- a/core/sql/exp/ExpHbaseInterface.cpp
+++ b/core/sql/exp/ExpHbaseInterface.cpp
@@ -1516,8 +1516,6 @@ NAArray<HbaseStr> * 
ExpHbaseInterface_JNI::getRegionStats(const HbaseStr& tblNam
   if (regionStats == NULL)
     return NULL;
 
-  Int32 numEntries = client_->getRegionStatsEntries();
-
   return regionStats;
 }
 
@@ -1534,7 +1532,7 @@ NAArray<HbaseStr> * 
ExpHbaseInterface_JNI::getClusterStats(Int32 &numEntries)
   if (regionStats == NULL)
     return NULL;
   
-  numEntries = client_->getRegionStatsEntries();
+  numEntries = regionStats->entries();
 
   return regionStats;
 }

http://git-wip-us.apache.org/repos/asf/trafodion/blob/4297cac9/core/sql/src/main/java/org/trafodion/sql/HBaseClient.java
----------------------------------------------------------------------
diff --git a/core/sql/src/main/java/org/trafodion/sql/HBaseClient.java 
b/core/sql/src/main/java/org/trafodion/sql/HBaseClient.java
index a00e1c8..e1dce7b 100644
--- a/core/sql/src/main/java/org/trafodion/sql/HBaseClient.java
+++ b/core/sql/src/main/java/org/trafodion/sql/HBaseClient.java
@@ -825,11 +825,11 @@ public class HBaseClient {
                         
                         int  numStores           = regionSizeInfo.numStores;
                         int  numStoreFiles       = 
regionSizeInfo.numStoreFiles;
-                        Long storeUncompSize     = 
regionSizeInfo.storeUncompSize;
-                        Long storeFileSize       = 
regionSizeInfo.storeFileSize;
-                        Long memStoreSize        = regionSizeInfo.memStoreSize;
-                        Long readRequestsCount   = 
regionSizeInfo.readRequestsCount;
-                        Long writeRequestsCount   = 
regionSizeInfo.writeRequestsCount;
+                        long storeUncompSize     = 
regionSizeInfo.storeUncompSize;
+                        long storeFileSize       = 
regionSizeInfo.storeFileSize;
+                        long memStoreSize        = regionSizeInfo.memStoreSize;
+                        long readRequestsCount   = 
regionSizeInfo.readRequestsCount;
+                        long writeRequestsCount   = 
regionSizeInfo.writeRequestsCount;
                         
                         String oneRegion = "";
                         oneRegion += serverName + "|";
@@ -852,12 +852,6 @@ public class HBaseClient {
 
     }
 
-    // number of regionInfo entries returned by getRegionStats.
-    public int getRegionStatsEntries() {
- 
-        return regionStatsEntries;
-    }
-
     public byte[][]  getRegionStats(String tableName) 
              throws MasterNotRunningException, IOException {
             if (logger.isDebugEnabled()) 
logger.debug("HBaseClient.getRegionStats(" + tableName + ") called.");
@@ -882,21 +876,30 @@ public class HBaseClient {
                 
                     hregInfo = entry.getKey();                    
                     ServerName serverName = entry.getValue();
-                     byte[] regionName = hregInfo.getRegionName();
+                    byte[] regionName = hregInfo.getRegionName();
                     String encodedRegionName = hregInfo.getEncodedName();
                     String ppRegionName = 
HRegionInfo.prettyPrint(encodedRegionName);
                     SizeInfo regionSizeInfo  = 
rsc.getRegionSizeInfo(regionName);
                     String serverNameStr     = regionSizeInfo.serverName;
-                    int  numStores           = regionSizeInfo.numStores;
-                    int  numStoreFiles       = regionSizeInfo.numStoreFiles;
-                    Long storeUncompSize     = regionSizeInfo.storeUncompSize;
-                    Long storeFileSize       = regionSizeInfo.storeFileSize;
-                    Long memStoreSize        = regionSizeInfo.memStoreSize;
-                    Long readRequestsCount   = 
regionSizeInfo.readRequestsCount;
-                    Long writeRequestsCount  = 
regionSizeInfo.writeRequestsCount;
-
-                    String ppTableName = regionSizeInfo.tableName;
-                    ppRegionName = regionSizeInfo.regionName;
+                    int  numStores           = 0;
+                    int  numStoreFiles       = 0;
+                    long storeUncompSize     = 0;
+                    long storeFileSize       = 0;
+                    long memStoreSize        = 0;
+                    long readRequestsCount   = 0;
+                    long writeRequestsCount  = 0;
+                    String ppTableName = "";
+                    if (regionSizeInfo != null) {
+                       numStores           = regionSizeInfo.numStores;
+                       numStoreFiles       = regionSizeInfo.numStoreFiles;
+                       storeUncompSize     = regionSizeInfo.storeUncompSize;
+                       storeFileSize       = regionSizeInfo.storeFileSize;
+                       memStoreSize        = regionSizeInfo.memStoreSize;
+                       readRequestsCount   = regionSizeInfo.readRequestsCount;
+                       writeRequestsCount  = regionSizeInfo.writeRequestsCount;
+                       ppTableName = regionSizeInfo.tableName;
+                       ppRegionName = regionSizeInfo.regionName;
+                    }
                     String oneRegion;
                     oneRegion = serverNameStr + "|";
                     oneRegion += ppTableName + "/" + ppRegionName + "|";

http://git-wip-us.apache.org/repos/asf/trafodion/blob/4297cac9/core/sql/src/main/java/org/trafodion/sql/TrafRegionStats.java
----------------------------------------------------------------------
diff --git a/core/sql/src/main/java/org/trafodion/sql/TrafRegionStats.java 
b/core/sql/src/main/java/org/trafodion/sql/TrafRegionStats.java
index aabdab2..db64663 100644
--- a/core/sql/src/main/java/org/trafodion/sql/TrafRegionStats.java
+++ b/core/sql/src/main/java/org/trafodion/sql/TrafRegionStats.java
@@ -33,6 +33,8 @@ import org.apache.hadoop.hbase.client.Admin;
 import org.apache.hadoop.hbase.client.HTable;
 import org.apache.hadoop.hbase.client.HBaseAdmin;
 import org.apache.hadoop.hbase.client.Connection;
+import org.apache.log4j.PropertyConfigurator;
+import org.apache.log4j.Logger;
 
 import java.io.IOException;
 import java.util.Arrays;
@@ -62,6 +64,7 @@ class SizeInfo {
 
 public class TrafRegionStats {
 
+    static Logger logger = Logger.getLogger(TrafRegionStats.class.getName());
     private Admin hbAdmin;
     private ClusterStatus clusterStatus;
     private Collection<ServerName> servers;
@@ -78,6 +81,14 @@ public class TrafRegionStats {
 
     private SizeInfo currRegionSizeInfo = null;
 
+    static {
+       String confFile = System.getProperty("trafodion.log4j.configFile");
+        System.setProperty("trafodion.root", System.getenv("TRAF_HOME"));
+       if (confFile == null) 
+           confFile = System.getenv("TRAF_CONF") + "/log4j.sql.config";
+       PropertyConfigurator.configure(confFile);
+    }
+
     static final String ENABLE_REGIONSIZECALCULATOR = 
"hbase.regionsizecalculator.enable";
     
     boolean enabled(Configuration configuration) {
@@ -101,7 +112,7 @@ public class TrafRegionStats {
     public TrafRegionStats (HTable table, Admin admin) throws IOException {
         
             if (!enabled(table.getConfiguration())) {
-                System.out.println("Region size calculation disabled.");
+                logger.error("Region size calculation disabled for table " + 
table.getTableName());
                 return;
             }
             

Reply via email to