+1 I will also contribute and will test this feature thoroughly. Regards Sadhu
-----Original Message----- From: Musayev, Ilya [mailto:imusa...@webmd.net] Sent: Friday, June 28, 2013 1:47 AM To: dev@cloudstack.apache.org Subject: RE: [MERGE] Merge VMSync improvement branch into master John and Hugo, I see and understand your concerns, however, this feature is really needed by many users - I've met with several large users at CCC13 who were looking forward to this work, blocking this merge - would delay cloudstack adoption and make ACS look inferior to other cloud platforms. Realistically, this is going to put us back at least 8+ months away until 4.3 comes out. If needed, I can dedicate QA cycles and work with Kelven to rule out all the bugs and issues - through as many scenarios as possible on my end. Thanks ilya > -----Original Message----- > From: John Burwell [mailto:jburw...@basho.com] > Sent: Thursday, June 27, 2013 2:33 PM > To: dev@cloudstack.apache.org > Subject: Re: [MERGE] Merge VMSync improvement branch into master > > Hugo, > > I completely agree with this stance, and will add a -1 as well. It > has been a significant challenge this cycle to complete high quality > reviews due to the large patch size and short time turnaround time. > Going forward, I am going to be more likely to follow Hugo's example, > and -1 patches for which there is not adequate review time. I think > the following techniques will help streamline the review process: > > 1. Narrow Patch Scope: Ensure that the patch is confined to a single > enhancement or defect fix. If you doing refactoring as part of a > feature, submit the refactoring work a separate commit. As a further > suggestion, if you are performing multiple refactorings (e.g. package > reorganization and utility method extraction), split each refactoring into > separate patches. > 2. Chunk Features: For each large feature being developed, logically > chunk the functionality and submit each chunk as a separate patch. I > recommend erring on the side of chunks being too small. Let the > reviewer determine if the patch is not large enough to be > representative of the feature. The worst that will happen is that a > reviewer will provide feedback on the work completed and simply ask > for more work to be done before it can be merged to master. For > features while chunks can't be merged all the way to master, > intermediate patches can be submitted to Review Board for review > throughout the cycle -- allowing large merges to actually be the accumulation > of several smaller reviews. > 3. Identify Reviewers Early: For big features, solicit the reviewers > whiling to review a feature from FS through merge from the mailing > list. Reviews will go much more quickly if reviewers have been > involved from the beginning because they will not help identify issues > before coding starts, but also will not have to develop context late. > To be clear, identifying reviewers early does not preclude a new > reviewer from becoming involved later, but the reviews will greatly > reduce the probability of a late review finding significant issues. > Additionally, the early reviewers can help the late reviewers come up to > speed -- reducing the burden on the contributor. > > Generally, my recommendation is to submit patches early and often. > Ultimately, contributors determine how they wish to develop and > deliver their contributions. These are only my recommendations as a > reviewer to increase the probability that a feature will make a particular > release train. > Finally, it appears that reviewers are growing less and less tolerant > of large patches that appear just before freeze dates, and those these > patches face a much higher risk of being immediately receiving a -1 > for the reasons stated above. > > Thanks, > -John > > On Jun 26, 2013, at 8:45 PM, Hugo Trippaers <h...@trippaers.nl> wrote: > > > Hey Alex, Kelven, > > > > I've been looking though the code and thanks for the very > > insightfull > conversation and follow-up email. With merges of this magnitude it's > essential to help people understand what is going on. > > > > Purely technical this is a merge i'm really pleased with. It will > > for sure > improve our handling of virtual machines. > > > > However the timing of the merge request is not ideal. We are very > > close to > the end of the already extended feature freeze. We have a consensus > within the dev community to do large architectural changes in the > beginning of the cycle and not at the very end. This not only means a > lot of extra testing effort and other developers will have to get used > to the changes introduced right at the moment when everybody should be > focussed on bug fixing, documentation and stability. Without wanting > to rip open old wounds, i can imagine we all want to avoid a javelin incident. > > > > I respect the work going into this and the effort it will take to > > keep this up > to date with the current speed of master, but still i'm going to veto > this with a > -1 for inclusion before we cut of the 4.2 branch. > > > > Cheers, > > > > Hugo > > > > On Jun 26, 2013, at 5:32 PM, Alex Huang <alex.hu...@citrix.com> wrote: > > > >> Hi All, > >> > >> Kelven had an emergency so I'm submitting the changes on vmsync for > >> him. The patch are on https://reviews.apache.org/r/12126. > >> > >> Hugo took a look and already had some questions on why so many > >> files > were changed and added/deleted, So I like to explain a bit in this email. > >> > >> As part of the vmsync change, we try to move the files that were > >> relevant > to vm orchestration into the right places so that others can properly > view the different jars and understand the relationship between the > jars. This process is ongoing and will continue in other changes. > >> > >> - Work related to jobs management have been put under cloud- > framework-jobs as handling jobs is not really part of our server but > is a library. By doing it this way, changes in it can be properly > reviewed and there's a good separation from things that utilizes jobs and > jobs itself. > >> - VirtualMachine orchestration has been moved from cloud-server to > cloud-engine-orchestration. Cloud-engine-orchestration then only > depends on cloud-engine-schema and cloud-api. This creates a clear > compilation boundary between the self-service server implementation > and orchestration implementation. > >> > >> As part of these changes, then we surfaced many problems because we > setup the proper compilation boundaries and fixed all of these problems. > For example, HostAllocator interface is something people can write > plugins for but it was buried inside cloud-server package. We've > moved a majority of these changes to cloud-api. There's quite a bit > of file movements based on this change. For vmsync changes itself, > the changes are centered around VirtualMachineManagerImpl.java so you can > review that file instead. > >> > >> Thanks. > >> > >> --Alex > >> > >> > >>> -----Original Message----- > >>> From: Kelven Yang [mailto:kelven.y...@citrix.com] > >>> Sent: Tuesday, June 18, 2013 3:38 PM > >>> To: dev@cloudstack.apache.org > >>> Subject: Re: [MERGE] Merge VMSync improvement branch into master > >>> > >>> > >>> > >>> On 6/17/13 7:43 PM, "Mice Xia" <mice_...@tcloudcomputing.com> > wrote: > >>> > >>>> Kelven, > >>>> > >>>> After the refactoring, will CS still restart HA enabled VM when > >>>> it is power off externally (e.g. using xencenter) or internally > >>>> (user initiated shutdown or crash)? > >>> > >>> > >>> If HA functionality is provided by external manager (i.e., > >>> vCenter), CS won't try to restart it, but if HA is provided by > >>> CloudStack, we will restore the legacy logic. The hook up with old > >>> HA manager (between > >>> HighAvailabilityManager/VirtualMachineManager) is in the wrap-up > >>> process > >>> > >>> > >>>> > >>>> Is seems the method HighAvailabilityManagerImpl > >>>> .scheduleRestart() is not called when VM's actual state is > >>>> stopped while expected state is > >>> running. > >>>> > >>>> > >>>> Regards > >>>> Mice > >>>> > >>>> -----Original Message----- > >>>> From: Kelven Yang [mailto:kelven.y...@citrix.com] > >>>> Sent: Tuesday, June 18, 2013 5:21 AM > >>>> To: dev@cloudstack.apache.org > >>>> Subject: Re: [MERGE] Merge VMSync improvement branch into master > >>>> > >>>> Haven't created a patch yet, will do it soon after some last wrap-ups. > >>>> > >>>> Kelven > >>>> > >>>> On 6/17/13 12:03 PM, "John Burwell" <jburw...@basho.com> wrote: > >>>> > >>>>> Kelven, > >>>>> > >>>>> Did this patch get pushed to Review Board? If so, what is the URL? > >>>>> > >>>>> Thanks. > >>>>> -John > >>>>> > >>>>> On Jun 17, 2013, at 1:40 PM, Kelven Yang > >>>>> <kelven.y...@citrix.com> > wrote: > >>>>> > >>>>>> Low level classes were tested in unit tests(MessageBus, Job > >>>>>> framework, Job dispatchers etc), interface layer changes are > >>>>>> guarded through matching the old semantics, but changes are > >>>>>> tested manually, we are planning to get this part of testing > >>>>>> through BVT system after we have re-based the latest master. > >>>>>> > >>>>>> Kelven > >>>>>> > >>>>>> On 6/17/13 10:01 AM, "Chip Childers" > >>>>>> <chip.child...@sungard.com> > >>> wrote: > >>>>>> > >>>>>>> On Mon, Jun 17, 2013 at 04:59:00PM +0000, Kelven Yang wrote: > >>>>>>>> I'd like to kick off the official merge process. We will > >>>>>>>> start the merge process after the branch has passed > >>>>>>>> necessary tests > >>>>>>>> > >>>>>>>> Kelven > >>>>>>> > >>>>>>> Can you share what testing is being run against the branch? > >>>>>> > >>>>> > >>>> > >> > >
