Modified patch for RA

Hi Dejan, here's another modified patch for the mysql agent of the commit version 4c18035 (git [at] github:y-trudeau/resource-agents.git branch mysql-repl). Following a comment of Keisuke, I put back the log level for mysql_status in probe mode.

On Fri, May 04, 2012 at 04:29:34PM -0400, Yves Trudeau wrote: > Hi Dejan, > here's another modified patch for the mysql agent of the commit > version 4c18035 (git [at] github:y-trudeau/resource-agents.git branch > mysql-repl). Following a comment of Keisuke, I put back the log level > for mysql_status in probe mode.

Cool!

So, according to the discussion at github, you insist that the replication IP needs to be specified in the node's static attributes. I'm not very happy with it, because it is a precedence, but I won't insist on it. Let's hope for the best.

There are a few concerns which went unanswered so far:

1a. In case this attribute is not set, would replication fail properly (with an informative error message) or misbehave? It is not explicitely checked for existence in the code.

1b. Mori-san suggested to make this attribute optional and in case it doesn't exist just to use uname. That sounds like a good idea to me.

2. Is it possible/plausible to have more than one mysql instance? If so, then the attribute name should include the instance name. Say ${INSTANCE_NAME}_mysql_replication_IP or something to that extent. Also, it would make for a better looking configuration. "IP" doesn't really say much.

3. This attribute is part of the configuration and supposed to be setup by the user. Please document that in the meta-data.

Le 2012-05-08 11:15, Dejan Muhamedagic a écrit : > Hi Yves, > > On Fri, May 04, 2012 at 04:29:34PM -0400, Yves Trudeau wrote: >> Hi Dejan, >> here's another modified patch for the mysql agent of the commit >> version 4c18035 (git [at] github:y-trudeau/resource-agents.git branch >> mysql-repl). Following a comment of Keisuke, I put back the log level >> for mysql_status in probe mode. > > Cool! > > So, according to the discussion at github, you insist that the > replication IP needs to be specified in the node's static > attributes. I'm not very happy with it, because it is a > precedence, but I won't insist on it. Let's hope for the best.

Like we discussed today, I'll add a fallback to 'uname -n' if the IP attribute is not present for the node.

> > There are a few concerns which went unanswered so far: > > 1a. In case this attribute is not set, would replication fail > properly (with an informative error message) or misbehave? It is > not explicitely checked for existence in the code. > > 1b. Mori-san suggested to make this attribute optional and in > case it doesn't exist just to use uname. That sounds like a good > idea to me. >

Agree

> 2. Is it possible/plausible to have more than one mysql > instance? If so, then the attribute name should include the > instance name. Say ${INSTANCE_NAME}_mysql_replication_IP or > something to that extent. Also, it would make for a better > looking configuration. "IP" doesn't really say much.

Yes, I just added that code for a customer. I'll use ${INSTANCE_NAME}_replication_info.

> > 3. This attribute is part of the configuration and supposed to > be setup by the user. Please document that in the meta-data. > > Cheers, > > Dejan > > P.S. Any chance of finishing this by Friday?

Hi Dejan, here's another modified patch for the mysql agent of the commit version 4c18035 (git [at] github:y-trudeau/resource-agents.git branch mysql-repl). This patch implements fallback on uname -n if the node IP attribute is not present and uses the instance name for the replication info attribute. I am also working with Raoul to get me back on track with git.

On Thu, May 10, 2012 at 05:06:25PM -0400, Yves Trudeau wrote: > Hi Dejan, > here's another modified patch for the mysql agent of the commit > version 4c18035 (git [at] github:y-trudeau/resource-agents.git branch > mysql-repl). This patch implements fallback on uname -n if the node > IP attribute is not present and uses the instance name for the > replication info attribute.

Hmm, it looks like there was a misunderstanding here. The attribute named "IP" is still named "IP" :)

On Fri, May 11, 2012 at 08:45:06AM -0400, Yves Trudeau wrote: > Hi Dejan, > I changed the name of the attribute to REPL_MASTER_IP

