On Fri, Feb 7, 2014 at 4:48 PM, Sheng Yang <sh...@yasker.org> wrote: > Thanks for the comment! > > On Fri, Feb 7, 2014 at 3:25 PM, Chiradeep Vittal < > chiradeep.vit...@citrix.com> wrote: > >> +1. >> * The guideline is not clear as to when a developer should use this >> executor. Why not use it all the time (even for a single command) >> > > The mechanism behind is NOT a "producer-consumer" model. It's just a > queue(or a list) to delay the commands execution for now. Only the code > would generated a large number of commands would need to use this, and > basically that's the VR reboot/re-create at this time. > > >> * Are there any issues when there are multiple management servers >> involved? >> > > It would remain the same as before. One VR(host) would only operated by > one mgmt server, the other one would reroute all the related commands to > the mgmt server in charge. And as soon as aggregated execution started, it > would block all the commands at least in VR level(also in the host queue if > network.element.sequence.execution is true). > > >> * Any threading concerns? That is, multiple threads are attempting to >> update the VR, some are using the aggregated approach, some are not. >> > > It is not a multithread solution in my mind. One queue for one VR, and > would be created only "prepare" func called. All the following commands, no > matter from which thread, would goes to this queue(by hooking > sendCommandsToRouter in VR), wait until "complete" func called(which is > expected quite soon). > > >> * What is the default value of aggregated period? >> > > No default, commands would wait until "complete" func called. > > >> * What if the caller dies before calling completeAggregatedExecution >> > > As noted in the spec, exception handler should call > abortAggregatedExecution(). As design for VR booting up period, one failure > would means VR fail to boot up. > > And, before commands send to VR, all the exception should be handled by > mgmt server. I don't think there are any factor we cannot control would > impact the command generation. But nonetheless, abortAggregatedExecution() > is provided to clean up the queue and fail the execution. > > >> * what is the queue mechanism? LinkedBlockingQueue? >> > > Since it's not async, any ordered queue should be fine. Probably I should > call it ArrayList... >
Rethink about this. The input of queue can potentially become async, say if e.g. some other commands was added by other threads, so it's still better to get a LinkedBlockingQueue. --Sheng > > >> * Any impact on the agent thread pool size? Does this use its own thread >> pool? >> > > No thread pool. > > >> * >> Can we also address the case of restoring state to a VR when restarting >> the VR outside of CloudStack. >> > > No. That's not in the scope. > > I was willing to take a more radical approach for this work, but > practically I would want to solve the 80% problem this time(which is VR > booting/upgrading time). > > --Sheng > > >> >> On 2/6/14 5:03 PM, "Sheng Yang" <sh...@yasker.org> wrote: >> >> >Hi Devs, >> > >> >Here I'd like to introduce this improvement of VR. >> > >> >https://issues.apache.org/jira/browse/CLOUDSTACK-6047 >> > >> https://cwiki.apache.org/confluence/display/CLOUDSTACK/Virtual+Router+aggr >> >egated+command+execution >> > >> >In short, we would speed up VR's rebooting and re-creating, by aggregated >> >execution the CloudStack configuration commands when booting up. >> Hopefully >> >we can get it done in O(1) rather than O(n)(which is current state). >> > >> >And to prepare for this work, I've get a long expected code refactor >> done. >> >Now one VirtualRoutingResources would take over all the VR execution >> >commands(rather than every hypervisor's resource). From now on, you would >> >only need to modify one place in order to update VR commands. >> > >> >I've put some details of VR aggregated execution in the FS. >> > >> >Comments are welcome! >> > >> >--Sheng >> >> >