Mid: attrd: Support of the dampen change by attrd.

[HideoYamauchi]

Hi All,

I added the support of the change of dampen by attrd and attrd_updater.
I added a function to change the value only for dampen and a function to change the combination of dampen and value.

The command-line of attrd_updater is as follows.

1)Change the combination of dampen and value.
attrd_updater -B value -n name -d dampen

2)Change the value only for dampen.
attrd_updater -Y -n name -d dampen

It is necessary to discuss the next matter.

• Mid: attrd: Support of the change of the delay(dampen). #906
• Mid: attrd: Support of the dampen change by attrd. #946

Best Regards,
Hideo Yamacuhi.

[kgaillot]

Hi Hideo,

This block of changes is unnecessary, because we've already returned before this point if op is ATTRD_OP_UPDATE_DELAY.

Everything else looks great, thanks.

[HideoYamauchi]

Hi Ken,

Thank you for comment.

(snip)
        if (safe_str_eq(op, ATTRD_OP_UPDATE) || safe_str_eq(op, ATTRD_OP_UPDATE_BOTH)) {
            crm_info("Setting %s[%s]: %s -> %s from %s", attr, host, v->current, value, peer->uname);
            free(v->current);
            if(value) {
                v->current = strdup(value);
            } else {
                v->current = NULL;
            }
            changed = TRUE;
        } else {
             crm_trace("Unchanged %s[%s] from %s.(ATTRD_OP_UPDATE_DELAY)", attr, host, peer->uname);
        }
(snip)

Because I have the next problem, I think that this judgment is necessary.

In the case of Y option, the value of the attribute is sent to attrd_peer_update in NULL.
It is not handled in write_or_elect_attribute when a dampen level is the same value.

If there is not this judgment, the attribute level is updated in NULL when I operated the next.

• I add an attribute by attrd_updater command.

[root@rh72-01]# attrd_updater -n test2 -v 5 -d 10

[root@rh72-01]# crm_mon -1 -Af
(snip)
2 nodes and 0 resources configured

Node rh72-02: UNCLEAN (offline)
Online: [ rh72-01 ]


Node Attributes:
* Node rh72-01:
    + test2                             : 5 
(snip)

• I appoint the same value in dampen.　(The value of the attribute becomes NULL.)

[root@rh72-01]# attrd_updater -n test2 -Y -d 10

[root@rh72-01]# crm_mon -1 -Af
(snip)
2 nodes and 0 resources configured

Node rh72-02: UNCLEAN (offline)
Online: [ rh72-01 ]


Node Attributes:
* Node rh72-01:
(snip)

This judgment may cause confusion later.
Should I attach the following comment by this correction?

(snip)
        } else {
             /* When a delay level does not change in ATTRD_OP_UPDATE_DELAY, the attribute level becomes NULL. */
             /* In this case it is necessary to ignore the attribute level. */
             crm_trace("Unchanged %s[%s] from %s.(ATTRD_OP_UPDATE_DELAY)", attr, host, peer->uname);
        }
(snip)

How do you think?

Best Regards,
Hideo Yamauchi.

[HideoYamauchi]

Hi Ken,

Thank you for comment.

I reflect your comment, and this patch revises next three places.

• Change a judgment.(I delete "else".)

(snip)
    attribute_t *a = g_hash_table_lookup(attributes, attr);

    if(a == NULL) {
        if (safe_str_eq(op, ATTRD_OP_UPDATE) || safe_str_eq(op, ATTRD_OP_UPDATE_BOTH)) {
            a = create_attribute(xml);
        } else {
            crm_warn("Update error (attribute %s not found)", attr);
            return;
        }
    }

    if (safe_str_eq(op, ATTRD_OP_UPDATE_BOTH) || safe_str_eq(op, ATTRD_OP_UPDATE_DELAY)) {
(snip)

• When a delay level does not have a change in ATTRD_OP_UPDATE_DELAY, attrd_peer_update returns.

(snip)
                if (dampen >= 0) {
                    if (a->timeout_ms != dampen) {
(snip)
                        //if dampen is changed, attrd writes in a current value immediately.
                        write_or_elect_attribute(a);
                        if (safe_str_eq(op, ATTRD_OP_UPDATE_DELAY)) {
                            return;
                        }
                    } else {
                        if (safe_str_eq(op, ATTRD_OP_UPDATE_DELAY)) {
                            crm_trace("Unchanged attribute %s with delay %dms (%s).(ATTRD_OP_UPDATE_DELAY)", a->id, dampen, dvalue);
                            return;
                        }
                    }
                } else {
                    crm_warn("Update error (A positive number is necessary for delay parameter. attribute %s : %dms (%s))", a->id, dampen, dvalue);
                    return;
                }
(snip)

• By these corrections, I delete the judgment that became unnecessary. ATTRD_OP_UPDATE_DELAY does not arrive at here.

(snip)
    } else if(safe_str_neq(v->current, value)) {
        crm_info("Setting %s[%s]: %s -> %s from %s", attr, host, v->current, value, peer->uname);
        free(v->current);
        if(value) {
            v->current = strdup(value);
        } else {
            v->current = NULL;
        }
        changed = TRUE;
    } else {
(snip)

Is my understanding wrong?

Best Regards,
Hideo Yamauchi.

[kgaillot]

Hi Hideo,

Your suggestions sound exactly correct.

[HideoYamauchi]

Hi Ken,

I reflected the last correction to a patch.
If this patch does not have a problem, please merge it.

Many Thanks!
Hideo Yamauchi.

[kgaillot]

Hi Hideo,

It looks great. Many thanks for all the effort you put into this.

[HideoYamauchi]

Hi Ken,

Thanks!

Hideo Yamauchi.