> On Dec. 14, 2012, 9:58 p.m., Rohit Yadav wrote: > > John, don't put all the patches on one review. Pl. close this one as > > submitted and open a new review (possibly backed by a jira issue) for your > > changes: > > Makes the Marvin database configuration optional > > Adds S3 template search into ManagementServerImpl#listTemplates > >
Rohit, The original patch was prematurely merged into master. I had re-opened the review previous to the merge requesting a hold until I push the response to feedback. Since this patch includes feedback to complete this review, I attached per my previous comment. All of this work pertains to the CLOUDSTACK-509 ticket. Thanks, -John - John ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/8123/#review14524 ----------------------------------------------------------- On Dec. 14, 2012, 11:48 p.m., John Burwell wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/8123/ > ----------------------------------------------------------- > > (Updated Dec. 14, 2012, 11:48 p.m.) > > > Review request for cloudstack and edison su. > > > Description > ------- > > Backs NFS-based secondary storage with an S3-compatible object store. > Periodically, a reaper thread synchronizes templates and ISOs stored on a NFS > secondary storage mount with a configured S3 object store. It also pushes > snapshots to the object store when they are created and downloads them in > other 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 > ----- > > api/src/com/cloud/api/commands/ListS3sCmd.java 507053d > build/package.xml 09ed939 > pom.xml e5aa024 > server/src/com/cloud/configuration/ConfigurationManagerImpl.java 2d7dfe2 > server/src/com/cloud/server/ManagementServerImpl.java 117be57 > server/src/com/cloud/storage/dao/VMTemplateDao.java 1284ba1 > server/src/com/cloud/storage/dao/VMTemplateDaoImpl.java 5c71f1b > server/src/com/cloud/storage/s3/S3ManagerImpl.java 6b07254 > tools/marvin/marvin/cloudstackConnection.py 8c4e325 > tools/marvin/marvin/deployDataCenter.py 01235fe > utils/src/com/cloud/utils/db/DbUtil.java feef7b3 > > Diff: https://reviews.apache.org/r/8123/diff/ > > > Testing > ------- > > I am submitting patch to begin the feedback process while we complete > integration testing. I have verified that it does not interfere with normal > CloudStack operations when S3-backed Secondary Storage is disabled (the > default setting) . I have successfully tested operation of single zone > template and ISO scenarios on devcloud described in the design document. I > am currently working through some issues in our multi-zone test environment > to complete all scenarios described. The following are the known > deficiencies of the current implementation which I plan to correct in a > subsequent patch: > > * Cross zone garbage collection: When a global asset is deleted from one > zone's secondary storage, it is not deleted from the secondary storage of > other zones which have downloaded it from the object store > * S3 Configuration Update: The API only supports adding an object store > 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 > >
