lib/rules: tweak how the read-only transactions work

Let's avoid reloading the RO transaction unless necessary.
For example, when normal config reload happens (one kresd at a time),
we most likely do *not* want to reload the rule DB prematurely.

6 files changed, 38 insertions, 19 deletions

diff --git a/lib/cache/api.c b/lib/cache/api.c
index 490f3d1c..0cd18534 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, true);
+		return cache_op(cache, commit, true, true);
 	}
 	return kr_ok();
 }
diff --git a/lib/cache/cdb_api.h b/lib/cache/cdb_api.h
index bce8740f..23a795d5 100644
--- a/lib/cache/cdb_api.h
+++ b/lib/cache/cdb_api.h
@@ -58,10 +58,11 @@ struct kr_cdb_api {
 	int (*clear)(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
+	 * \param accept_rw whether the RW transaction should accept changes (commit vs. abort)
+	 * \param reset_ro whether the RO transaction should be ended (newest data next time)
 	 * \return error code - accepting RW transactions can fail with LMDB.
 	 */
-	int (*commit)(kr_cdb_pt db, struct kr_cdb_stats *stat, bool accept);
+	int (*commit)(kr_cdb_pt db, struct kr_cdb_stats *stat, bool accept_rw, bool reset_ro);
 
 	/* Data access */
 
diff --git a/lib/cache/cdb_lmdb.c b/lib/cache/cdb_lmdb.c
index 5351cd73..10611513 100644
--- a/lib/cache/cdb_lmdb.c
+++ b/lib/cache/cdb_lmdb.c
@@ -76,7 +76,7 @@ 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, bool accept);
+static int cdb_commit(kr_cdb_pt db, struct kr_cdb_stats *stats, bool accept_rw, bool reset_ro);
 static void txn_abort(struct lmdb_env *env);
 
 /** @brief Convert LMDB error code. */
@@ -114,7 +114,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, true);
+	int ret = cdb_commit(env2db(env), NULL, true, true);
 	if (!ret) ret = lmdb_error(env, mdb_env_set_mapsize(env->env, 0));
 	if (ret) return ret;
 
@@ -223,20 +223,20 @@ 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, bool accept)
+static int cdb_commit(kr_cdb_pt db, struct kr_cdb_stats *stats, bool accept_rw, bool reset_ro)
 {
 	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++;
-		ret = lmdb_error(env, mdb_txn_commit(env->txn.rw));
+		if (accept_rw) {
+			if (stats) stats->commit++;
+			ret = lmdb_error(env, mdb_txn_commit(env->txn.rw));
+		} else {
+			mdb_txn_abort(env->txn.rw);
+		}
 		env->txn.rw = NULL; /* the transaction got freed even in case of errors */
-	} else if (env->txn.ro && env->txn.ro_active) {
+	} else if (reset_ro && env->txn.ro && env->txn.ro_active) {
 		mdb_txn_reset(env->txn.ro);
 		env->txn.ro_active = false;
 		env->txn.ro_curs_active = false;
@@ -256,7 +256,7 @@ static int txn_curs_get(struct lmdb_env *env, MDB_cursor **curs, struct kr_cdb_s
 	 * At least for rules we don't do the auto-commit feature. */
 	if (env->txn.rw) {
 		if (!env->is_cache) return kr_error(EINPROGRESS);
-		int ret = cdb_commit(env2db(env), stats, true);
+		int ret = cdb_commit(env2db(env), stats, true, false);
 		if (ret) return ret;
 	}
 	MDB_txn *txn = NULL;
@@ -312,7 +312,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, env->is_cache);
+	cdb_commit(env2db(env), stats, env->is_cache, true);
 
 	mdb_env_sync(env->env, 1);
 	stats->close++;
@@ -574,7 +574,7 @@ static int cdb_clear(kr_cdb_pt db, struct kr_cdb_stats *stats)
 		if (ret == kr_ok()) {
 			ret = lmdb_error(env, mdb_drop(txn, env->dbi, 0));
 			if (ret == kr_ok() && env->is_cache) {
-				ret = cdb_commit(db, stats, true);
+				ret = cdb_commit(db, stats, true, true);
 			}
 			if (ret == kr_ok()) {
 				return ret;
@@ -588,7 +588,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, env->is_cache);
+	(void)cdb_commit(db, stats, env->is_cache, true);
 
 	const char *path = NULL;
 	int ret = mdb_env_get_path(env->env, &path);
diff --git a/lib/rules/api.c b/lib/rules/api.c
index 8d2c1028..9dc01a4f 100644
--- a/lib/rules/api.c
+++ b/lib/rules/api.c
@@ -200,7 +200,13 @@ void kr_rules_deinit(void)
 int kr_rules_commit(bool accept)
 {
 	if (!the_rules) return kr_error(EINVAL);
-	return ruledb_op(commit, accept);
+	return ruledb_op(commit, accept, false);
+}
+
+int kr_rules_reset(void)
+{
+	if (!the_rules) return kr_error(EINVAL);
+	return ruledb_op(commit, false, true);
 }
 
 static bool kr_rule_consume_tags(knot_db_val_t *val, const struct kr_request *req)
diff --git a/lib/rules/api.h b/lib/rules/api.h
index f7f3b466..f1737a19 100644
--- a/lib/rules/api.h
+++ b/lib/rules/api.h
@@ -38,10 +38,22 @@ void kr_rules_deinit(void);
  * Normally commit happens only on successfully loading a config file.
  * However, an advanced user may get in trouble e.g. if calling resolve() from there,
  * causing even an assertion failure.  In that case they might want to commit explicitly.
+ *
+ * If only read-only transaction is open, this will NOT reset it to the newest data.
  */
 KR_EXPORT
 int kr_rules_commit(bool accept);
 
+/** Reset to the latest version of rules committed in the DB.
+ *
+ * Note that this is not always a good idea.  For example, the `forward` rules
+ * now use data from both the DB and lua config, so reloading only the DB
+ * may lead to weird behavior in some cases.
+ * (Modifications will also do this, as you can only modify the latest DB.)
+ */
+KR_EXPORT
+int kr_rules_reset(void);
+
 /** Try answering the query from local data; WIP: otherwise determine data source overrides.
  *
  * \return kr_error() on errors, >0 if answered, 0 otherwise (also when forwarding)
diff --git a/lib/selection.c b/lib/selection.c
index 9cdd1a60..cdef1701 100644
--- a/lib/selection.c
+++ b/lib/selection.c
@@ -173,7 +173,7 @@ int put_rtt_state(const uint8_t *ip, size_t len, struct rtt_state state,
 				.data = &state };
 
 	int ret = cache->api->write(db, stats, &key, &value, 1);
-	cache->api->commit(db, stats, true);
+	kr_cache_commit(cache);
 
 	free(key.data);
 	return ret;