[GitHub] trafodion pull request #1499: [TRAFODION-3003]Trafodion keepalive support

2018-04-16 Thread asfgit
Github user asfgit closed the pull request at:

https://github.com/apache/trafodion/pull/1499


---


[GitHub] trafodion pull request #1499: [TRAFODION-3003]Trafodion keepalive support

2018-03-28 Thread CoderSong2015
Github user CoderSong2015 commented on a diff in the pull request:

https://github.com/apache/trafodion/pull/1499#discussion_r177963928
  
--- Diff: 
core/conn/odbc/src/odbc/nsksrvr/Interface/linux/Listener_srvr_ps.cpp ---
@@ -263,7 +263,11 @@ void* CNSKListenerSrvr::OpenTCPIPSession()
 //LCOV_EXCL_STOP
}
 
-
+TCP_SetKeepalive(nSocketFnum,
--- End diff --

I think you are right, but I have seen the same name style in mxosrvr a lot 
of times, like in SrvrConnect.cpp and odbcas_drvr.cpp. So I think they are all 
allowed here.


---


[GitHub] trafodion pull request #1499: [TRAFODION-3003]Trafodion keepalive support

2018-03-28 Thread CoderSong2015
Github user CoderSong2015 commented on a diff in the pull request:

https://github.com/apache/trafodion/pull/1499#discussion_r177961670
  
--- Diff: core/conn/odbc/src/odbc/Common/Global.h ---
@@ -139,10 +139,16 @@ class ODBCMXTraceMsg;
 #define DEFAULT_REFRESH_RATE_SECS  60
 #define DEFAULT_SRVR_IDLE_TIMEOUT  0
 #define DEFAULT_CONN_IDLE_TIMEOUT  0
+#define DEFAULT_KEEPALIVE   1 //OPEN KEEPALIVE
+#define DEFAULT_KEEPALIVE_TIMESEC   3600
+#define DEFAULT_KEEPALIVE_COUNT 3
--- End diff --

I think 3 times and 5 times are both enough here. Can your tell me why it 
is better to retry more times?


---


[GitHub] trafodion pull request #1499: [TRAFODION-3003]Trafodion keepalive support

2018-03-28 Thread xiaozhongwang
Github user xiaozhongwang commented on a diff in the pull request:

https://github.com/apache/trafodion/pull/1499#discussion_r177483293
  
--- Diff: core/conn/odbc/src/odbc/Common/Global.h ---
@@ -935,7 +941,10 @@ typedef struct _SRVR_GLOBAL_Def
bzero(m_ProcName,sizeof(m_ProcName));
m_bNewConnection = false;
m_bNewService = false;
-
+   bzero(clientKeepaliveStatus, sizeof(clientKeepaliveStatus));
--- End diff --

Why use clientKeepaliveStatus instead of keepalive option?


---


[GitHub] trafodion pull request #1499: [TRAFODION-3003]Trafodion keepalive support

2018-03-28 Thread xiaozhongwang
Github user xiaozhongwang commented on a diff in the pull request:

https://github.com/apache/trafodion/pull/1499#discussion_r177485881
  
--- Diff: core/conn/odbc/src/odbc/Common/Global.h ---
@@ -139,10 +139,16 @@ class ODBCMXTraceMsg;
 #define DEFAULT_REFRESH_RATE_SECS  60
 #define DEFAULT_SRVR_IDLE_TIMEOUT  0
 #define DEFAULT_CONN_IDLE_TIMEOUT  0
+#define DEFAULT_KEEPALIVE   1 //OPEN KEEPALIVE
+#define DEFAULT_KEEPALIVE_TIMESEC   3600
+#define DEFAULT_KEEPALIVE_COUNT 3
--- End diff --

where the default value come from?
I think it's better to retry more times。


---


[GitHub] trafodion pull request #1499: [TRAFODION-3003]Trafodion keepalive support

2018-03-28 Thread xiaozhongwang
Github user xiaozhongwang commented on a diff in the pull request:

https://github.com/apache/trafodion/pull/1499#discussion_r177489126
  
--- Diff: dcs/src/main/java/org/trafodion/dcs/Constants.java ---
@@ -112,6 +112,29 @@
 /** Default value for DCS server user program exit after disconnect */
 public static final int 
DEFAULT_DCS_SERVER_USER_PROGRAM_EXIT_AFTER_DISCONNECT = 0;
 
+/** Configuration key for DCS server program mxosrvr keepalive STATUS*/
+public static final String  
DEFAULT_DCS_SERVER_PROGRAM_TCP_KEEPALIVE_STATUS= 
"dcs.server.user.program.tcp.keepalive.status";
+
+/** Default value for DCS server program mxosrvr keepalive STATUS*/
+public static final String DCS_SERVER_PROGRAM_KEEPALIVE_STATUS = 
"enable";
+
+/** Configuration key for DCS server program mxosrvr keepalive 
IDLETIME*/
+public static final String 
DEFAULT_DCS_SERVER_PROGRAM_TCP_KEEPALIVE_IDLETIME = 
"dcs.server.user.program.tcp.keepalive.idletime";
+
+/** Default value for DCS server program mxosrvr keepalive IDLETIME*/
+public static final int DCS_SERVER_PROGRAM_KEEPALIVE_IDLETIME = 300;
--- End diff --

Why client side use different value with server?


---


[GitHub] trafodion pull request #1499: [TRAFODION-3003]Trafodion keepalive support

2018-03-28 Thread xiaozhongwang
Github user xiaozhongwang commented on a diff in the pull request:

https://github.com/apache/trafodion/pull/1499#discussion_r177487560
  
--- Diff: 
core/conn/odbc/src/odbc/nsksrvr/Interface/linux/Listener_srvr_ps.cpp ---
@@ -263,7 +263,11 @@ void* CNSKListenerSrvr::OpenTCPIPSession()
 //LCOV_EXCL_STOP
}
 
