Hi Winson, On Fri, Aug 30, 2013 at 02:23:18PM +0000, Chan, Winson C wrote: > Regarding the last set of comments on the UpdatePolicy, I want to bring your > attention to a few items. I already submitted a new patch set and didn't > want to reply on the old patch set so that's why I emailed.
Sorry for the slow response on this, you've already had some good feedback from Zane and Clint, and I've just reviewed your latest patch: https://review.openstack.org/#/c/43571/ I'd really like input from the other core guys on this, particularly my complaints about the changes to the internal interfaces (where we start passing lists-of-names around again instead of a number) I'm opposed to spreading this interface change around as in the current patch (as I said in Patch Set 3, prompting your mail) - the problem is the interfaces just get much less obvious and far more error-prone IMHO (as has been proven previously). > As you are aware, IG/ASG currently create instances by appending group name > and #. On resize, it identifies the newest instances to remove by sorting on > the name string and removing from the end of the list. So I agree with Zane that this is not a requirement for your patch, merely an implementation detail of the old code. However if we can support this mode of replacement (either now or in a subsequent patch) then fine. > > Based on your comments, in the new patch set I have changed the naming of the > instances to just a # without prefixing the group name (or self.name). I > also remove the name ranges stuff. But we still have the following problems… > > 1. On a decrease in size where the oldest instances should be removed… > Since the naming is still number based, this means we'll have to remove > instances starting from 0 (since 0 is the oldest). This leaves a gap in the > beginning of the list. So on the next resize to increase, where to increase? > Continue the numbering from the end? > 2. On replace, I let the UpdateReplace handle the batch replacement. > However, for the use case where we need to support MinInstancesInService (min > instances in service = 2, batch size = 2, current size = 2), this means we > need to create the new instances first before deleting the old ones instead > of letting the instance update to handle it. Also, with the naming > restriction, this means I will have to create the 2 new replacements as '2' > and '3'. After I delete the original '0' and '1', there's a gap in the > numbering of the instances… Then this leads to the same question as above. > What happen on a resize after? > > The ideal I think is to just use some random short id for the name of the > instances and then store a creation timestamp somewhere with the resource and > use the timestamp to determine the age of the instances for removal. > Thoughts? +1, using the random short ID seems like a good plan, as does using the DB timestamp instead of string sorting to decide ordering However, I still don't think this means we need to pass lists of resource names around inside the autoscaling implementation - the only place where we care about the resource names should be in the function creating the template, e.g not in resize/replace, we just pass the number to replace and the number to create into _create_template, then update the nested stack with the new template. Thanks for the work so far on this, looks like it will be a nice new feature when we get these final issues sorted out! :) Steve _______________________________________________ OpenStack-dev mailing list [email protected] http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev
