cache: log if we received a commit request while already one in progress

This patch improves the case in which we receive a commit request
but we are already performing one. This behaviour is suspicious
since the HA manager should not trigger a double master transition.
Otherwise, something probably is not configured appropriately.

This improves 98756c2608f0879a2322919c7441973216565272
"cache: close commit request if we already have one in progress".

Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>

3 files changed, 14 insertions, 10 deletions

diff --git a/include/cache.h b/include/cache.h
index 28917f2..ddf2049 100644
--- a/include/cache.h
+++ b/include/cache.h
@@ -121,7 +121,7 @@ void cache_iterate_limit(struct cache *c, void *data, uint32_t from, uint32_t st
 struct nfct_handle;
 
 void cache_dump(struct cache *c, int fd, int type);
-void cache_commit(struct cache *c, struct nfct_handle *h, int clientfd);
+int cache_commit(struct cache *c, struct nfct_handle *h, int clientfd);
 void cache_flush(struct cache *c);
 void cache_bulk(struct cache *c);
 
diff --git a/src/cache_iterators.c b/src/cache_iterators.c
index 2707366..3248c70 100644
--- a/src/cache_iterators.c
+++ b/src/cache_iterators.c
@@ -175,7 +175,7 @@ static int do_commit_master(void *data, void *n)
 	return 0;
 }
 
-void cache_commit(struct cache *c, struct nfct_handle *h, int clientfd)
+int cache_commit(struct cache *c, struct nfct_handle *h, int clientfd)
 {
 	unsigned int commit_ok, commit_fail;
 	struct __commit_container tmp = {
@@ -184,11 +184,11 @@ void cache_commit(struct cache *c, struct nfct_handle *h, int clientfd)
 	};
 	struct timeval commit_stop, res;
 
-	/* we already have one commit in progress, close this request. */
-	if (clientfd && STATE_SYNC(commit).clientfd != -1) {
-		close(clientfd);
-		return;
-	}
+	/* we already have one commit in progress, skip this. The clientfd
+	 * descriptor has to be closed by the caller. */
+	if (clientfd && STATE_SYNC(commit).clientfd != -1)
+		return 0;
+
 	switch(STATE_SYNC(commit).state) {
 	case COMMIT_STATE_INACTIVE:
 		gettimeofday(&STATE_SYNC(commit).stats.start, NULL);
@@ -205,7 +205,7 @@ void cache_commit(struct cache *c, struct nfct_handle *h, int clientfd)
 			STATE_SYNC(commit).state = COMMIT_STATE_MASTER;
 			/* give it another step as soon as possible */
 			write_evfd(STATE_SYNC(commit).evfd);
-			return;
+			return 1;
 		}
 		STATE_SYNC(commit).current = 0;
 		STATE_SYNC(commit).state = COMMIT_STATE_RELATED;
@@ -219,7 +219,7 @@ void cache_commit(struct cache *c, struct nfct_handle *h, int clientfd)
 			STATE_SYNC(commit).state = COMMIT_STATE_RELATED;
 			/* give it another step as soon as possible */
 			write_evfd(STATE_SYNC(commit).evfd);
-			return;
+			return 1;
 		}
 		/* calculate the time that commit has taken */
 		gettimeofday(&commit_stop, NULL);
@@ -248,6 +248,7 @@ void cache_commit(struct cache *c, struct nfct_handle *h, int clientfd)
 		close(STATE_SYNC(commit).clientfd);
 		STATE_SYNC(commit).clientfd = -1;
 	}
+	return 1;
 }
 
 static int do_flush(void *data, void *n)
diff --git a/src/external_cache.c b/src/external_cache.c
index c1181dc..59c706a 100644
--- a/src/external_cache.c
+++ b/src/external_cache.c
@@ -90,7 +90,10 @@ static void external_cache_dump(int fd, int type)
 
 static int external_cache_commit(struct nfct_handle *h, int fd)
 {
-	cache_commit(external, h, fd);
+	if (!cache_commit(external, h, fd)) {
+		dlog(LOG_NOTICE, "commit already in progress, skipping");
+		return LOCAL_RET_OK;
+	}
 	/* Keep the client socket open, we want synchronous commits. */
 	return LOCAL_RET_STOLEN;
 }