-
+TCP_SetKeepalive(nSocketFnum,
--- End diff --

it's better to use single name style in one name.


---


[GitHub] trafodion pull request #1499: [TRAFODION-3003]Trafodion keepalive support

2018-03-27 Thread CoderSong2015
Github user CoderSong2015 commented on a diff in the pull request:

https://github.com/apache/trafodion/pull/1499#discussion_r177620171
  
--- Diff: dcs/src/main/java/org/trafodion/dcs/server/ServerManager.java ---
@@ -83,6 +83,10 @@
 private int maxRestartAttempts;
 private int retryIntervalMillis;
 private String nid = null;
+private static String mxosrvrKeepaliveStatus;
--- End diff --

OK.


---


[GitHub] trafodion pull request #1499: [TRAFODION-3003]Trafodion keepalive support

2018-03-27 Thread hegdean
Github user hegdean commented on a diff in the pull request:

https://github.com/apache/trafodion/pull/1499#discussion_r177531067
  
--- Diff: dcs/src/main/java/org/trafodion/dcs/server/ServerManager.java ---
@@ -83,6 +83,10 @@
 private int maxRestartAttempts;
 private int retryIntervalMillis;
 private String nid = null;
+private static String mxosrvrKeepaliveStatus;
--- End diff --

can u change mxosrvr to userProg just like the other properties already 
defined


---


[GitHub] trafodion pull request #1499: [TRAFODION-3003]Trafodion keepalive support

2018-03-27 Thread hegdean
Github user hegdean commented on a diff in the pull request:

https://github.com/apache/trafodion/pull/1499#discussion_r177530157
  
--- Diff: dcs/src/main/resources/dcs-default.xml ---
@@ -386,4 +386,33 @@
 Timeout minutes between first and max times (6 default) DCS Server 
startup MXOSRVR.
 
   
+  
+dcs.server.user.program.tcp.keepalive.status
+enable
+
+Used in  mxosrvr keepalive , parameter is ENABLE IDLETIME 
INTERTIME RETRYCNT.
+
+  
+  
+dcs.server.user.program.tcp.keepalive.idletime
+300
+
+Used in  mxosrvr keepalive , parameter is ENABLE IDLETIME 
INTERTIME RETRYCNT.
+
+  
+  
+dcs.server.user.program.tcp.keepalive.intervaltime
+5
+
+Used in  mxosrvr keepalive , parameter is ENABLE IDLETIME 
INTERTIME RETRYCNT.
--- End diff --

description should be specific to the property. Also would be good to 
indicate the unit of time  mins, secs or msec and so on


---


[GitHub] trafodion pull request #1499: [TRAFODION-3003]Trafodion keepalive support

2018-03-27 Thread hegdean
Github user hegdean commented on a diff in the pull request:

https://github.com/apache/trafodion/pull/1499#discussion_r177529786
  
--- Diff: dcs/src/main/resources/dcs-default.xml ---
@@ -386,4 +386,33 @@
 Timeout minutes between first and max times (6 default) DCS Server 
startup MXOSRVR.
 
   
+  
+dcs.server.user.program.tcp.keepalive.status
+enable
+
+Used in  mxosrvr keepalive , parameter is ENABLE IDLETIME 
INTERTIME RETRYCNT.
--- End diff --

Description should be specific to the property name/value..  specify the 
default for the property.  Look at how the description for statistics enable is 
done. This applies to the other tcp keepalive properties as well


---


[GitHub] trafodion pull request #1499: [TRAFODION-3003]Trafodion keepalive support

2018-03-27 Thread hegdean
Github user hegdean commented on a diff in the pull request:

https://github.com/apache/trafodion/pull/1499#discussion_r177527743
  
--- Diff: core/conn/odbc/src/odbc/nsksrvr/Interface/Listener_srvr.cpp ---
@@ -81,4 +81,49 @@ void CNSKListenerSrvr::TCP_PROCESSNAME_PORT(FILE* fp)
fprintf(fp,"<==TCP/PORT (%s/%d)==>\n",m_TcpProcessName, 
m_port);
 }
 
