[GitHub] incubator-trafodion pull request #1234: [TRAFODION-2746] Fixed various probl...

[zcorrea]

GitHub user zcorrea opened a pull request:

https://github.com/apache/incubator-trafodion/pull/1234

[TRAFODION-2746] Fixed various problem detected in large clusters (> 30)

The problems were:

1. A segmentation violation occurred during the Integration phase, when the 
new 
   monitor is establishing the socket communication paths between itself 
and 
   the existing monitors.
   a. Information is exchanged between the master (creator) monitor and the
  slave (new) monitor process which tells the new monitor which nodes
  monitor process make up the existing cluster instance. During these
  exchanges, in CCluster::ReceiveSock() one of the messages was large
  enough to require chunking and the logic which kept track of the
  number of bytes received was not calculated correctly which resulted
  in an overwrite past the boundary of the receive buffer. 
2. A second segmentation violation was due to a buffer overwrite during the
   Joining (revive) phase.
   a. In requeue.cxx, when creating the buffer in the master (creator) 
monitor
  which is populated with the cluster state information to be sent to 
the
  slave (new) monitor process, the calculation did not properly account 
for
  the number of logical and physical nodes. So that when the buffer was
  populated, it would overwrite past the allocated buffer.
3. A third problem was also note in the one of the monitor would remain in
   the Joining state and never come out of it.
   a. The problem was in the order of logic when calling 
  CCluster::ResetIntegratingPNid() which triggers the
  CCommAccept::commAcceptorSock() to accept another new node to
  integrate. The invocation to ResetIntegratingPNid() was done before
  resetting the creator flag. Due to kernel scheduling, this resetting
  of the creator flag was happening after another monitor started the
  Integration phase and it was breaking the node integration protocol
  by terminating it too early. So the new monitor would stay in the
  Joining state for ever since the protocol was broken.
4. The last segmentation violation was due to stderr buffer overwrite in
   CRedirectStderr::handleOutput() where the size returned by snprintf() 
   was used to terminate the buffer containing stderr data >= 4096 which
   is the size of the buffer.


You can merge this pull request into a Git repository by running:

$ git pull https://github.com/zcorrea/incubator-trafodion TRAFODION-2746

Alternatively you can review and apply these changes as the patch at:

https://github.com/apache/incubator-trafodion/pull/1234.patch

To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:

This closes #1234


commit 19555630d5c0d63e8a8ea1e02f92545da983cb35
Author: Zalo Correa 
Date:   2017-09-16T00:02:48Z

[TRAFODION-2746] Fixed various problem detected in large clusters (> 30)




---

[GitHub] incubator-trafodion pull request #1232: [TRAFODION-2738] Rowset buffer size ...

[selvaganesang]

Github user selvaganesang commented on a diff in the pull request:


https://github.com/apache/incubator-trafodion/pull/1232#discussion_r139270056
  
--- Diff: core/sql/sqlcomp/nadefaults.cpp ---
@@ -1336,6 +1336,8 @@ SDDkwd__(EXE_DIAGNOSTIC_EVENTS,   "OFF"),
 
  SDDui___(EXE_MEMORY_FOR_PROBE_CACHE_IN_MB,"100"),
 
+ SDDui___(EXE_MEMORY_FOR_UNPACK_ROWS_IN_MB,"100"),
--- End diff --

Because there is an easy way to change the memory size, we can determine 
the optimal default at a later date.  I believe the current default of 100 MB 
should be good enough for most of the use cases.


---

[GitHub] incubator-trafodion pull request #1233: [TRAFODION-2651] Fixed monitor-to-mo...

[zcorrea]

GitHub user zcorrea opened a pull request:

https://github.com/apache/incubator-trafodion/pull/1233

[TRAFODION-2651] Fixed monitor-to-monitor communication by adding

reconnect logic to better handle various communication glitches which 
previously
resulted in false node down situations.

You can merge this pull request into a Git repository by running:

$ git pull https://github.com/zcorrea/incubator-trafodion TRAFODION-2651

Alternatively you can review and apply these changes as the patch at:

https://github.com/apache/incubator-trafodion/pull/1233.patch

To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:

This closes #1233


commit 5e09a1e3a41d81394a0beee7eb67f7475a344daf
Author: Zalo Correa 
Date:   2017-09-15T22:39:06Z

[TRAFODION-2651] Fixed monitor-to-monitor communication by adding reconnect
logic to better handle various communication glitches which previously
resulted in false node down situations.




---

[GitHub] incubator-trafodion pull request #1232: [TRAFODION-2738] Rowset buffer size ...

[sureshsubbiah]

Github user sureshsubbiah commented on a diff in the pull request:


https://github.com/apache/incubator-trafodion/pull/1232#discussion_r139228319
  
