From d3592db45bfc4939c2d72c73a42ed9f79cb64052 Mon Sep 17 00:00:00 2001
From: Rainer Gerhards <rgerhards@adiscon.com>
Date: Wed, 16 Dec 2009 16:21:52 +0100
Subject: bugfix: ompgsql did not properly check the server connection in
 tryResume()

what could lead to rsyslog running in a thight loop.
Also did some code cleanup of previous patch.
---
 ChangeLog                 |  4 ++++
 action.c                  |  8 --------
 plugins/ompgsql/ompgsql.c | 19 ++++++++++++++-----
 3 files changed, 18 insertions(+), 13 deletions(-)

diff --git a/ChangeLog b/ChangeLog
index 37f04988..a52261de 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,5 +1,9 @@
 ---------------------------------------------------------------------------
 Version 5.3.6  [BETA] (rgerhards), 2009-11-??
+- bugfix: ompgsql did not properly check the server connection in
+  tryResume(), which could lead to rsyslog running in a thight loop
+- bugfix: suspension during beginTransaction() was not properly handled
+  by rsyslog core
 - bugfix: omfile output was only written when buffer was full, not at
   end of transaction
 - bugfix: commit transaction was not properly conveyed to message layer,
diff --git a/action.c b/action.c
index 5724de2f..67858742 100644
--- a/action.c
+++ b/action.c
@@ -491,17 +491,14 @@ static rsRetVal actionDoRetry(action_t *pThis, time_t ttNow)
 
 	ASSERT(pThis != NULL);
 
-dbgprintf("XXX: we are doing a retry, action state %d\n", pThis->eState);
 	iRetries = 0;
 	while(pThis->eState == ACT_STATE_RTRY) {
 		iRet = pThis->pMod->tryResume(pThis->pModData);
-dbgprintf("XXX: result of tryResume %d\n", iRet);
 		if(iRet == RS_RET_OK) {
 			actionSetState(pThis, ACT_STATE_RDY);
 		} else if(iRet == RS_RET_SUSPENDED) {
 			/* max retries reached? */
 			if((pThis->iResumeRetryCount != -1 && iRetries >= pThis->iResumeRetryCount)) {
-dbgprintf("XXX: action suspeneded!\n");
 				actionSuspend(pThis, ttNow);
 			} else {
 				++pThis->iNbrResRtry;
@@ -533,7 +530,6 @@ static rsRetVal actionTryResume(action_t *pThis)
 
 	ASSERT(pThis != NULL);
 
-dbgprintf("XXX: actionTryResume, action state %d\n", pThis->eState);
 	if(pThis->eState == ACT_STATE_SUSP) {
 		/* if we are suspended, we need to check if the timeout expired.
 		 * for this handling, we must always obtain a fresh timestamp. We used
@@ -580,7 +576,6 @@ static rsRetVal actionPrepare(action_t *pThis)
 	 */
 	if(pThis->eState == ACT_STATE_RDY) {
 		iRet = pThis->pMod->mod.om.beginTransaction(pThis->pModData);
-dbgprintf("XXX: actionPrepare, result of beginTranscation %d\n", iRet);
 		switch(iRet) {
 			case RS_RET_OK:
 				actionSetState(pThis, ACT_STATE_ITX);
@@ -717,7 +712,6 @@ actionCallDoAction(action_t *pThis, msg_t *pMsg)
 
 	pThis->bHadAutoCommit = 0;
 	iRet = pThis->pMod->mod.om.doAction(pThis->ppMsgs, pMsg->msgFlags, pThis->pModData);
-dbgprintf("XXX: result of doAction: %d\n", iRet);
 	switch(iRet) {
 		case RS_RET_OK:
 			actionCommitted(pThis);
@@ -790,7 +784,6 @@ finishBatch(action_t *pThis, batch_t *pBatch)
 	CHKiRet(actionPrepare(pThis));
 	if(pThis->eState == ACT_STATE_ITX) {
 		iRet = pThis->pMod->mod.om.endTransaction(pThis->pModData);
-dbgprintf("XXX: finishBatch, result of endTranscation %d\n", iRet);
 		switch(iRet) {
 			case RS_RET_OK:
 				actionCommitted(pThis);
@@ -908,7 +901,6 @@ submitBatch(action_t *pAction, batch_t *pBatch, int nElem, int *pbShutdownImmedi
 	bDone = 0;
 	do {
 		localRet = tryDoAction(pAction, pBatch, &nElem, pbShutdownImmediate);
-dbgprintf("submitBatch: localRet %d\n", localRet);
 		if(localRet == RS_RET_FORCE_TERM)
 			FINALIZE;
 		if(   localRet == RS_RET_OK
diff --git a/plugins/ompgsql/ompgsql.c b/plugins/ompgsql/ompgsql.c
index 784a722d..c13f58e9 100644
--- a/plugins/ompgsql/ompgsql.c
+++ b/plugins/ompgsql/ompgsql.c
@@ -65,6 +65,8 @@ typedef struct _instanceData {
 } instanceData;
 
 
+static rsRetVal writePgSQL(uchar *psz, instanceData *pData);
+
 BEGINcreateInstance
 CODESTARTcreateInstance
 ENDcreateInstance
@@ -170,9 +172,6 @@ tryExec(uchar *pszCmd, instanceData *pData)
 	int bHadError = 0;
 
 	/* try insert */
-BEGINfunc
-RUNLOG_VAR("%p", pData->f_hpgsql);
-RUNLOG_VAR("%s", pszCmd);
 	pgRet = PQexec(pData->f_hpgsql, (char*)pszCmd);
 	execState = PQresultStatus(pgRet);
 	if(execState != PGRES_COMMAND_OK && execState != PGRES_TUPLES_OK) {
@@ -181,7 +180,6 @@ RUNLOG_VAR("%s", pszCmd);
 	}
 	PQclear(pgRet);
 
-ENDfunc
 	return(bHadError);
 }
 
@@ -193,7 +191,8 @@ ENDfunc
  * a sql format error - connection aborts were properly handled
  * before my patch. -- rgerhards, 2009-04-17
  */
-rsRetVal writePgSQL(uchar *psz, instanceData *pData)
+static rsRetVal
+writePgSQL(uchar *psz, instanceData *pData)
 {
 	int bHadError = 0;
 	DEFiRet;
@@ -231,6 +230,16 @@ BEGINtryResume
 CODESTARTtryResume
 	if(pData->f_hpgsql == NULL) {
 		iRet = initPgSQL(pData, 1);
+		if(iRet == RS_RET_OK) {
+			/* the code above seems not to actually connect to the database. As such, we do a
+			 * dummy statement (a pointless select...) to verify the connection and return
+			 * success only when that statemetn succeeds. Note that I am far from being a 
+			 * PostgreSQL expert, so any patch that does the desired result in a more
+			 * intelligent way is highly welcome. -- rgerhards, 2009-12-16
+			 */
+			iRet = writePgSQL((uchar*)"select 'a' as a", pData);
+		}
+
 	}
 ENDtryResume
 
-- 
cgit