+void CNSKListenerSrvr::TCP_SetKeepalive(int socketnum,
+char *keepaliveStatus,
+int idleTime,
+int intervalTime,
+int retryCount)
+{
+//all need to be configured
+if(NULL == keepaliveStatus){
+return;
+}
+if(0 == strcmp(keepaliveStatus,"default")){
--- End diff --

Can the status be boolean (TRUE or FALSE)  and default value can be set 
FALSE


---


[GitHub] trafodion pull request #1499: [TRAFODION-3003]Trafodion keepalive support

2018-03-27 Thread CoderSong2015
GitHub user CoderSong2015 opened a pull request:

https://github.com/apache/trafodion/pull/1499

[TRAFODION-3003]Trafodion keepalive support

Keepalive could be configured by modifying file 
src/main/java/org/trafodion/dcs/Constants.java

Modify variable 
DCS_SERVER_PROGRAM_TCP_KEEPALIVE_STATUS/IDLETIME/INTERVAL/RETRYCOUNT;

DCS_SERVER_PROGRAM_TCP_KEEPALIVE_STATUS has three 
value:enable,default,unenable;

Default value is enable,300,3,20(Only effective when value configured is 
set incorrectly)

The value will be read in when mxosrvr start. Mxosrvr will set the socket 
after getting a connection.

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

$ git pull https://github.com/CoderSong2015/Apache-Trafodion keepalive

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

https://github.com/apache/trafodion/pull/1499.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 #1499


commit 1b19b963c8284f974a76e8eed8d7cd433a536024
Author: Haolin.song <403438485@...>
Date:   2018-03-20T15:31:31Z

[TRAFODION-3003]Trafodion keepalive support

Keepalive could be configured by modifying file 
src/main/java/org/trafodion/dcs/Constants.java

Modify variable 
DCS_SERVER_PROGRAM_TCP_KEEPALIVE_STATUS/IDLETIME/INTERVAL/RETRYCOUNT;

DCS_SERVER_PROGRAM_TCP_KEEPALIVE_STATUS has three 
value:enable,default,unenable;

Default value is enable,300,3,20(Only effective when value configured is 
set incorrectly)

The value will be read in when mxosrvr start. Mxosrvr will set the socket 
after getting a connection.




---