--- Diff: core/sql/sqlcomp/nadefaults.cpp ---
@@ -1336,6 +1336,8 @@ SDDkwd__(EXE_DIAGNOSTIC_EVENTS,   "OFF"),
 
  SDDui___(EXE_MEMORY_FOR_PROBE_CACHE_IN_MB,"100"),
 
+ SDDui___(EXE_MEMORY_FOR_UNPACK_ROWS_IN_MB,"100"),
--- End diff --

A default value of 100 MB maybe too small. I understand that this is good 
for mixed workloads, but do consider than plans with Unpack, especially when 
used for insert/upsert are simple. Unpack is always serial and part of master 
exe. Often there is only one in a query.  The cost of having a low value here 
seems to be that queue length from unpack could be limited by this when we are 
loading a table with wide rows (even if actual data in row is not wide). We 
know from performance runs that queue length is critical to upsert load 
performance. This is a hunch on my part and could be misguided. It will be good 
to performance test for upsert of table with wide rows, if you think that is 
worthwhile.


---

[GitHub] incubator-trafodion pull request #1225: [TRAFODION-2704]close unused statmen...

[sandhyasun]

Github user sandhyasun commented on a diff in the pull request:


https://github.com/apache/incubator-trafodion/pull/1225#discussion_r139204340
  
--- Diff: core/conn/odbc/src/odbc/nsksrvrcore/srvrothers.cpp ---
@@ -6489,6 +6506,31 @@ odbc_SQLSrvr_ExtractLob_sme_(
 exception_->u.SQLError.errorList._buffer = 
QryLobExtractSrvrStmt->sqlError.errorList._buffer;
 exception_->u.ParamError.ParamDesc = 
SQLSVC_EXCEPTION_EXECUTE_FAILED;
 }
+
+IDL_long_long zeroValue = 0;
+snprintf(LobExtractQuery, sizeof(LobExtractQuery), "EXTRACT 
LOBTOBUFFER(LOB'%s', LOCATION %Ld, SIZE %Ld)", lobHandle, (Int64)lobDataValue, 
);
+
+retcode = QryLobExtractSrvrStmt->ExecDirect(NULL, 
LobExtractQuery, EXTERNAL_STMT, TYPE_CALL, SQL_ASYNC_ENABLE_OFF, 0);
+
--- End diff --

Ok if this will be reworked as part of TRAFODION-2740, we can keep this as 
temp code to close the statement - although prematurely. Just make sure there 
is a comment in the code saying this will be removed when TRAFODION-2740 is 
addressed.  


---

[GitHub] incubator-trafodion pull request #1220: [TRAFODION-2725] SQL types are REAL,...

[selvaganesang]

Github user selvaganesang commented on a diff in the pull request:


https://github.com/apache/incubator-trafodion/pull/1220#discussion_r139188923
  
