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