Merge branch 'separate-policy-kresd' into 'master'

manager: policy-loader: new process for loading policy rules separately

See merge request knot/knot-resolver!1540

7 files changed, 53 insertions, 37 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 8e908a7a..9dc01a4f 100644
--- a/lib/rules/api.c
+++ b/lib/rules/api.c
@@ -141,9 +141,9 @@ int kr_rules_init_ensure(void)
 {
 	if (the_rules)
 		return kr_ok();
-	return kr_rules_init(NULL, 0);
+	return kr_rules_init(NULL, 0, true);
 }
-int kr_rules_init(const char *path, size_t maxsize)
+int kr_rules_init(const char *path, size_t maxsize, bool overwrite)
 {
 	if (the_rules)
 		return kr_error(EINVAL);
@@ -157,22 +157,17 @@ int kr_rules_init(const char *path, size_t maxsize)
 		// FIXME: the file will be sparse, but we still need to choose its size somehow.
 		// Later we might improve it to auto-resize in case of running out of space.
 		// Caveat: mdb_env_set_mapsize() can only be called without transactions open.
-		.maxsize = maxsize ? maxsize :
-			(size_t)(sizeof(size_t) > 4 ? 2048 : 500) * 1024*1024,
+		.maxsize = !overwrite ? 0 :
+			(maxsize ? maxsize : (size_t)(sizeof(size_t) > 4 ? 2048 : 500) * 1024*1024),
 	};
 	int ret = the_rules->api->open(&the_rules->db, &the_rules->stats, &opts, NULL);
-	/* No persistence - we always refill from config for now.
-	 * LATER:
-	 *  - Make it include versioning?
-	 *  - "\0stamp" key when loading config(s)?
-	 *  - Don't clear ruleset data that doesn't come directly from config;
-	 *    and add marks for that, etc.
-	 *    (after there actually are any kinds of rules like that)
-	 */
-	if (ret == 0) ret = ruledb_op(clear);
+
+	if (ret == 0 && overwrite) ret = ruledb_op(clear);
 	if (ret != 0) goto failure;
 	kr_require(the_rules->db);
 
+	if (!overwrite) return kr_ok(); // we assume that the caller ensured OK contents
+
 	ret = tag_names_default();
 	if (ret != 0) goto failure;
 
@@ -205,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 1069ef4d..f1737a19 100644
--- a/lib/rules/api.h
+++ b/lib/rules/api.h
@@ -19,11 +19,13 @@ typedef uint64_t kr_rule_tags_t;
 
 /** Open the rule DB.
  *
- * You can call this to override the path or size (NULL/0 -> default).
- * Not allowed if already open (EINVAL), so this optional call has to come
- * before writing anything into the DB. */
+ * You can call this to override the path or size (NULL/0 -> default)
+ * or choose not to overwrite the DB with just the defaults.
+ *
+ * \return error code.  Not allowed if already open (EINVAL),
+ * so this optional call has to come before writing anything into the DB. */
 KR_EXPORT
-int kr_rules_init(const char *path, size_t maxsize);
+int kr_rules_init(const char *path, size_t maxsize, bool overwrite);
 /** kr_rules_init() but OK if already open, and not allowing to override defaults. */
 KR_EXPORT
 int kr_rules_init_ensure(void);
@@ -36,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/rules/impl.h b/lib/rules/impl.h
index 0d7de513..1a2ee4dd 100644
--- a/lib/rules/impl.h
+++ b/lib/rules/impl.h
@@ -20,7 +20,7 @@ extern struct kr_rules *the_rules;
 
 #define ENSURE_the_rules \
 	if (!the_rules) { \
-		int ret = kr_rules_init(NULL, 0); \
+		int ret = kr_rules_init(NULL, 0, true); \
 		if (ret) return ret; \
 	}
 
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;