--- Diff: win-odbc64/odbcclient/drvr35/drvrglobal.cpp ---
@@ -865,77 +865,18 @@ bool use_gcvt(double number, char* string, short size)
 
 bool double_to_char (double number, int precision, char* string, short 
size)
 {
-   char *buffer,*temp ;
-   bool rc = true;
-
-   int decimal_spot,
-   sign,
-   count,
-   current_location = 0,
-   length;
-
-   *string = 0;
-
-   temp = _fcvt (number, precision, _spot, ) ;
-   length = strlen(temp);
-   if (length == 0)
-   {
-   return use_gcvt(number,string,size);
-   }
-   if (length > precision)
-   buffer = (char *) malloc (length + 3) ;
-   else
-   buffer = (char *) malloc (precision + 3) ;
-
-   if (buffer == NULL)
-   return false;
-
-/* Add negative sign if required. */ 
-
-   if (sign)
-   buffer [current_location++] = '-' ;
-
-/* Place decimal point in the correct location. */ 
-
-   if (decimal_spot > 0)
-   {
-   strncpy ( [current_location], temp, decimal_spot) ;
-   buffer [decimal_spot + current_location] = '.' ;
-   strcpy ( [decimal_spot + current_location + 1],
-[decimal_spot]) ;
-   }
-   else
-   {
-   buffer [current_location] = '.' ;
-   for(count = current_location;
-   count< abs(decimal_spot)+current_location; count++)
-   buffer [count + 1] = '0' ;
-   strcpy ( [count + 1], temp) ;
-   }
+bool rc = false;
+char format[16];
+char buf[MAX_DOUBLE_TO_CHAR_LEN];
 
-   rSup(buffer);
-   length = strlen(buffer);
-   if (buffer[0] == '.' || (buffer[0] == '-' && buffer[1] == '.')) 
length++;
+sprintf(format, "%%.%dlg", precision);
+sprintf(buf, format, number);
--- End diff --

There is a risk of writing out of range because precision is not checked to 
be within the buf size. Also, you need to allow the numbers to be truncated 
after a decimal even when there is no sufficient length passed by the caller.  
So  size can be less than strlen(buf) as long as the fraction part alone is 
getting cut. 


---

[GitHub] incubator-trafodion pull request #1229: [Trafodion-2734] Create *Trafodion L...

[liuyu000]

Github user liuyu000 commented on a diff in the pull request:


https://github.com/apache/incubator-trafodion/pull/1229#discussion_r139076734
  
--- Diff: docs/lob_guide/src/asciidoc/_chapters/work_with_lob.adoc ---
@@ -0,0 +1,738 @@
+
+/**
+* @@@ START COPYRIGHT @@@
+*
+* 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.
+*
+* @@@ END COPYRIGHT @@@
+*/
+
+
+[#work with lob]
+= Work with LOB
+
+[#create a sql table with lob columns]
+== Create a SQL Table with LOB Columns
+
+When creating a SQL table with LOB columns, following relevant tables and 
files are created as well:
+
+* One LOB MD table.
+* Two dependent descriptor tables.
+* HDFS data file (locates at /user/trafodion/lobs) for each column.
+
+[#syntax]
+== Syntax
+
+```
+CREATE TABLE table-name (lob-column-spec[, lob-column-spec]…)
+```
+
+```
+lob-column-spec is:
+column {lob type}
+
+lob type is:
+BLOB | CLOB [({numeric literal} [unit])] [STORAGE 'storage literal']
+
+unit is:
+empty | 
+K | 
+M | 
+G 
+```
+
+[#semantics]
+=== Semantics
+
+* `_storage literal_`
+
++
+Currently Trafodion only support `'EXTERNAL'` here. 
+
++
+External LOB object that are not managed by Trafodion.
+
+* `_empty_`
+
++
+Number of bytes specified by the numeric literal.
+
+* `_K_`
+
++
+Numeric literal value * 1024.
+
+* `_M_`
+
++
+Numeric literal value * 1024 * 1024.
+
+* `_G_`
+
++
+Numeric literal value * 1024 * 1024 * 1024.
+
+[#examples]
+=== Examples
+
+* This example creates a table tlob1 with 2 columns and primary key on the 
c1.
+
++
+
+```
+CREATE TABLE tlob1 (c1 INT NOT NULL, c2 BLOB, PRIMARY KEY (c1));
+```
+
+* This example creates a table tlob2 with 3 columns and primary key on the 
c1.
+
++
+
+```
+CREATE TABLE tlob2 (c1 INT NOT NULL, c2 BLOB, c3 CLOB, PRIMARY KEY (c1));
+```
+
+* This example creates a table tlob130txt_limit50 with 2 columns and 
primary key on the c1.
+
++
+
+```
+CREATE TABLE tlob130txt_limit50 (c1 INT NOT NULL, c2 CLOB(50), PRIMARY KEY 
(c1));
+```
+
+* This example creates a table tlob130bin_limit1K with 2 columns and 
primary key on the c1.
+
++
+
+```
+CREATE TABLE tlob130bin_limit1K (c1 INT NOT NULL, c2 BLOB(1 K), PRIMARY 
KEY (c1));
+```
+
+* This example creates a table tlob130ext with 4 columns and primary key 
on the c1.
+
++
+
+```
+CREATE TABLE tlob130ext (c1 INT NOT NULL, c2 BLOB, c3 CLOB, c4 BLOB 
STORAGE 'EXTERNAL', PRIMARY KEY (c1));
+```
+
+[#hdfs location of lob data]
+=== HDFS Location of LOB Data
+
+When a LOB table is created, the underlying LOB data needs to be stored in 
HDFS.It is in the /user/trafodion/lobs by default. 
+
+All columns of a table that are declared as LOB types will have all their 
data in one file derived from the Object UID and the LOB number of that column 
which gets assigned during creation.
+
+The following is a LOB file with 2 columns you will see 2 files in HDFS:
+
+/user/trafodion/lobs /LOBP_03683514167332904796_0001
--- End diff --

Thanks Dave, for your eagle eyes.
Yes, I've corrected. :)


---

[GitHub] incubator-trafodion pull request #1229: [Trafodion-2734] Create *Trafodion L...

[liuyu000]

Github user liuyu000 commented on a diff in the pull request:


https://github.com/apache/incubator-trafodion/pull/1229#discussion_r139076558
  
--- Diff: docs/lob_guide/src/asciidoc/_chapters/work_with_lob.adoc ---
@@ -0,0 +1,738 @@
+
+/**
+* @@@ START COPYRIGHT @@@
+*
+* 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.
+*
+* @@@ END COPYRIGHT @@@
+*/
+
+
+[#work with lob]
+= Work with LOB
+
+[#create a sql table with lob columns]
+== Create a SQL Table with LOB Columns
+
+When creating a SQL table with LOB columns, following relevant tables and 
files are created as well:
+
+* One LOB MD table.
+* Two dependent descriptor tables.
+* HDFS data file (locates at /user/trafodion/lobs) for each column.
+
+[#syntax]
+== Syntax
+
+```
+CREATE TABLE table-name (lob-column-spec[, lob-column-spec]…)
+```
+
+```
+lob-column-spec is:
+column {lob type}
+
+lob type is:
+BLOB | CLOB [({numeric literal} [unit])] [STORAGE 'storage literal']
+
+unit is:
+empty | 
+K | 
+M | 
+G 
+```
+
+[#semantics]
+=== Semantics
+
+* `_storage literal_`
+
++
+Currently Trafodion only support `'EXTERNAL'` here. 
+
++
+External LOB object that are not managed by Trafodion.
+
+* `_empty_`
+
++
+Number of bytes specified by the numeric literal.
+
+* `_K_`
+
++
+Numeric literal value * 1024.
+
+* `_M_`
+
++
+Numeric literal value * 1024 * 1024.
+
+* `_G_`
+
++
+Numeric literal value * 1024 * 1024 * 1024.
+
+[#examples]
+=== Examples
+
+* This example creates a table tlob1 with 2 columns and primary key on the 
c1.
+
++
+
+```
+CREATE TABLE tlob1 (c1 INT NOT NULL, c2 BLOB, PRIMARY KEY (c1));
+```
+
+* This example creates a table tlob2 with 3 columns and primary key on the 
c1.
+
++
+
+```
+CREATE TABLE tlob2 (c1 INT NOT NULL, c2 BLOB, c3 CLOB, PRIMARY KEY (c1));
+```
+
+* This example creates a table tlob130txt_limit50 with 2 columns and 
primary key on the c1.
+
++
+
+```
+CREATE TABLE tlob130txt_limit50 (c1 INT NOT NULL, c2 CLOB(50), PRIMARY KEY 
(c1));
+```
+
+* This example creates a table tlob130bin_limit1K with 2 columns and 
primary key on the c1.
+
++
+
+```
+CREATE TABLE tlob130bin_limit1K (c1 INT NOT NULL, c2 BLOB(1 K), PRIMARY 
KEY (c1));
+```
+
+* This example creates a table tlob130ext with 4 columns and primary key 
on the c1.
+
++
+
+```
+CREATE TABLE tlob130ext (c1 INT NOT NULL, c2 BLOB, c3 CLOB, c4 BLOB 
STORAGE 'EXTERNAL', PRIMARY KEY (c1));
+```
+
+[#hdfs location of lob data]
+=== HDFS Location of LOB Data
+
+When a LOB table is created, the underlying LOB data needs to be stored in 
HDFS.It is in the /user/trafodion/lobs by default. 
+
+All columns of a table that are declared as LOB types will have all their 
data in one file derived from the Object UID and the LOB number of that column 
which gets assigned during creation.
--- End diff --

Thanks Dave.
Yes, I've corrected. :)


---

[GitHub] incubator-trafodion pull request #1229: [Trafodion-2734] Create *Trafodion L...

[liuyu000]

Github user liuyu000 commented on a diff in the pull request:


https://github.com/apache/incubator-trafodion/pull/1229#discussion_r139075962
  
--- Diff: docs/lob_guide/src/asciidoc/_chapters/work_with_lob.adoc ---
@@ -0,0 +1,738 @@
+
+/**
+* @@@ START COPYRIGHT @@@
+*
+* 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.
+*
+* @@@ END COPYRIGHT @@@
+*/
+
+
+[#work with lob]
+= Work with LOB
+
+[#create a sql table with lob columns]
+== Create a SQL Table with LOB Columns
+
+When creating a SQL table with LOB columns, following relevant tables and 
files are created as well:
+
+* One LOB MD table.
+* Two dependent descriptor tables.
+* HDFS data file (locates at /user/trafodion/lobs) for each column.
+
+[#syntax]
+== Syntax
+
+```
+CREATE TABLE table-name (lob-column-spec[, lob-column-spec]…)
+```
+
+```
+lob-column-spec is:
+column {lob type}
+
+lob type is:
+BLOB | CLOB [({numeric literal} [unit])] [STORAGE 'storage literal']
+
+unit is:
+empty | 
+K | 
+M | 
+G 
+```
+
+[#semantics]
+=== Semantics
+
+* `_storage literal_`
+
++
+Currently Trafodion only support `'EXTERNAL'` here. 
--- End diff --

Thanks Dave, I've corrected. :)



---