Delete the validation thread pool

My original intent was "deprecate thread-pool.validation.max," but it
turns out it was just a symptom of a (mostly inoffensive)
overcomplication.

thread-pool.validation.max has proven confusing to users, because it
doesn't make sense for it to be configurable. The thread count should
always equal the number of RPKI trees, which in turn equals the number
of TALs. There's no reason why Fort should offload this decision to the
user.

As for the thread pool, the validation cycle is not really a fitting
problem for such an ellaborate solution, because the former involves a
very small amount (typically 5) of long-lived threads that start at the
same time, once every hour or so.

So instead of pooling a configured amount of threads in the beginning,
spawn raw threads as needed.

The RTR server still employs a thread pool.

diff --git a/docs/usage.md b/docs/usage.md
index 02ff1a09e2b3d3b964807735e58281e51d1400c1..a0a1661545da7084250cff26c0a5e99b0c244b0e 100644 (file)
--- a/docs/usage.md
+++ b/docs/usage.md
@@ -62,7 +62,6 @@ description: Guide to use arguments of FORT Validator.
        46. [`--asn1-decode-max-stack`](#--asn1-decode-max-stack)
        47. [`--stale-repository-period`](#--stale-repository-period)
        48. [`--thread-pool.server.max`](#--thread-poolservermax)
-       49. [`--thread-pool.validation.max`](#--thread-poolvalidationmax)
        50. [`--rsync.enabled`](#--rsyncenabled)
        51. [`--rsync.priority`](#--rsyncpriority)
        52. [`--rsync.strategy`](#--rsyncstrategy)
@@ -84,6 +83,7 @@ description: Guide to use arguments of FORT Validator.
        5. [`--rrdp.retry.interval`](#--rrdpretryinterval)
        60. [`init-locations`](#init-locations)
        41. [`--http.idle-timeout`](#--httpidle-timeout)
+       49. [`--thread-pool.validation.max`](#--thread-poolvalidationmax)
 
 ## Syntax
 
@@ -912,19 +912,6 @@ Number of threads the RTR server will reserve for RTR client (router) request ha
 
 > Before Fort 1.5.1, this value used to represent the maximum number of client _connections_ the server would be able to hold at any given time. It scales better now.
 
-### `--thread-pool.validation.max`
-
-- **Type:** Integer
-- **Availability:** `argv` and JSON
-- **Default:** 5
-- **Range:** 1--100
-
-> ![Warning!](img/warn.svg) Deprecated. This value should always equal the number of TALs you have, and will therefore be automated and retired in the future.
-
-Number of threads in the validation thread pool.
-
-During every validation cycle, one thread is borrowed from this pool per TAL, to validate the RPKI tree of the corresponding TAL.
-
 ### `--rsync.enabled`
 
 - **Type:** Boolean (`true`, `false`)
@@ -1298,3 +1285,14 @@ rsync synchronization strategy. Commands the way rsync URLs are approached durin
 - **Range:** 0--[`UINT_MAX`](http://pubs.opengroup.org/onlinepubs/9699919799/basedefs/limits.h.html)
 
 Deprecated alias for [`--http.low-speed-time`](#--httplow-speed-time).
+
+### `--thread-pool.validation.max`
+
+- **Type:** Integer
+- **Availability:** `argv` and JSON
+- **Default:** 5
+- **Range:** 1--100
+
+> ![img/warn.svg](img/warn.svg) This argument **is DEPRECATED**.
+
+Does nothing as of Fort 1.6.0.
\ No newline at end of file

diff --git a/src/config.c b/src/config.c
index 46206d470bc234c3d4a84161c4954f8d096b132d..18b7bec8208b3b8941c3773ed5e39ecd89618853 100644 (file)
--- a/src/config.c
+++ b/src/config.c
@@ -187,7 +187,7 @@ struct rpki_config {
                } server;
                /* Threads related to validation cycles */
                struct {
-                       unsigned int max;
+                       unsigned int max; /* Deprecated */
                } validation;
        } thread_pool;
 };
@@ -739,7 +739,8 @@ static const struct option_field options[] = {
                .type = &gt_uint,
                .offset = offsetof(struct rpki_config,
                    thread_pool.validation.max),
-               .doc = "Number of threads in the validation thread pool. (Each thread handles one TAL tree.)",
+               .doc = "Deprecated; does nothing.",
+               .deprecated = true,
                .min = 0,
                .max = 100,
        },
@@ -1429,12 +1430,6 @@ config_get_thread_pool_server_max(void)
        return rpki_config.thread_pool.server.max;
 }
 
-unsigned int
-config_get_thread_pool_validation_max(void)
-{
-       return rpki_config.thread_pool.validation.max;
-}
-
 void
 config_set_rsync_enabled(bool value)
 {

diff --git a/src/config.h b/src/config.h
index c7d353dbff21e5a0d0c6838fb5f503c4d95c9a37..0ef1c65d5bcee6ab2cd07e143c4c70a3a13d1619 100644 (file)
--- a/src/config.h
+++ b/src/config.h
@@ -54,7 +54,6 @@ char const *config_get_output_bgpsec(void);
 enum output_format config_get_output_format(void);
 unsigned int config_get_asn1_decode_max_stack(void);
 unsigned int config_get_thread_pool_server_max(void);
-unsigned int config_get_thread_pool_validation_max(void);
 
 /* Logging getters */
 bool config_get_op_log_enabled(void);

diff --git a/src/object/tal.c b/src/object/tal.c
index f0256adf2afa665d36a061d71e53eeffb5284efb..1a8fd3123655147b2685a8394d92f2ddcd0b74cc 100644 (file)
--- a/src/object/tal.c
+++ b/src/object/tal.c
@@ -37,6 +37,8 @@ struct tal {
 };
 
 struct validation_thread {
+       pthread_t pid;
+
        /* TAL file name */
        char *tal_file;
        /*
@@ -56,7 +58,6 @@ struct validation_thread {
 SLIST_HEAD(threads_list, validation_thread);
 
 struct tal_param {
-       struct thread_pool *pool;
        struct db_table *db;
        struct threads_list threads;
 };
@@ -539,10 +540,10 @@ end:      validation_destroy(state);
        return error;
 }
 
-static void
-do_file_validation(void *thread_arg)
+static void *
+do_file_validation(void *arg)
 {
-       struct validation_thread *thread = thread_arg;
+       struct validation_thread *thread = arg;
        struct tal *tal;
        int error;
 
@@ -582,6 +583,7 @@ destroy_tal:
 end:
        fnstack_cleanup();
        thread->exit_status = error;
+       return NULL;
 }
 
 static void
@@ -597,6 +599,7 @@ __do_file_validation(char const *tal_file, void *arg)
 {
        struct tal_param *t_param = arg;
        struct validation_thread *thread;
+       int error;
 
        thread = pmalloc(sizeof(struct validation_thread));
 
@@ -606,21 +609,27 @@ __do_file_validation(char const *tal_file, void *arg)
        thread->retry_local = true;
        thread->sync_files = true;
 
-       thread_pool_push(t_param->pool, thread->tal_file, do_file_validation,
-           thread);
+       error = pthread_create(&thread->pid, NULL, do_file_validation, thread);
+       if (error) {
+               pr_op_err("Could not spawn the file validation thread: %s",
+                   strerror(error));
+               free(thread->tal_file);
+               free(thread);
+               return error;
+       }
+
        SLIST_INSERT_HEAD(&t_param->threads, thread, next);
 
        return 0;
 }
 
 int
-perform_standalone_validation(struct thread_pool *pool, struct db_table *table)
+perform_standalone_validation(struct db_table *table)
 {
        struct tal_param param;
        struct validation_thread *thread;
        int error;
 
-       param.pool = pool;
        param.db = table;
        SLIST_INIT(&param.threads);
 
@@ -637,10 +646,12 @@ perform_standalone_validation(struct thread_pool *pool, struct db_table *table)
        }
 
        /* Wait for all */
-       thread_pool_wait(pool);
-
        while (!SLIST_EMPTY(&param.threads)) {
                thread = SLIST_FIRST(&param.threads);
+               error = pthread_join(thread->pid, NULL);
+               if (error)
+                       pr_crit("pthread_join() threw %d on the '%s' thread.",
+                           error, thread->tal_file);
                SLIST_REMOVE_HEAD(&param.threads, next);
                if (thread->exit_status) {
                        error = thread->exit_status;

diff --git a/src/object/tal.h b/src/object/tal.h
index f0f7b62821daac9e71759481e1fe61ca31119dc7..8f4d72b029642dcc58340cd7d7c15d47235a4faf 100644 (file)
--- a/src/object/tal.h
+++ b/src/object/tal.h
@@ -5,7 +5,6 @@
 
 #include "types/uri.h"
 #include "rtr/db/db_table.h"
-#include "thread/thread_pool.h"
 
 struct tal;
 
@@ -15,6 +14,6 @@ void tal_destroy(struct tal *);
 char const *tal_get_file_name(struct tal *);
 void tal_get_spki(struct tal *, unsigned char const **, size_t *);
 
-int perform_standalone_validation(struct thread_pool *, struct db_table *);
+int perform_standalone_validation(struct db_table *);
 
 #endif /* TAL_OBJECT_H_ */

diff --git a/src/rtr/db/vrps.c b/src/rtr/db/vrps.c
index 57abc26afc7b8f78b09bbe7051b01d9934d3d4c1..65a39f9fa6001813197a2b406083b3be7c6c5533 100644 (file)
--- a/src/rtr/db/vrps.c
+++ b/src/rtr/db/vrps.c
@@ -14,7 +14,6 @@
 #include "rtr/rtr.h"
 #include "rtr/db/db_table.h"
 #include "slurm/slurm_loader.h"
-#include "thread/thread_pool.h"
 #include "cache/local_cache.h"
 
 struct vrp_node {
@@ -78,9 +77,6 @@ struct state {
 
 static struct state state;
 
-/* Thread pool to use when the TALs will be processed */
-static struct thread_pool *pool;
-
 /** Protects @state.base, @state.deltas and @state.serial. */
 static pthread_rwlock_t state_lock;
 
@@ -108,12 +104,6 @@ vrps_init(void)
        time_t now;
        int error;
 
-       pool = NULL;
-       error = thread_pool_create("Validation",
-           config_get_thread_pool_validation_max(), &pool);
-       if (error)
-               return error;
-
        state.base = NULL;
        state.deltas = darray_create();
 
@@ -149,15 +139,12 @@ vrps_init(void)
 
 revert_deltas:
        darray_destroy(state.deltas);
-       thread_pool_destroy(pool);
        return error;
 }
 
 void
 vrps_destroy(void)
 {
-       thread_pool_destroy(pool);
-
        pthread_rwlock_destroy(&state_lock);
 
        if (state.slurm != NULL)
@@ -225,7 +212,7 @@ __perform_standalone_validation(struct db_table **result)
 
        db = db_table_create();
 
-       error = perform_standalone_validation(pool, db);
+       error = perform_standalone_validation(db);
        if (error) {
                db_table_destroy(db);
                return error;