lib/rules,cache: use transactions, improve assertions

When inserting rules from a config file, process everything
in a single transaction to avoid using inconsistent sets of rules,
especially in a different instance and/or in case some error happens.

Also fix some over-eager assertions (CHECK_RET).

3 files changed, 23 insertions, 12 deletions

diff --git a/lib/cache/api.c b/lib/cache/api.c
index 4b4a8c59..37dff26e 100644
--- a/lib/cache/api.c
+++ b/lib/cache/api.c
@@ -175,7 +175,7 @@ int kr_cache_commit(struct kr_cache *cache)
 		return kr_error(EINVAL);
 	}
 	if (cache->api->commit) {
-		return cache_op(cache, commit);
+		return cache_op(cache, commit, true);
 	}
 	return kr_ok();
 }
diff --git a/lib/cache/cdb_api.h b/lib/cache/cdb_api.h
index 43d1433f..41130b62 100644
--- a/lib/cache/cdb_api.h
+++ b/lib/cache/cdb_api.h
@@ -57,8 +57,11 @@ struct kr_cdb_api {
 	int (*count)(kr_cdb_pt db, struct kr_cdb_stats *stat);
 	int (*clear)(kr_cdb_pt db, struct kr_cdb_stats *stat);
 
-	/** Run after a row of operations to release transaction/lock if needed. */
-	int (*commit)(kr_cdb_pt db, struct kr_cdb_stats *stat);
+	/** Run after a row of operations to release transaction/lock if needed.
+	 * \param accept true=commit / false=abort
+	 * \return error code - accepting RW transactions can fail with LMDB.
+	 */
+	int (*commit)(kr_cdb_pt db, struct kr_cdb_stats *stat, bool accept);
 
 	/* Data access */
 
diff --git a/lib/cache/cdb_lmdb.c b/lib/cache/cdb_lmdb.c
index 76570f19..742744bb 100644
--- a/lib/cache/cdb_lmdb.c
+++ b/lib/cache/cdb_lmdb.c
@@ -72,7 +72,8 @@ static inline kr_cdb_pt env2db(struct lmdb_env *env)
 	return (kr_cdb_pt)env;
 }
 
-static int cdb_commit(kr_cdb_pt db, struct kr_cdb_stats *stats);
+static int cdb_commit(kr_cdb_pt db, struct kr_cdb_stats *stats, bool accept);
+static void txn_abort(struct lmdb_env *env);
 
 /** @brief Convert LMDB error code. */
 static int lmdb_error(int error)
@@ -106,7 +107,7 @@ static inline MDB_val val_knot2mdb(knot_db_val_t v)
  * It's much lighter than reopen_env(). */
 static int refresh_mapsize(struct lmdb_env *env)
 {
-	int ret = cdb_commit(env2db(env), NULL);
+	int ret = cdb_commit(env2db(env), NULL, true);
 	if (!ret) ret = lmdb_error(mdb_env_set_mapsize(env->env, 0));
 	if (ret) return ret;
 
@@ -215,9 +216,14 @@ static int txn_get(struct lmdb_env *env, MDB_txn **txn, bool rdonly)
 	return kr_ok();
 }
 
-static int cdb_commit(kr_cdb_pt db, struct kr_cdb_stats *stats)
+static int cdb_commit(kr_cdb_pt db, struct kr_cdb_stats *stats, bool accept)
 {
 	struct lmdb_env *env = db2env(db);
+	if (!accept) {
+		txn_abort(env);
+		return kr_ok();
+	}
+
 	int ret = kr_ok();
 	if (env->txn.rw) {
 		if (stats) stats->commit++;
@@ -238,9 +244,11 @@ static int txn_curs_get(struct lmdb_env *env, MDB_cursor **curs, struct kr_cdb_s
 		return kr_error(EINVAL);
 	if (env->txn.ro_curs_active)
 		goto success;
-	/* Only in a read-only txn; TODO: it's a bit messy/coupled */
+	/* Only in a read-only txn; TODO: it's a bit messy/coupled
+	 * At least for rules we don't do the auto-commit feature. */
 	if (env->txn.rw) {
-		int ret = cdb_commit(env2db(env), stats);
+		if (!env->is_cache) return kr_error(EINPROGRESS);
+		int ret = cdb_commit(env2db(env), stats, true);
 		if (ret) return ret;
 	}
 	MDB_txn *txn = NULL;
@@ -296,7 +304,7 @@ static void cdb_close_env(struct lmdb_env *env, struct kr_cdb_stats *stats)
 
 	/* Get rid of any transactions. */
 	txn_free_ro(env);
-	cdb_commit(env2db(env), stats);
+	cdb_commit(env2db(env), stats, env->is_cache);
 
 	mdb_env_sync(env->env, 1);
 	stats->close++;
@@ -562,8 +570,8 @@ static int cdb_clear(kr_cdb_pt db, struct kr_cdb_stats *stats)
 		int ret = txn_get(env, &txn, false);
 		if (ret == kr_ok()) {
 			ret = lmdb_error(mdb_drop(txn, env->dbi, 0));
-			if (ret == kr_ok()) {
-				ret = cdb_commit(db, stats);
+			if (ret == kr_ok() && env->is_cache) {
+				ret = cdb_commit(db, stats, true);
 			}
 			if (ret == kr_ok()) {
 				return ret;
@@ -577,7 +585,7 @@ static int cdb_clear(kr_cdb_pt db, struct kr_cdb_stats *stats)
 
 	/* We are about to switch to a different file, so end all txns, to be sure. */
 	txn_free_ro(env);
-	(void) cdb_commit(db, stats);
+	(void) cdb_commit(db, stats, env->is_cache);
 
 	const char *path = NULL;
 	int ret = mdb_env_get_path(env->env, &path);