policies: implement asc/desc modifier for sort order

When defining parameter "lru_sort_attr" or "default_lru_sort_attr",
a modifier 'asc' or 'desc' can now be specified, e.g.
	lru_sort_attr = size(desc);
	lru_sort_attr = mtime(asc);

Default if no modifier is specified is 'asc' i.e. from smallest to
largest for sizes, and from oldest to newest for times.

Change-Id: I674eaf5062e9b69d544a80b6f38f477302b40215
Signed-off-by: Gael Delbary <[email protected]>
Signed-off-by: Thomas Leibovici <[email protected]>
Reviewed-on: https://review.gerrithub.io/c/cea-hpc/robinhood/+/523438
Reviewed-by: Sebastien Gougeaud <[email protected]>

src/include/list_mgr.h

@@ -531,6 +531,13 @@ typedef struct lmgr_simple_filter_t {
     unsigned int         prealloc;
 } lmgr_simple_filter_t;
 
+/** Sort types */
+typedef enum {
+    SORT_NONE, /**< no sorting */
+    SORT_ASC,  /**< sort from lower value to higher */
+    SORT_DESC  /**< sort from higher value to lower */
+} sort_order_t;
+
 /* needed here for defining filters, obj_type_t... */
 #include "policy_rules.h"
 
@@ -579,13 +586,6 @@ typedef struct lmgr_filter_t {
 #define filter_simple   filter_u.simple_filter
 #define filter_boolexpr filter_u.boolean_expr
 
-/** Sort types */
-typedef enum {
-    SORT_NONE, /**< no sorting */
-    SORT_ASC,  /**< sort from lower value to higher */
-    SORT_DESC  /**< sort from higher value to lower */
-} sort_order_t;
-
 /** specifies result order */
 typedef struct lmgr_sort_type_t {
     unsigned int    attr_index;
@@ -1293,6 +1293,9 @@ bool cond2sql_ok(bool_node_t *boolexpr,
                       const struct sm_instance *smi,
                       const struct time_modifier *time_mod);
 
+/* return a sort_order_t or a negative value on error */
+int str2sort_order(const char *str);
+
 #endif
 
 /** @} */

src/include/policy_rules.h

@@ -21,8 +21,8 @@
 #define _POLICIES_H
 
 #include "rbh_boolexpr.h"
-#include "list_mgr.h"
 #include "rbh_params.h"
+#include "list_mgr.h"
 #include <sys/time.h>
 
 /** whitelist item is just a boolean expression */
@@ -187,6 +187,8 @@ typedef struct policy_descr_t {
     /* attr index of the sort order (e.g. last_mod, creation_time, ...) */
     /* default value for policy_run_config_t.lru_sort_attr */
     unsigned int        default_lru_sort_attr;
+    /* default value for policy_run_config_t.lru_sort_order */
+    sort_order_t        default_lru_sort_order;
 
     policy_rules_t      rules;
 

src/include/policy_run.h

@@ -187,6 +187,8 @@ typedef struct policy_run_config_t {
     /** attr index of the sort order (e.g. last_mod, creation_time, ...).
      * overrides default_lru_sort_attr (from policy descr). */
     unsigned int        lru_sort_attr;
+    /* overrides default_lru_sort_order */
+    sort_order_t        lru_sort_order;
 
     /** if specified, overrides default_action from the policy descriptor.
      * Can then be overriden by rules. */

src/include/rbh_boolexpr.h

@@ -89,7 +89,7 @@ compare_criteria_t str2criteria(const char *str, const struct sm_instance *smi,
 #define LRU_ATTR_INVAL (ATTR_INDEX_FLG_UNSPEC | 0x1)
 
 #define ALLOWED_LRU_ATTRS_STR "none, creation, last_access, last_mod, "\
-                              "rm_time, or status manager specific."
+                              "rm_time, size, or status manager specific."
 
 /**
  * Return the attribute index for the given lru_sort_attr string.

src/list_mgr/listmgr_common.c

@@ -2210,3 +2210,12 @@ void attr_mask_unset_readonly(attr_mask_t *mask)
 {
     *mask = attr_mask_and_not(mask, &readonly_attr_set);
 }
+
+int str2sort_order(const char *str)
+{
+    if (!strcasecmp(str, "ASC"))
+        return SORT_ASC;
+    if (!strcasecmp(str, "DESC"))
+        return SORT_DESC;
+    return -1;
+}

src/list_mgr/listmgr_iterators.c

@@ -146,7 +146,8 @@ static inline void check_sort(const lmgr_sort_type_t *p_sort_type,
     *sort_dirattr = ATTR_INDEX_FLG_UNSPEC;
 
     /* is there a sort order ? */
-    if (p_sort_type == NULL || p_sort_type->order == SORT_NONE)
+    if (p_sort_type == NULL || p_sort_type->order == SORT_NONE
+        || ((p_sort_type->attr_index & ATTR_INDEX_FLG_UNSPEC) != 0))
         return;
 
     /* check sort order */

src/policies/policy_loader.c

@@ -379,16 +379,38 @@ static int parse_policy_decl(config_item_t config_blk, const char *block_name,
     }
 
     /* smi must be set to call str2lru_attr */
+    extra = NULL;
+    extra_cnt = 0;
     rc = GetStringParam(config_blk, block_name, "default_lru_sort_attr",
                         PFLG_NO_WILDCARDS | PFLG_MANDATORY, tmpstr,
-                        sizeof(tmpstr), NULL, NULL, msg_out);
+                        sizeof(tmpstr), &extra, &extra_cnt, msg_out);
     if (rc)
         return rc;
-    /* is it a time attribute? */
+
+    /* check extra parameter (allowed values are "asc" or "desc"). */
+    if (extra_cnt > 1) {
+        sprintf(msg_out, "Too many parameters found for default_lru_sort_attr = "
+                         " '%s' in block '%s': '(asc)' or '(desc)' expected",
+                         tmpstr, block_name);
+        return EINVAL;
+    } else if (extra_cnt == 0) {
+        /* Default to "asc" if no parameter is specified. */
+        policy->default_lru_sort_order = SORT_ASC;
+    } else {
+        rc = str2sort_order(extra[0]);
+        if (rc < 0) {
+            sprintf(msg_out, "Invalid sort order '%s' in block '%s':"
+                    " 'asc' or 'desc' expected", extra[0], block_name);
+            return EINVAL;
+        }
+        policy->default_lru_sort_order = rc;
+    }
+
+    /* is it a supported attribute? */
     rc = str2lru_attr(tmpstr, policy->status_mgr);
     if (rc == LRU_ATTR_INVAL) {
-        strcpy(msg_out, "time attribute expected for 'default_lru_sort_attr': "
-               ALLOWED_LRU_ATTRS_STR "...");
+        strcpy(msg_out, "Attribute not supported for 'default_lru_sort_attr': "
+               "Expected: "ALLOWED_LRU_ATTRS_STR "...");
         return EINVAL;
     } else
         policy->default_lru_sort_attr = rc;

src/policies/policy_run.c

@@ -1006,14 +1006,31 @@ static void set_ignore_filters(policy_info_t *policy, lmgr_filter_t *filter)
     }
 }
 
+/**
+ * Return the required comparator to filter next entries, depending on the
+ * sort order.
+ */
+static filter_comparator_t policy_order_to_listmgr_comp(sort_order_t order)
+{
+    switch (order) {
+    case SORT_ASC:
+        return MORETHAN;
+    case SORT_DESC:
+        return LESSTHAN;
+    default:
+        RBH_BUG("Invalid policy order");
+    }
+}
+
 /**
  * Add time filter based on sort order and last checked entry
  */
-static void set_time_optim_filter(policy_info_t *policy, lmgr_filter_t *filter)
+static void set_optim_filter(policy_info_t *policy, lmgr_filter_t *filter)
 {
     filter_value_t fval;
     char datestr[128];
     struct tm ts;
+    const char *sort_char;
 
     /* no sort order or no previous filter: do nothing */
     if (policy->config->lru_sort_attr == LRU_ATTR_NONE
@@ -1025,12 +1042,21 @@ static void set_time_optim_filter(policy_info_t *policy, lmgr_filter_t *filter)
      * restart from initial file when no migration could be done. */
     fval.value.val_uint = policy->first_eligible;
     lmgr_simple_filter_add(filter, policy->config->lru_sort_attr,
-                           MORETHAN, fval, 0);
-    strftime(datestr, 128, "%Y/%m/%d %T",
-             localtime_r(&policy->first_eligible, &ts));
+                           policy_order_to_listmgr_comp(
+                                policy->config->lru_sort_order),
+                           fval, 0);
+
+    sort_char = policy->config->lru_sort_order == SORT_ASC ? ">=" : "<=";
+
+    if (policy->config->lru_sort_attr == ATTR_INDEX_size)
+        snprintf(datestr, 128, "%lu", policy->first_eligible);
+    else
+        strftime(datestr, 128, "%Y/%m/%d %T",
+                 localtime_r(&policy->first_eligible, &ts));
+
     DisplayLog(LVL_EVENT, tag(policy),
-               "Optimization: considering entries with %s newer than %s",
-               sort_attr_name(policy), datestr);
+               "Optimization: considering entries with %s %s %s",
+               sort_attr_name(policy), sort_char, datestr);
 }
 
 /**
@@ -1670,19 +1696,26 @@ static pass_status_e fill_workers_queue(policy_info_t *pol,
 
             /* filter on <sort_time> */
             if (pol->config->lru_sort_attr != LRU_ATTR_NONE) {
+                const char * sort_char;
+
                 fval.value.val_int = *last_sort_time;
                 rc = lmgr_simple_filter_add_or_replace(filter,
                                                pol->config->lru_sort_attr,
-                                               MORETHAN, fval,
-                                               FILTER_FLAG_ALLOW_NULL);
+                                               policy_order_to_listmgr_comp(
+                                                   pol->config->lru_sort_order),
+                                               fval, FILTER_FLAG_ALLOW_NULL);
                 if (rc)
                     return PASS_ERROR;
 
+                sort_char = pol->config->lru_sort_order == SORT_ASC ? ">=" :
+                                                                      "<=";
+
                 DisplayLog(LVL_DEBUG, tag(pol),
                            "Performing new request with a limit of %u entries"
-                           " and %s >= %d and md_update < %ld ",
+                           " and %s %s %d and md_update < %ld ",
                            req_opt->list_count_max, sort_attr_name(pol),
-                           *last_sort_time, pol->progress.policy_start);
+                           sort_char, *last_sort_time,
+                           pol->progress.policy_start);
             } else {
                 DisplayLog(LVL_DEBUG, tag(pol),
                            "Performing new request with a limit of %u entries"
@@ -1934,14 +1967,8 @@ int run_policy(policy_info_t *p_pol_info, const policy_param_t *p_param,
     attr_mask = db_attr_mask(p_pol_info, p_param);
 
     /* sort by last access */
-    // TODO manage random
     sort_type.attr_index = p_pol_info->config->lru_sort_attr;
-    /* entries are sorted:
-     * - by size, largest first;
-     * - by time, oldest first. */
-    sort_type.order = p_pol_info->config->lru_sort_attr == LRU_ATTR_NONE ?
-        SORT_NONE : p_pol_info->config->lru_sort_attr == ATTR_INDEX_size ?
-        SORT_DESC : SORT_ASC;
+    sort_type.order = p_pol_info->config->lru_sort_order;
 
     rc = lmgr_simple_filter_init(&filter);
     if (rc)
@@ -1976,7 +2003,7 @@ int run_policy(policy_info_t *p_pol_info, const policy_param_t *p_param,
         set_rule_filters(p_pol_info, &filter);
     }
 
-    set_time_optim_filter(p_pol_info, &filter);
+    set_optim_filter(p_pol_info, &filter);
 
     /* Do not retrieve all entries at once, as the result may exceed
      * the client memory! */
@@ -2888,6 +2915,16 @@ static void run_sched_cb(void *udata, sched_status_e st)
     }
 }
 
+static void update_first_eligible(policy_info_t *pol, int val)
+{
+    if ((!pol->first_eligible) ||
+        (pol->config->lru_sort_order == SORT_ASC
+         && val < pol->first_eligible) ||
+        (pol->config->lru_sort_order == SORT_DESC
+         && val > pol->first_eligible))
+        pol->first_eligible = val;
+}
+
 /**
 * Manage an entry by path or by fid, depending on FS
 */
@@ -2938,8 +2975,8 @@ static void process_entry(policy_info_t *pol, lmgr_t *lmgr,
 
     /* it is the first matching entry? */
     rc = get_sort_attr(pol, &p_item->entry_attr);
-    if (rc != -1 && (!pol->first_eligible || (rc < pol->first_eligible)))
-        pol->first_eligible = rc;
+    if (rc != -1)
+        update_first_eligible(pol, rc);
 
     ectx->time_save = rc;
 

src/policies/policy_run_cfg.c

@@ -56,6 +56,7 @@ static int polrun_set_default(const policy_descr_t *pol,
 
     /* attr index of the sort order (e.g. last_mod, creation_time, ...) */
     cfg->lru_sort_attr = pol->default_lru_sort_attr;
+    cfg->lru_sort_order = pol->default_lru_sort_order;
 
     cfg->action = pol->default_action;
     cfg->action_params.param_set = NULL;
@@ -777,9 +778,11 @@ static int polrun_read_config(config_file_t config, const char *policy_name,
 
     /* read specific parameters */
 
+    extra = NULL;
+    extra_cnt = 0;
     /* 'lru_sort_attr' overrides 'default_lru_sort_attr' from 'define_policy' */
     rc = GetStringParam(param_block, block_name, "lru_sort_attr",
-                        PFLG_NO_WILDCARDS, tmp, sizeof(tmp), NULL, NULL,
+                        PFLG_NO_WILDCARDS, tmp, sizeof(tmp), &extra, &extra_cnt,
                         msg_out);
     if ((rc != 0) && (rc != ENOENT))
         return rc;
@@ -792,6 +795,24 @@ static int polrun_read_config(config_file_t config, const char *policy_name,
             return EINVAL;
         }
         conf->lru_sort_attr = rc;
+
+        /* check extra parameter (allowed values are "asc" or "desc"). */
+        /* Leave unchanged (default to "asc") if no parameter is specified. */
+        if (extra_cnt > 1) {
+            sprintf(msg_out, "Too many parameters found for lru_sort_attr = "
+                             " '%s' in block '%s': '(asc)' or '(desc)' expected",
+                             tmp, block_name);
+            return EINVAL;
+        }
+        if (extra_cnt == 1) {
+            rc = str2sort_order(extra[0]);
+            if (rc < 0) {
+                sprintf(msg_out, "Invalid sort order '%s' in block '%s':"
+                        " 'asc' or 'desc' expected", extra[0], block_name);
+                return EINVAL;
+            }
+            conf->lru_sort_order = rc;
+        }
     }
 
     /* 'action' overrides 'default_action' from 'define_policy' */

tests/test_suite/2-run-tests.sh

@@ -1004,18 +1004,20 @@ function test_lru_policy
     # 15s  | access        |      x           x   x       x
     # 20s  | 1st archive   |  $expected_migr_1
     # +$4  | 2nd archive   |  $expected_migr_2
+    # size (M)             |  1   2   3+  4+  5+  1   2+  3+  4   5
+    #			desc=	4 9 3 8 2=7 6 1
 
 	echo "1-Creating test files..."
     # creation times
 	echo -n "  Creating files 0 1 2 3, "
 	for i in {0..3}; do
-		dd if=/dev/zero of=$RH_ROOT/file.$i bs=1M count=$((1+i%3)) >/dev/null 2>/dev/null || error "writing file.$i"
+		dd if=/dev/zero of=$RH_ROOT/file.$i bs=1M count=$((1+i%5)) >/dev/null 2>/dev/null || error "writing file.$i"
 	done
 	echo "sleeping $cr_sleep seconds..."
     sleep $cr_sleep
 	echo -n "  Creating files 4 5 6 7 8 9, "
 	for i in {4..9}; do
-		dd if=/dev/zero of=$RH_ROOT/file.$i bs=1M count=$((1+i%3)) >/dev/null 2>/dev/null || error "writing file.$i"
+		dd if=/dev/zero of=$RH_ROOT/file.$i bs=1M count=$((1+i%5)) >/dev/null 2>/dev/null || error "writing file.$i"
 	done
 	echo "sleeping $cr_sleep seconds..."
     sleep $cr_sleep
@@ -13435,7 +13437,8 @@ run_test 220c test_lru_policy lru_sort_mod_2pass.conf "" "0 1 2 3 4 5 6 7 8 9" 3
 run_test 220d test_lru_policy lru_sort_access.conf "" "0 2 3 6 8 9" 20 "lru sort on last_access"
 run_test 220e test_lru_policy lru_sort_archive.conf "0 1 2 3 4 5 6 7 8 9" "" 15 "lru sort on last_archive"
 run_test 220f test_lru_policy lru_sort_creat_last_arch.conf "0 1 2 3" "4 5 6 7 8 9" 10 "lru sort on creation and last_archive==0"
-run_test 220g test_lru_policy lru_sort_size.conf "2 5 8" "1 4 7" 10 "lru sort on size"
+run_test 220g test_lru_policy lru_sort_size_desc.conf "3 4 8 9" "1 2 6 7" 10 "lru sort on size"
+run_test 220h test_lru_policy lru_sort_size_asc.conf "1 2 6 7" "3 4 8 9" 10 "lru sort on size"
 run_test 221  test_suspend_on_error migr_fail.conf  2 "suspend migration if too many errors"
 run_test 222  test_custom_purge test_custom_purge.conf 2 "custom purge command"
 run_test 223  test_default test_default_case.conf "ignore entries if no default case is specified"

tests/test_suite/cfg/lru_sort_size.conf → tests/test_suite/cfg/lru_sort_size_asc.conf

@@ -13,5 +13,7 @@ migration_parameters {
 	# set a small result size to check request continuation
 	db_result_size_max = 3;
 
-	lru_sort_attr = size;
+	max_action_count = 4;
+
+	lru_sort_attr = size(asc);
 }

tests/test_suite/cfg/lru_sort_size_desc.conf

@@ -0,0 +1,19 @@
+%include "common.conf"
+
+migration_rules
+{
+	policy default { condition { size >= 2M } }
+}
+
+migration_parameters {
+	# serialize processing to make the check easy in test output
+	nb_threads = 1;
+	queue_size = 1;
+
+	# set a small result size to check request continuation
+	db_result_size_max = 3;
+
+	max_action_count = 4;
+
+	lru_sort_attr = size(desc);
+}