remove deterministic load balancing, and revert to random

which means we have less magic knowledge in the interpreter, and
less recursive code.  At the cost of being less deterministic

diff --git a/src/lib/unlang/interpret.c b/src/lib/unlang/interpret.c
index 4881b49d5024126c677456b853886031804f05f3..ab93800ba98cff061466395a368eb22e631ea732 100644 (file)
--- a/src/lib/unlang/interpret.c
+++ b/src/lib/unlang/interpret.c
@@ -106,56 +106,6 @@ static void stack_dump(REQUEST *request)
 unlang_op_t unlang_ops[UNLANG_TYPE_MAX];
 
 
-/** Recursively collect active callers.  Slow, but correct
- *
- */
-uint64_t unlang_interpret_active_callers(unlang_t *instruction)
-{
-       uint64_t active_callers;
-       unlang_t *child;
-       unlang_group_t *g;
-
-       switch (instruction->type) {
-       default:
-               return 0;
-
-       case UNLANG_TYPE_MODULE:
-       {
-               module_thread_instance_t *thread;
-               unlang_module_t *sp;
-
-               sp = unlang_generic_to_module(instruction);
-               rad_assert(sp != NULL);
-
-               thread = module_thread(sp->module_instance);
-               rad_assert(thread != NULL);
-
-               return thread->active_callers;
-       }
-
-       case UNLANG_TYPE_GROUP:
-       case UNLANG_TYPE_LOAD_BALANCE:
-       case UNLANG_TYPE_REDUNDANT_LOAD_BALANCE:
-       case UNLANG_TYPE_IF:
-       case UNLANG_TYPE_ELSE:
-       case UNLANG_TYPE_ELSIF:
-       case UNLANG_TYPE_FOREACH:
-       case UNLANG_TYPE_SWITCH:
-       case UNLANG_TYPE_CASE:
-               g = unlang_generic_to_group(instruction);
-
-               active_callers = 0;
-               for (child = g->children;
-                    child != NULL;
-                    child = child->next) {
-                       active_callers += unlang_interpret_active_callers(child);
-               }
-               break;
-       }
-
-       return active_callers;
-}
-
 static inline void frame_state_init(unlang_stack_t *stack, unlang_stack_frame_t *frame)
 {
        unlang_t        *instruction = frame->instruction;

diff --git a/src/lib/unlang/load_balance.c b/src/lib/unlang/load_balance.c
index cfd6021feb407ecc271767fdcd731809b71a9449..37ee51899d928b7ef13118cd0d554a999e1c47e5 100644 (file)
--- a/src/lib/unlang/load_balance.c
+++ b/src/lib/unlang/load_balance.c
@@ -191,75 +191,29 @@ static unlang_action_t unlang_load_balance(REQUEST *request, rlm_rcode_t *presul
                }
 
        } else {
-               int num;
-               uint64_t lowest_active_callers;
-
        randomly_choose:
-               lowest_active_callers = ~(uint64_t ) 0;
+               count = 0;
 
                /*
                 *      Choose a child at random.
                 *
                 *      @todo - leverage the "power of 2", as per
-                *      lib/io/network.c.  This is good enough for
-                *      most purposes.  And, it avoids many calls to
-                *      active_callers(), which is recursive and slow.
+                *      lib/io/network.c.  This is good enough for
+                *      most purposes.  However, in order to do this,
+                *      we need to track active callers across
+                *      *either* multiple modules in one thread, *or*
+                *      across multiple threads.
+                *
+                *      We don't have thread-specific instance data
+                *      for this load-balance section.  So for now,
+                *      just pick a random child.
                 */
-               for (redundant->child = redundant->found = g->children, num = 0;
+               for (redundant->child = redundant->found = g->children;
                     redundant->child != NULL;
-                    redundant->child = redundant->child->next, num++) {
-                       uint64_t active_callers;
-                       unlang_t *child = redundant->child;
-
-                       if (child->type != UNLANG_TYPE_MODULE) {
-                               active_callers = unlang_interpret_active_callers(child);
-                               RDEBUG3("load-balance child %d sub-section has %" PRIu64 " active", num, active_callers);
-
-                       } else {
-                               module_thread_instance_t *thread;
-                               unlang_module_t *sp;
-
-                               sp = unlang_generic_to_module(child);
-                               rad_assert(sp != NULL);
-
-                               thread = module_thread(sp->module_instance);
-                               rad_assert(thread != NULL);
-
-                               active_callers = thread->active_callers;
-                               RDEBUG3("load-balance child %d sub-module has %" PRIu64 " active", num, active_callers);
-                       }
-
-
-                       /*
-                        *      Reset the found, and the count
-                        *      of children with this level of
-                        *      activity.
-                        */
-                       if (active_callers < lowest_active_callers) {
-                               RDEBUG3("load-balance choosing child %d as active %" PRIu64 " < %" PRIu64 "",
-                                       num, active_callers, lowest_active_callers);
-
-                               count = 1;
-                               lowest_active_callers = active_callers;
-                               redundant->found = redundant->child;
-                               continue;
-                       }
-
-                       /*
-                        *      Skip callers who are busier
-                        *      than the one we found.
-                        */
-                       if (active_callers > lowest_active_callers) {
-                               RDEBUG3("load-balance skipping child %d, as active %" PRIu64 " > %" PRIu64 "",
-                                       num, active_callers, lowest_active_callers);
-                               continue;
-                       }
-
+                    redundant->child = redundant->child->next) {
                        count++;
-                       RDEBUG3("load-balance found %d children with %" PRIu64 " active", count, active_callers);
 
-                       if ((count * (fr_rand() & 0xffff)) < (uint32_t) 0x10000) {
-                               RDEBUG3("load-balance choosing random child %d", num);
+                       if ((count * (fr_rand() & 0xffffff)) < (uint32_t) 0x1000000) {
                                redundant->found = redundant->child;
                        }
                }

diff --git a/src/lib/unlang/module.c b/src/lib/unlang/module.c
index aa0d898ab812d31a04f59d61d775640f788aa96a..74c40b30c1d09f1c5d284f857ceaecbeda080673 100644 (file)
--- a/src/lib/unlang/module.c
+++ b/src/lib/unlang/module.c
@@ -536,6 +536,16 @@ static void unlang_module_signal(REQUEST *request, fr_state_signal_t action)
        state->signal(sp->module_instance->dl_inst->data, state->thread->data, request,
                   state->rctx, action);
        request->module = caller;
+
+       /*
+        *      One fewer caller for this module.  Since this module
+        *      has been cancelled, decrement the active callers and
+        *      ignore any future signals.
+        */
+       if (action == FR_SIGNAL_CANCEL) {
+               state->thread->active_callers--;
+               state->signal = NULL;
+       }
 }
 
 static unlang_action_t unlang_module_resume(REQUEST *request, rlm_rcode_t *presult)
@@ -563,14 +573,17 @@ static unlang_action_t unlang_module_resume(REQUEST *request, rlm_rcode_t *presu
 
        /*
         *      It is now marked as "stop" when it wasn't before, we
-        *      must have been blocked.
+        *      must have been blocked.  We do NOT decrement
+        *      "active_callers" if it's already been cancelled in a
+        *      signal.
         */
        if (request->master_state == REQUEST_STOP_PROCESSING) {
                RWARN("Module %s became unblocked", sp->module_instance->module->name);
                if (state->presult) *state->presult = rcode;
 
+               if (state->signal) state->thread->active_callers--;
+
                *presult = rcode;
-               state->thread->active_callers--;
                return UNLANG_ACTION_STOP_PROCESSING;
        }
 

diff --git a/src/lib/unlang/unlang_priv.h b/src/lib/unlang/unlang_priv.h
index f8605e0e89084d5324ff0936aa320573e8170c52..140f4ff69a778838177a4f39e8824408073f1a2f 100644 (file)
--- a/src/lib/unlang/unlang_priv.h
+++ b/src/lib/unlang/unlang_priv.h
@@ -336,8 +336,6 @@ static inline unlang_t *unlang_xlat_inline_to_generic(unlang_xlat_inline_t *p)
  *
  * @{
  */
-uint64_t       unlang_interpret_active_callers(unlang_t *instruction);
-
 void           unlang_interpret_push(REQUEST *request, unlang_t *instruction,
                                      rlm_rcode_t default_rcode, bool do_next_sibling, bool top_frame);
 rlm_rcode_t    unlang_interpret_run(REQUEST *request);