Let's quote from one of previous emails:

2. Is it possible/plausible to have more than one mysql instance? If so, then the attribute name should include the instance name. Say ${INSTANCE_NAME}_mysql_replication_IP or something to that extent. Also, it would make for a better looking configuration. "IP" doesn't really say much.

> and added 2 > lines of comment for the get_local_ip function.

Again, a quote:

3. This attribute is part of the configuration and supposed to be setup by the user. Please document that in the meta-data.

Note that one of the key words here is "meta-data." That is supposed to be documentation for the users, not for developers. Users don't normally read the code.

> Is that inline with > what you want?

OT:

It doesn't really matter what _I_ want. We're having a discussion here on how to improve the feature. It is just by chance that I am right now the only one talking about it.

Hi Dejan, ok, here the latest version using ${INSTANCE_ATTR_NAME}_MYSQL_MASTER_IP and I agree with the merits of this :) I added a paragraph explaining the use of the attribute in the longdesc of the meta-data.

> >> and I agree with the merits of >> this :) I added a paragraph explaining the use of the attribute in >> the longdesc of the meta-data. > > Excellent! Looks good to me. If nobody objects, we can push this > come Monday. > > BTW, on what repository is this based? Can you produce a set of > patches to be applied to upstream?

set of patches... here's _one_ patch bringing the latest commit of ClusterLabs/resource-agents to my version. I also lowered the case of the attribute.

On Fri, May 11, 2012 at 11:27:23AM -0400, Yves Trudeau wrote: > Hi Dejan, > > Le 2012-05-11 11:04, Dejan Muhamedagic a écrit : >> Hi, >> >> On Fri, May 11, 2012 at 10:25:19AM -0400, Yves Trudeau wrote: >>> Hi Dejan, >>> ok, here the latest version using >>> ${INSTANCE_ATTR_NAME}_MYSQL_MASTER_IP >> >> I'd vote for less "yelling" as cluster configurations are mostly >> lower case. > > :) Ok, never thought about the impact of the upper case... > >> >>> and I agree with the merits of >>> this :) I added a paragraph explaining the use of the attribute in >>> the longdesc of the meta-data. >> >> Excellent! Looks good to me. If nobody objects, we can push this >> come Monday. >> >> BTW, on what repository is this based? Can you produce a set of >> patches to be applied to upstream? > > set of patches... here's _one_ patch bringing the latest commit of > ClusterLabs/resource-agents to my version. I also lowered the case of > the attribute.

Good!

One thing I forgot though: Can you also provide a commit message, i.e. a patch description (what changed, what's new, etc).

Hi Raoul and Dejan, I completely forgot about this one but I am wondering about the impacts. I have many setups in production and none reported any problem related to this. The fix pretty easy though, Dejan, is it too late to submit a patch?

On Fri, May 18, 2012 at 10:08:54AM -0400, Yves Trudeau wrote: > Hi Raoul and Dejan, > I completely forgot about this one but I am wondering about the > impacts. I have many setups in production and none reported any > problem related to this. The fix pretty easy though, Dejan, is it > too late to submit a patch?

Hi Dejan, here's the patch for Raoul comments. The patch is over git://github.com/ClusterLabs/resource-agents.git commit bc1991fd0e

commit message:

Better reset slave handling and cleanup for get_slave_info

Regards,

Yves

Le 2012-05-18 11:13, Dejan Muhamedagic a écrit : > On Fri, May 18, 2012 at 10:08:54AM -0400, Yves Trudeau wrote: >> Hi Raoul and Dejan, >> I completely forgot about this one but I am wondering about the >> impacts. I have many setups in production and none reported any >> problem related to this. The fix pretty easy though, Dejan, is it >> too late to submit a patch? > > No, if the patch is fine. Up to you. It's very mysql-specific, > cannot offer much help. > _______________________________________________________ > Linux-HA-Dev: Linux-HA-Dev [at] lists > http://lists.linux-ha.org/mailman/listinfo/linux-ha-dev> Home Page: http://linux-ha.org/>