Return-Path: X-Original-To: apmail-incubator-cloudstack-dev-archive@minotaur.apache.org Delivered-To: apmail-incubator-cloudstack-dev-archive@minotaur.apache.org Received: from mail.apache.org (hermes.apache.org [140.211.11.3]) by minotaur.apache.org (Postfix) with SMTP id 524BDD4B4 for ; Fri, 7 Dec 2012 23:15:49 +0000 (UTC) Received: (qmail 6369 invoked by uid 500); 7 Dec 2012 23:15:49 -0000 Delivered-To: apmail-incubator-cloudstack-dev-archive@incubator.apache.org Received: (qmail 6335 invoked by uid 500); 7 Dec 2012 23:15:48 -0000 Mailing-List: contact cloudstack-dev-help@incubator.apache.org; run by ezmlm Precedence: bulk List-Help: List-Unsubscribe: List-Post: List-Id: Reply-To: cloudstack-dev@incubator.apache.org Delivered-To: mailing list cloudstack-dev@incubator.apache.org Received: (qmail 6322 invoked by uid 99); 7 Dec 2012 23:15:48 -0000 Received: from reviews-vm.apache.org (HELO reviews.apache.org) (140.211.11.40) by apache.org (qpsmtpd/0.29) with ESMTP; Fri, 07 Dec 2012 23:15:48 +0000 Received: from reviews.apache.org (localhost [127.0.0.1]) by reviews.apache.org (Postfix) with ESMTP id 7A6EF1C53CC; Fri, 7 Dec 2012 23:15:44 +0000 (UTC) Content-Type: multipart/alternative; boundary="===============6748520405974865130==" MIME-Version: 1.0 Subject: Re: Review Request: S3-backed Secondary Storage From: "Rohit Yadav" To: "cloudstack" , "Rohit Yadav" , "Prasanna Santhanam" , "John Burwell" Date: Fri, 07 Dec 2012 23:15:44 -0000 Message-ID: <20121207231544.19218.81075@reviews.apache.org> X-ReviewBoard-URL: https://reviews.apache.org Auto-Submitted: auto-generated Sender: "Rohit Yadav" X-ReviewGroup: cloudstack X-ReviewRequest-URL: https://reviews.apache.org/r/8123/ X-Sender: "Rohit Yadav" References: <20121130191501.22473.34841@reviews.apache.org> In-Reply-To: <20121130191501.22473.34841@reviews.apache.org> Reply-To: "Rohit Yadav" --===============6748520405974865130== Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: quoted-printable > On Nov. 30, 2012, 7:15 p.m., Rohit Yadav wrote: > > John, thanks for patch. Much better now. Can you check, why the code th= at configure db in the testclient is commented? tools/marvin/marvin/deployD= ataCenter.py > > For the actual functional review, Edison can help on storage and Alena/= Kishan on API/DB layers. > = > John Burwell wrote: > Rohit, > = > I removed the code (which I forgot to delete) because, as I understan= d it, Marvin no longer connects to the database during testing but uses the= API. If it is no longer necessary, removing the requirement for remote ac= cess to the database on development machines the tool much lighter weight. > = > Thanks, > -John > = > Rohit Yadav wrote: > John, we need that. May be for this particular patch these are not re= quired. But marvin needs to be generic, and should be able to provide testC= lient with db settings configured, these can be used by some kind of testin= g, for ex. usage event, we cannot be always sure if an API is working for e= xample. That's why I want them, please uncomment them: > = > #dbSvr =3D self.config.dbSvr = > #self.testClient.dbConfigure(dbSvr.dbSvr, dbSvr.port, dbSvr.u= ser, \ > # dbSvr.passwd, dbSvr.db) > = > John Burwell wrote: > Rohit, > = > What if I modified the patch not attempt a database connection if no = settings are provided? This would allow test runs that do not require a da= tabase connection to omit the configuration process while leaving it availa= ble for those that do. > = > Thanks, > -John = > > = > Prasanna Santhanam wrote: > Yes, that will do it. If the dbSvr settings are provided in the json = config make the connection to create a dbClient or else return None. Alright let's do one more round of review, pl. add Edison as the reviewer. - Rohit ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/8123/#review13904 ----------------------------------------------------------- On Nov. 26, 2012, 9:44 p.m., John Burwell wrote: > = > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/8123/ > ----------------------------------------------------------- > = > (Updated Nov. 26, 2012, 9:44 p.m.) > = > = > Review request for cloudstack. > = > = > Description > ------- > = > Backs NFS-based secondary storage with an S3-compatible object store. Per= iodically, a reaper thread synchronizes templates and ISOs stored on a NFS = secondary storage mount with a configured S3 object store. It also pushes s= napshots to the object store when they are created and downloads them in ot= her zones on-demand. In addition to permitting the use of commodity or IaaS= storage solutions for static assets, it provides a means of automatically = synchronizing template and ISO assets across multiple zones. > = > For more information about the design of the patch, please see the design= document (https://cwiki.apache.org/confluence/display/CLOUDSTACK/S3-backed= +Secondary+Storage). > = > = > This addresses bug CLOUDSTACK-509. > = > = > Diffs > ----- > = > .gitignore 33f95c7 = > api/src/com/cloud/agent/api/BackupSnapshotCommand.java 9476d7d = > api/src/com/cloud/agent/api/DeleteSnapshotBackupCommand.java 3fa8c2b = > api/src/com/cloud/agent/api/DeleteTemplateFromS3Command.java PRE-CREATI= ON = > api/src/com/cloud/agent/api/DownloadSnapshotFromS3Command.java PRE-CREA= TION = > api/src/com/cloud/agent/api/DownloadTemplateFromS3ToSecondaryStorageCom= mand.java PRE-CREATION = > api/src/com/cloud/agent/api/UploadTemplateToS3FromSecondaryStorageComma= nd.java PRE-CREATION = > api/src/com/cloud/agent/api/to/S3TO.java PRE-CREATION = > api/src/com/cloud/api/ApiConstants.java 78a3ded = > api/src/com/cloud/api/ResponseGenerator.java 4e8fbd8 = > api/src/com/cloud/api/commands/AddS3Cmd.java PRE-CREATION = > api/src/com/cloud/api/commands/ListS3sCmd.java PRE-CREATION = > api/src/com/cloud/api/response/S3Response.java PRE-CREATION = > api/src/com/cloud/resource/ResourceService.java 1065453 = > api/src/com/cloud/storage/S3.java PRE-CREATION = > build/package.xml 09ed939 = > client/WEB-INF/classes/resources/messages.properties 626e44a = > client/tomcatconf/commands.properties.in 149547e = > core/pom.xml 15f0f7b = > core/src/com/cloud/storage/S3VO.java PRE-CREATION = > core/src/com/cloud/storage/SnapshotVO.java 08dfafa = > core/src/com/cloud/storage/VMTemplateS3VO.java PRE-CREATION = > core/src/com/cloud/storage/resource/NfsSecondaryStorageResource.java 15= 5210d = > patches/systemvm/debian/config/etc/sysctl.conf 7f945b0 = > plugins/hypervisors/xen/src/com/cloud/hypervisor/xen/resource/CitrixRes= ourceBase.java d2db85c = > pom.xml 4a4276e = > scripts/vm/hypervisor/xenserver/s3xen PRE-CREATION = > scripts/vm/hypervisor/xenserver/xenserver56/patch d485414 = > scripts/vm/hypervisor/xenserver/xenserver56fp1/patch 9fe9740 = > scripts/vm/hypervisor/xenserver/xenserver60/patch f049109 = > server/pom.xml e3308d8 = > server/src/com/cloud/api/ApiDBUtils.java 3b5f634 = > server/src/com/cloud/api/ApiDispatcher.java dfe4a1f = > server/src/com/cloud/api/ApiResponseHelper.java ebe8415 = > server/src/com/cloud/api/doc/ApiXmlDocWriter.java d31ef5a = > server/src/com/cloud/configuration/Config.java 66ac276 = > server/src/com/cloud/configuration/ConfigurationManagerImpl.java ef940e= 8 = > server/src/com/cloud/configuration/DefaultComponentLibrary.java ef61044 = > server/src/com/cloud/resource/ResourceManagerImpl.java ced601b = > server/src/com/cloud/storage/StorageManagerImpl.java e252633 = > server/src/com/cloud/storage/dao/S3Dao.java PRE-CREATION = > server/src/com/cloud/storage/dao/S3DaoImpl.java PRE-CREATION = > server/src/com/cloud/storage/dao/VMTemplateDao.java f5b6913 = > server/src/com/cloud/storage/dao/VMTemplateDaoImpl.java 2a0dfc8 = > server/src/com/cloud/storage/dao/VMTemplateS3Dao.java PRE-CREATION = > server/src/com/cloud/storage/dao/VMTemplateS3DaoImpl.java PRE-CREATION = > server/src/com/cloud/storage/s3/S3Manager.java PRE-CREATION = > server/src/com/cloud/storage/s3/S3ManagerImpl.java PRE-CREATION = > server/src/com/cloud/storage/snapshot/SnapshotManager.java a10298e = > server/src/com/cloud/storage/snapshot/SnapshotManagerImpl.java 32e37e6 = > server/src/com/cloud/template/S3SyncTask.java PRE-CREATION = > server/src/com/cloud/template/TemplateManagerImpl.java 1e87de2 = > setup/db/create-schema.sql fff084e = > setup/db/db/schema-40to410.sql PRE-CREATION = > tools/apidoc/gen_toc.py eeaf2a2 = > tools/marvin/marvin/cloudstackConnection.py c805213 = > tools/marvin/marvin/deployDataCenter.py bdf08cc = > ui/dictionary.jsp b80e296 = > ui/scripts/cloudStack.js de3bd73 = > ui/scripts/sharedFunctions.js b6b3ef8 = > ui/scripts/system.js 9e3932f = > ui/scripts/templates.js 74511f0 = > utils/pom.xml 6bed67f = > utils/src/com/cloud/utils/DateUtil.java be1627d = > utils/src/com/cloud/utils/S3Utils.java PRE-CREATION = > utils/src/com/cloud/utils/StringUtils.java 0f0ef05 = > utils/src/com/cloud/utils/db/GlobalLock.java 7c1c943 = > = > Diff: https://reviews.apache.org/r/8123/diff/ > = > = > Testing > ------- > = > I am submitting patch to begin the feedback process while we complete int= egration testing. I have verified that it does not interfere with normal C= loudStack operations when S3-backed Secondary Storage is disabled (the defa= ult setting) . I have successfully tested operation of single zone templat= e and ISO scenarios on devcloud described in the design document. I am cur= rently working through some issues in our multi-zone test environment to co= mplete all scenarios described. The following are the known deficiencies o= f the current implementation which I plan to correct in a subsequent patch: > = > * Cross zone garbage collection: When a global asset is deleted from o= ne zone's secondary storage, it is not deleted from the secondary storage o= f other zones which have downloaded it from the object store > * S3 Configuration Update: The API only supports adding an object stor= e configuration. Users should be able to edit the access key, secret key, = connection timeout. max error retries, and socket timeout. > * Multi-threaded Uploads: Permit the upload of multiple assets to the = object store simultaneously to decrease the propagation latency across all = zones. > = > = > Screenshots > ----------- > = > S3 Configuration Form > https://reviews.apache.org/r/8123/s/13/ > S3 Enable Menu on the Zone Tab > https://reviews.apache.org/r/8123/s/14/ > = > = > Thanks, > = > John Burwell > = > --===============6748520405974865130==--