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.

diff --git a/daemon/lua/kres-gen-30.lua b/daemon/lua/kres-gen-30.lua
index 56f7abb11b2539a1d5b03e74a8fbbeb23c9f1159..565812aa0c91aaeb469350884cdecd17e1eab312 100644 (file)
--- a/daemon/lua/kres-gen-30.lua
+++ b/daemon/lua/kres-gen-30.lua
@@ -499,6 +499,7 @@ int kr_cache_commit(struct kr_cache *);
 uint32_t packet_ttl(const knot_pkt_t *);
 int kr_rules_init(const char *, size_t, _Bool);
 int kr_rules_commit(_Bool);
+int kr_rules_reset(void);
 int kr_view_insert_action(const char *, const char *, kr_proto_set, const char *);
 int kr_view_select_action(const struct kr_request *, knot_db_val_t *);
 int kr_rule_tag_add(const char *, kr_rule_tags_t *);

diff --git a/daemon/lua/kres-gen-31.lua b/daemon/lua/kres-gen-31.lua
index 1855c6d8bc1dd929b66b35f7cb411091ab180beb..7b7274f37bf2c584524d4032c76fc70ad0a04ee4 100644 (file)
--- a/daemon/lua/kres-gen-31.lua
+++ b/daemon/lua/kres-gen-31.lua
@@ -499,6 +499,7 @@ int kr_cache_commit(struct kr_cache *);
 uint32_t packet_ttl(const knot_pkt_t *);
 int kr_rules_init(const char *, size_t, _Bool);
 int kr_rules_commit(_Bool);
+int kr_rules_reset(void);
 int kr_view_insert_action(const char *, const char *, kr_proto_set, const char *);
 int kr_view_select_action(const struct kr_request *, knot_db_val_t *);
 int kr_rule_tag_add(const char *, kr_rule_tags_t *);

diff --git a/daemon/lua/kres-gen-32.lua b/daemon/lua/kres-gen-32.lua
index 9fe1698153d7e57e0017083ce3e4ab6c39aa8da0..c3bd0d9cede780291d4cf30278a116cd00f38f07 100644 (file)
--- a/daemon/lua/kres-gen-32.lua
+++ b/daemon/lua/kres-gen-32.lua
@@ -500,6 +500,7 @@ int kr_cache_commit(struct kr_cache *);
 uint32_t packet_ttl(const knot_pkt_t *);
 int kr_rules_init(const char *, size_t, _Bool);
 int kr_rules_commit(_Bool);
+int kr_rules_reset(void);
 int kr_view_insert_action(const char *, const char *, kr_proto_set, const char *);
 int kr_view_select_action(const struct kr_request *, knot_db_val_t *);
 int kr_rule_tag_add(const char *, kr_rule_tags_t *);

diff --git a/daemon/lua/kres-gen.sh b/daemon/lua/kres-gen.sh
index 5039a723d1ffb392587dd452f881468bdfe99023..8554a7837145e6ea73023a5ea913e06f41b1ae6d 100755 (executable)
--- a/daemon/lua/kres-gen.sh
+++ b/daemon/lua/kres-gen.sh
@@ -299,6 +299,7 @@ ${CDEFS} ${LIBKRES} functions <<-EOF
 # New policy
        kr_rules_init
        kr_rules_commit
+       kr_rules_reset
        kr_view_insert_action
        kr_view_select_action
        kr_rule_tag_add

diff --git a/lib/cache/api.c b/lib/cache/api.c
index 490f3d1cd41259cfeea267a3827d11852bbc719f..0cd185346f7217d95dbe71fffbedb1a0ade3e0fe 100644 (file)
--- 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 bce8740f7701c0beb951190f7da6999762c27e78..23a795d532d942ea9efc20f6b9f786c04c827dcb 100644 (file)
--- 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 5351cd737b6e8722d57658cadaab27673dc92754..10611513396a8dcf1010079a6df4e0858c1c8669 100644 (file)
--- 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 8d2c102849d8540b707aaba11c15201bcf43cb9f..9dc01a4fe80c1d9390d8aa2c59335dc737f6b3f9 100644 (file)
--- 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 f7f3b4660f8c2fb9659367331e49208cab383e21..f1737a1908cb1f990d85e57705b645219c4ffdfc 100644 (file)
--- 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 9cdd1a6065ac146ede4884dc97f0733ceda74c54..cdef17011659b0d4ab1f817a27d260576aaff54e 100644 (file)
--- 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;