From: Jérémie Galarneau Date: Thu, 23 Nov 2017 02:16:38 +0000 (-0500) Subject: Fix: evaluate trigger condition on registration X-Git-Tag: v2.11.0-rc1~425 X-Git-Url: https://git.lttng.org/?p=lttng-tools.git;a=commitdiff_plain;h=2ae99f0b1ce4b1ae352a7bf179b1f040111afd46 Fix: evaluate trigger condition on registration Since there is nothing preventing clients from subscribing to a condition before the corresponding trigger is registered, we have to evaluate this new condition right away. The current implementation is waiting for the next "evaluation" of conditions (e.g. on reception of a channel sample) to evaluate this newly registered trigger conditions, but this is broken. The reason it is broken is that waiting for the next sample does not allow us to properly handle transitions for edge-triggered conditions. Consider this example: when we handle a new channel sample, we evaluate each conditions twice: once with the previous state, and again with the newest state. We then use those two results to determine whether a state change happened: a condition was false and became true. If a state change happened, we have to notify clients. Now, if a client subscribes to a given notification and registers a trigger *after* that subscription, we have to make sure the condition is evaluated at this point while considering only the current state. Otherwise, the next evaluation cycle may only see that the evaluations remain the same (true for samples n-1 and n) and the client will never know that the condition has been met. Signed-off-by: Jérémie Galarneau --- diff --git a/src/bin/lttng-sessiond/notification-thread-events.c b/src/bin/lttng-sessiond/notification-thread-events.c index 1793cac6b..6ba6a2e60 100644 --- a/src/bin/lttng-sessiond/notification-thread-events.c +++ b/src/bin/lttng-sessiond/notification-thread-events.c @@ -416,7 +416,7 @@ int evaluate_condition_for_client(struct lttng_trigger *trigger, assert(current_condition); if (!lttng_condition_is_equal(condition, - current_condition)) { + current_condition)) { continue; } @@ -555,12 +555,22 @@ int notification_thread_client_subscribe(struct notification_client *client, &iter); node = cds_lfht_iter_get_node(&iter); if (!node) { + /* + * No notification-emiting trigger registered with this + * condition. We don't evaluate the condition right away + * since this trigger is not registered yet. + */ free(client_list_element); goto end_unlock; } client_list = caa_container_of(node, struct notification_client_list, notification_trigger_ht_node); + /* + * The condition to which the client just subscribed is evaluated + * at this point so that conditions that are already TRUE result + * in a notification being sent out. + */ if (evaluate_condition_for_client(client_list->trigger, condition, client, state)) { WARN("[notification-thread] Evaluation of a condition on client subscription failed, aborting."); @@ -1104,11 +1114,6 @@ int handle_notification_thread_command_register_trigger( cds_lfht_add(state->notification_trigger_clients_ht, lttng_condition_hash(condition), &client_list->notification_trigger_ht_node); - /* - * Client list ownership transferred to the - * notification_trigger_clients_ht. - */ - client_list = NULL; /* * Add the trigger to list of triggers bound to the channels currently @@ -1147,6 +1152,47 @@ int handle_notification_thread_command_register_trigger( break; } + /* + * Since there is nothing preventing clients from subscribing to a + * condition before the corresponding trigger is registered, we have + * to evaluate this new condition right away. + * + * At some point, we were waiting for the next "evaluation" (e.g. on + * reception of a channel sample) to evaluate this new condition, but + * that was broken. + * + * The reason it was broken is that waiting for the next sample + * does not allow us to properly handle transitions for edge-triggered + * conditions. + * + * Consider this example: when we handle a new channel sample, we + * evaluate each conditions twice: once with the previous state, and + * again with the newest state. We then use those two results to + * determine whether a state change happened: a condition was false and + * became true. If a state change happened, we have to notify clients. + * + * Now, if a client subscribes to a given notification and registers + * a trigger *after* that subscription, we have to make sure the + * condition is evaluated at this point while considering only the + * current state. Otherwise, the next evaluation cycle may only see + * that the evaluations remain the same (true for samples n-1 and n) and + * the client will never know that the condition has been met. + */ + cds_list_for_each_entry_safe(client_list_element, tmp, + &client_list->list, node) { + ret = evaluate_condition_for_client(trigger, condition, + client_list_element->client, state); + if (ret) { + goto error_free_client_list; + } + } + + /* + * Client list ownership transferred to the + * notification_trigger_clients_ht. + */ + client_list = NULL; + *cmd_result = LTTNG_OK; error_free_client_list: if (client_list) { diff --git a/src/bin/lttng-sessiond/notification-thread.h b/src/bin/lttng-sessiond/notification-thread.h index 2aa76e71c..e59f74f62 100644 --- a/src/bin/lttng-sessiond/notification-thread.h +++ b/src/bin/lttng-sessiond/notification-thread.h @@ -127,6 +127,8 @@ struct notification_thread_handle { * notification_trigger_clients_ht, * - add trigger to channel_triggers_ht (if applicable), * - add trigger to triggers_ht + * - evaluate the trigger's condition right away to react if that condition + * is true from the beginning. * * 4) Unregistration of a trigger * - if the trigger's action is of type "notify", @@ -153,6 +155,8 @@ struct notification_thread_handle { * - Add the condition to the client's list of subscribed conditions, * - Look-up notification_trigger_clients_ht and add the client to * list of clients. + * - Evaluate the condition for the client that subscribed if the trigger + * was already registered. * * 9) Unsubscription of a client to a condition's notifications * - Remove the condition from the client's list of subscribed conditions,