From 9c22b31cd639911a2faffad02f2ed9f7cc10b9e1 Mon Sep 17 00:00:00 2001
From: Jiri Vymazal <jvymazal@redhat.com>
Date: Fri, 15 Mar 2019 09:29:04 +0100
Subject: [PATCH] Fetching journal cursor only for valid journal

The sd_journal_get_cursor() got called regradless of previous
retcodes from other jorunal calls which flooded logs with journald
errors. Now skipping the call in case of previous journal call
non-zero result. Fixed success checking of get_cursor() call
to eliminate double-free possibility.

Also, making WorkAroundJournalBug true by default, as there were no
confirmed performance regressions for a quite long time.
---
 plugins/imjournal/imjournal.c | 43 ++++++++++++++++++++++------------
 1 file changed, 28 insertions(+), 15 deletions(-)

diff --git a/plugins/imjournal/imjournal.c b/plugins/imjournal/imjournal.c
index 7225fae1ab..5419762cf1 100644
--- a/plugins/imjournal/imjournal.c
+++ b/plugins/imjournal/imjournal.c
@@ -135,7 +135,7 @@ static char *last_cursor = NULL;
 
 #define J_PROCESS_PERIOD 1024  /* Call sd_journal_process() every 1,024 records */
 
-static rsRetVal persistJournalState(void);
+static rsRetVal persistJournalState(int trySave);
 static rsRetVal loadJournalState(void);
 
 static rsRetVal openJournal(void) {
@@ -158,9 +158,9 @@ static rsRetVal openJournal(void) {
 	RETiRet;
 }
 
-static void closeJournal(void) {
+static void closeJournal(int trySave) {
 	if (cs.stateFile) { /* can't persist without a state file */
-		persistJournalState();
+		persistJournalState(trySave);
 	}
 	sd_journal_close(j);
 	j_inotify_fd = 0;
@@ -461,7 +461,7 @@ readjournal(void)
 /* This function gets journal cursor and saves it into state file
  */
 static rsRetVal
-persistJournalState(void)
+persistJournalState(int trySave)
 {
 	DEFiRet;
 	FILE *sf; /* state file */
@@ -469,7 +470,7 @@ persistJournalState(void)
 		if (!last_cursor) {
 			ABORT_FINALIZE(RS_RET_OK);
 		}
-	} else {
+	} else if (trySave) {
 		int ret;
 		free(last_cursor);
 		if ((ret = sd_journal_get_cursor(j, &last_cursor))) {
@@ -477,6 +478,8 @@ persistJournalState(void)
 			last_cursor = NULL;
 			ABORT_FINALIZE(RS_RET_ERR);
 		}
+	} else { /* not trying to get cursor out of invalid journal state */
+		ABORT_FINALIZE(RS_RET_OK);
 	}
 
 	/* we create a temporary name by adding a ".tmp"
@@ -535,14 +535,24 @@ pollJournal(void)
 	err = sd_journal_wait(j, POLL_TIMEOUT);
 	if (err == SD_JOURNAL_INVALIDATE) {
 		STATSCOUNTER_INC(statsCounter.ctrRotations, statsCounter.mutCtrRotations);
-		closeJournal();
+		closeJournal(0);
 
 		iRet = openJournal();
 		if (iRet != RS_RET_OK) {
 			ABORT_FINALIZE(RS_RET_ERR);
 		}
 
-		if (cs.stateFile) {
+		/* If we have locally saved cursor there is no need to read it from state file */
+		if (cs.bWorkAroundJournalBug && last_cursor)
+		{
+			if (sd_journal_seek_cursor(j, last_cursor) != 0) {
+				LogError(0, RS_RET_ERR, "imjournal: "
+					"couldn't seek to cursor `%s'\n", last_cursor);
+				iRet = RS_RET_ERR;
+			}
+			sd_journal_next(j);
+		}
+		else if (cs.stateFile) {
 			iRet = loadJournalState();
 		}
 		LogMsg(0, RS_RET_OK, LOG_NOTICE, "imjournal: journal reloaded...");
@@ -668,10 +680,9 @@ loadJournalState(void)
 
 static void
 tryRecover(void) {
-	LogMsg(0, RS_RET_OK, LOG_INFO, "imjournal: trying to recover from unexpected "
-		"journal error");
+	LogMsg(0, RS_RET_OK, LOG_INFO, "imjournal: trying to recover from journal error");
 	STATSCOUNTER_INC(statsCounter.ctrRecoveryAttempts, statsCounter.mutCtrRecoveryAttempts);
-	closeJournal();
+	closeJournal(0);
 	srSleep(10, 0);	// do not hammer machine with too-frequent retries
 	openJournal();
 }
@@ -768,7 +779,7 @@ CODESTARTrunInput
 		if (cs.stateFile) { /* can't persist without a state file */
 			/* TODO: This could use some finer metric. */
 			if ((count % cs.iPersistStateInterval) == 0) {
-				persistJournalState();
+				persistJournalState(1);
 			}
 		}
 	}
@@ -790,7 +801,7 @@ CODESTARTbeginCnfLoad
 	cs.iDfltFacility = DFLT_FACILITY;
 	cs.bUseJnlPID = -1;
 	cs.usePid = NULL;
-	cs.bWorkAroundJournalBug = 0;
+	cs.bWorkAroundJournalBug = 1;
 	cs.dfltTag = NULL;
 ENDbeginCnfLoad
 
@@ -860,7 +871,7 @@ ENDwillRun
 /* close journal */
 BEGINafterRun
 CODESTARTafterRun
-	closeJournal();
+	closeJournal(1);
 	ratelimitDestruct(ratelimiter);
 ENDafterRun