[GitHub] trafodion pull request #1499: [TRAFODION-3003]Trafodion keepalive support
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
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
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
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
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
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
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
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
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
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
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
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
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. ---