)]}'
{"/COMMIT_MSG":[{"author":{"_account_id":6491,"name":"xing-yang","email":"xingyang105@gmail.com","username":"xing-yang"},"change_message_id":"2930e48c4ec3752850100ae0464059ca57ee61de","unresolved":false,"context_lines":[{"line_number":13,"context_line":"Here we propose loosening this coupling to enable backup processes"},{"line_number":14,"context_line":"to run on multiple nodes without having to be colocated with volume"},{"line_number":15,"context_line":"services."},{"line_number":16,"context_line":""},{"line_number":17,"context_line":"Change-Id: Ibd56014ca15e058918bacfde8d8a71e7d179dd77"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":3,"id":"3a50d1a3_c42e96a4","line":16,"updated":"2015-07-23 05:16:14.000000000","message":"Can you reference your blueprint here too?","commit_id":"3bccd22a82941037f65ec5888e584485e0ff1628"},{"author":{"_account_id":9003,"name":"Tom Barron","email":"tpb@dyncloud.net","username":"tbarron"},"change_message_id":"b559fdf30d5d1d2fe8253e45ad87e8a8d3b58b31","unresolved":false,"context_lines":[{"line_number":13,"context_line":"Here we propose loosening this coupling to enable backup processes"},{"line_number":14,"context_line":"to run on multiple nodes without having to be colocated with volume"},{"line_number":15,"context_line":"services."},{"line_number":16,"context_line":""},{"line_number":17,"context_line":"Change-Id: Ibd56014ca15e058918bacfde8d8a71e7d179dd77"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":3,"id":"3a50d1a3_e28d822b","line":16,"in_reply_to":"3a50d1a3_c42e96a4","updated":"2015-07-24 17:47:38.000000000","message":"Yes, I will do that in the next patch.","commit_id":"3bccd22a82941037f65ec5888e584485e0ff1628"}],"specs/liberty/scalable-backup-service.rst":[{"author":{"_account_id":1207,"name":"Duncan Thomas","email":"duncan.thomas@gmail.com","username":"duncan-thomas"},"change_message_id":"d6147804d8b9056cac0846f4e2f33db37ff76525","unresolved":false,"context_lines":[{"line_number":305,"context_line":"References"},{"line_number":306,"context_line":"\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d"},{"line_number":307,"context_line":""},{"line_number":308,"context_line":"* [1]: https://review.openstack.netapp.com/#/c/595/"},{"line_number":309,"context_line":"* [2]: https://etherpad.openstack.org/p/cinder-scaling-backup-service"},{"line_number":310,"context_line":"* [3]: https://blueprints.launchpad.net/cinder/+spec/non-disruptive-backup"}],"source_content_type":"text/x-rst","patch_set":2,"id":"3a50d1a3_b9339613","line":308,"updated":"2015-07-22 13:00:55.000000000","message":"review.openstack.org? Even then it points to the wrong review","commit_id":"384f81d21576592adfaf8e50e0b0da0c188f07af"},{"author":{"_account_id":6491,"name":"xing-yang","email":"xingyang105@gmail.com","username":"xing-yang"},"change_message_id":"2930e48c4ec3752850100ae0464059ca57ee61de","unresolved":false,"context_lines":[{"line_number":56,"context_line":"it for restore."},{"line_number":57,"context_line":""},{"line_number":58,"context_line":"Today, this backend-specific information is provided as follows.  When"},{"line_number":59,"context_line":"the a backup service process starts up, the backup manager loads the"},{"line_number":60,"context_line":"volume manager module for each enabled backend configured in"},{"line_number":61,"context_line":"``cinder.conf``.  When an OpenStack tenant or administrator makes a"},{"line_number":62,"context_line":"request to create a backup from a cinder volume or to restore a backup"}],"source_content_type":"text/x-rst","patch_set":3,"id":"3a50d1a3_476da8ca","line":59,"updated":"2015-07-23 05:16:14.000000000","message":"only need one of \"the\" or \"a\"","commit_id":"3bccd22a82941037f65ec5888e584485e0ff1628"},{"author":{"_account_id":9003,"name":"Tom Barron","email":"tpb@dyncloud.net","username":"tbarron"},"change_message_id":"b559fdf30d5d1d2fe8253e45ad87e8a8d3b58b31","unresolved":false,"context_lines":[{"line_number":56,"context_line":"it for restore."},{"line_number":57,"context_line":""},{"line_number":58,"context_line":"Today, this backend-specific information is provided as follows.  When"},{"line_number":59,"context_line":"the a backup service process starts up, the backup manager loads the"},{"line_number":60,"context_line":"volume manager module for each enabled backend configured in"},{"line_number":61,"context_line":"``cinder.conf``.  When an OpenStack tenant or administrator makes a"},{"line_number":62,"context_line":"request to create a backup from a cinder volume or to restore a backup"}],"source_content_type":"text/x-rst","patch_set":3,"id":"3a50d1a3_42a9d67a","line":59,"in_reply_to":"3a50d1a3_476da8ca","updated":"2015-07-24 17:47:38.000000000","message":"Thanks.  Will fix in the next patch.","commit_id":"3bccd22a82941037f65ec5888e584485e0ff1628"},{"author":{"_account_id":6491,"name":"xing-yang","email":"xingyang105@gmail.com","username":"xing-yang"},"change_message_id":"2930e48c4ec3752850100ae0464059ca57ee61de","unresolved":false,"context_lines":[{"line_number":172,"context_line":"  for such an elastic backup service placement capability but it is"},{"line_number":173,"context_line":"  only a small step towards enabling such a capability."},{"line_number":174,"context_line":""},{"line_number":175,"context_line":"* Cleanup of aborted backups and restores."},{"line_number":176,"context_line":""},{"line_number":177,"context_line":"  At startup, the current backup service code makes an attempt to"},{"line_number":178,"context_line":"  discover and cleanup orphaned, incomplete backup and restore"}],"source_content_type":"text/x-rst","patch_set":3,"id":"3a50d1a3_7025ed59","line":175,"updated":"2015-07-23 05:16:14.000000000","message":"This is a very important one listed under \"What this specification does not solve\" section. Are you intending to solve this during implementation?  I see you have some ideas listed below, but it is not clear to me whether you will implement it or not.","commit_id":"3bccd22a82941037f65ec5888e584485e0ff1628"},{"author":{"_account_id":9003,"name":"Tom Barron","email":"tpb@dyncloud.net","username":"tbarron"},"change_message_id":"b559fdf30d5d1d2fe8253e45ad87e8a8d3b58b31","unresolved":false,"context_lines":[{"line_number":172,"context_line":"  for such an elastic backup service placement capability but it is"},{"line_number":173,"context_line":"  only a small step towards enabling such a capability."},{"line_number":174,"context_line":""},{"line_number":175,"context_line":"* Cleanup of aborted backups and restores."},{"line_number":176,"context_line":""},{"line_number":177,"context_line":"  At startup, the current backup service code makes an attempt to"},{"line_number":178,"context_line":"  discover and cleanup orphaned, incomplete backup and restore"}],"source_content_type":"text/x-rst","patch_set":3,"id":"3a50d1a3_62f7328d","line":175,"in_reply_to":"3a50d1a3_7025ed59","updated":"2015-07-24 17:47:38.000000000","message":"I agree that we can\u0027t just ignore this issue :-)\n\nI left any best-effort fix out of the original POC code to highlight the issue and solicit ideas.","commit_id":"3bccd22a82941037f65ec5888e584485e0ff1628"},{"author":{"_account_id":6491,"name":"xing-yang","email":"xingyang105@gmail.com","username":"xing-yang"},"change_message_id":"2930e48c4ec3752850100ae0464059ca57ee61de","unresolved":false,"context_lines":[{"line_number":189,"context_line":"  backup service process pid alongside the backup host in the database"},{"line_number":190,"context_line":"  backup record.  At startup, a backup process would only consider"},{"line_number":191,"context_line":"  doing cleanup if the host matched and no other backup process on"},{"line_number":192,"context_line":"  that host had the pid."},{"line_number":193,"context_line":""},{"line_number":194,"context_line":"  It may however be well worth taking a step back to separate the"},{"line_number":195,"context_line":"  cleanup interrupted backup concern from that of actually running"}],"source_content_type":"text/x-rst","patch_set":3,"id":"3a50d1a3_d0e3a1d7","line":192,"updated":"2015-07-23 05:16:14.000000000","message":"This seems okay to me. Even if this may not be the best approach, it is better to do cleanup than skipping it.","commit_id":"3bccd22a82941037f65ec5888e584485e0ff1628"},{"author":{"_account_id":15961,"name":"lisali","email":"xiaoyan.li@intel.com","username":"lisali"},"change_message_id":"8673d1151444f90527fa879d7aa404b654a57032","unresolved":false,"context_lines":[{"line_number":189,"context_line":"  backup service process pid alongside the backup host in the database"},{"line_number":190,"context_line":"  backup record.  At startup, a backup process would only consider"},{"line_number":191,"context_line":"  doing cleanup if the host matched and no other backup process on"},{"line_number":192,"context_line":"  that host had the pid."},{"line_number":193,"context_line":""},{"line_number":194,"context_line":"  It may however be well worth taking a step back to separate the"},{"line_number":195,"context_line":"  cleanup interrupted backup concern from that of actually running"}],"source_content_type":"text/x-rst","patch_set":3,"id":"3a50d1a3_f27517fa","line":192,"in_reply_to":"3a50d1a3_4140c235","updated":"2015-07-31 01:54:19.000000000","message":"Agree with you.","commit_id":"3bccd22a82941037f65ec5888e584485e0ff1628"},{"author":{"_account_id":9003,"name":"Tom Barron","email":"tpb@dyncloud.net","username":"tbarron"},"change_message_id":"857a740fb909cc2c35bf3846febd450d868fddde","unresolved":false,"context_lines":[{"line_number":189,"context_line":"  backup service process pid alongside the backup host in the database"},{"line_number":190,"context_line":"  backup record.  At startup, a backup process would only consider"},{"line_number":191,"context_line":"  doing cleanup if the host matched and no other backup process on"},{"line_number":192,"context_line":"  that host had the pid."},{"line_number":193,"context_line":""},{"line_number":194,"context_line":"  It may however be well worth taking a step back to separate the"},{"line_number":195,"context_line":"  cleanup interrupted backup concern from that of actually running"}],"source_content_type":"text/x-rst","patch_set":3,"id":"3a50d1a3_4140c235","line":192,"in_reply_to":"3a50d1a3_867c7921","updated":"2015-07-30 21:41:30.000000000","message":"Lisa, w.r.t. #1 that\u0027s really already the way I\u0027m understanding/re-interpreting the \"host\" field.   And any time we start up a backup service and run init_host we can as you say do cleanup for all backup services because we can query the DB for all backup service instances running.  So we\u0027ll know all hosts in play at the time.  So for each backup or restore that we find in flight (and its associated volume) when we start up, we can tell whether it belongs to one of the other service instances currently running and only do the cleanup if it does not belong to one of those.  If we\u0027re doing the cleanup, we\u0027d go on and set the last_host/host to ourself before proceeding.  Then clear it on completion of the cleanup.\n\nDo I have the essence of your suggestion correct?\n\nWe can query the db for all cinder services today and see which have \"backup\" for their binary and the associated \"host\".  That info alone would only be sufficient if we constrain the number of services on a host to one.  Not sure we\u0027d hold that constraint over the long run if we scaled out to containerized services and the like.  Maybe we could save a uuid for a service instance as well for each backup service instance and save that as part of the \"host\" field if the one service per host constraint is relaxed ....","commit_id":"3bccd22a82941037f65ec5888e584485e0ff1628"},{"author":{"_account_id":15961,"name":"lisali","email":"xiaoyan.li@intel.com","username":"lisali"},"change_message_id":"f38e35c3a02113971d76f26fdaed79c457cbcd95","unresolved":false,"context_lines":[{"line_number":189,"context_line":"  backup service process pid alongside the backup host in the database"},{"line_number":190,"context_line":"  backup record.  At startup, a backup process would only consider"},{"line_number":191,"context_line":"  doing cleanup if the host matched and no other backup process on"},{"line_number":192,"context_line":"  that host had the pid."},{"line_number":193,"context_line":""},{"line_number":194,"context_line":"  It may however be well worth taking a step back to separate the"},{"line_number":195,"context_line":"  cleanup interrupted backup concern from that of actually running"}],"source_content_type":"text/x-rst","patch_set":3,"id":"3a50d1a3_867c7921","line":192,"in_reply_to":"3a50d1a3_a1f746f4","updated":"2015-07-27 06:36:52.000000000","message":"Tom, just give me my considerations, we may discuss:\n\n1. How about to understand/change \"host\" in the table cinder.backups to \"last_host\", which means the last host doing backup-create or backup-restore. \nBackups don\u0027t belong to any specified host, but every time a task is run in a specified host.\n\n\n2. Every time a backup service restarts, it does cleanup for all backup services. We can use database transaction (compare and set) to make sure only one backup service does the cleanup.","commit_id":"3bccd22a82941037f65ec5888e584485e0ff1628"},{"author":{"_account_id":9003,"name":"Tom Barron","email":"tpb@dyncloud.net","username":"tbarron"},"change_message_id":"b559fdf30d5d1d2fe8253e45ad87e8a8d3b58b31","unresolved":false,"context_lines":[{"line_number":189,"context_line":"  backup service process pid alongside the backup host in the database"},{"line_number":190,"context_line":"  backup record.  At startup, a backup process would only consider"},{"line_number":191,"context_line":"  doing cleanup if the host matched and no other backup process on"},{"line_number":192,"context_line":"  that host had the pid."},{"line_number":193,"context_line":""},{"line_number":194,"context_line":"  It may however be well worth taking a step back to separate the"},{"line_number":195,"context_line":"  cleanup interrupted backup concern from that of actually running"}],"source_content_type":"text/x-rst","patch_set":3,"id":"3a50d1a3_a1f746f4","line":192,"in_reply_to":"3a50d1a3_d0e3a1d7","updated":"2015-07-24 17:47:38.000000000","message":"Well I\u0027ll likely fall back to this approach then if I can\u0027t think of anything better.  I\u0027d like to not require placement of backup processes on particular hosts just so they can clean up stuff left over on that host.  Still thinking on this one ....","commit_id":"3bccd22a82941037f65ec5888e584485e0ff1628"},{"author":{"_account_id":9003,"name":"Tom Barron","email":"tpb@dyncloud.net","username":"tbarron"},"change_message_id":"ce11af25ffa4230869b0fd91ad54f26e6c4a4084","unresolved":false,"context_lines":[{"line_number":189,"context_line":"  backup service process pid alongside the backup host in the database"},{"line_number":190,"context_line":"  backup record.  At startup, a backup process would only consider"},{"line_number":191,"context_line":"  doing cleanup if the host matched and no other backup process on"},{"line_number":192,"context_line":"  that host had the pid."},{"line_number":193,"context_line":""},{"line_number":194,"context_line":"  It may however be well worth taking a step back to separate the"},{"line_number":195,"context_line":"  cleanup interrupted backup concern from that of actually running"}],"source_content_type":"text/x-rst","patch_set":3,"id":"3a50d1a3_6ccca761","line":192,"in_reply_to":"3a50d1a3_f27517fa","updated":"2015-07-31 18:13:55.000000000","message":"Just logging that in IRC Lisa and I decided no extra field in the services table is required to handle multiple services on one node.  We could just extend the \"host\" field as required to distinguish, much as this was done for pools within volume services.","commit_id":"3bccd22a82941037f65ec5888e584485e0ff1628"},{"author":{"_account_id":15961,"name":"lisali","email":"xiaoyan.li@intel.com","username":"lisali"},"change_message_id":"ccf00399e81bd15b3787fb7b61df799955b55cfd","unresolved":false,"context_lines":[{"line_number":194,"context_line":"than itself.  (If in the future multiple services are running on a"},{"line_number":195,"context_line":"single node, we can enrich the ``host`` field to distinguish among"},{"line_number":196,"context_line":"them.)  The service can then check the ``backup host`` field of the"},{"line_number":197,"context_line":"backup object for all in-flight or orphaned backup objects to"},{"line_number":198,"context_line":"distinguish whether they and their associated volume or snapshot"},{"line_number":199,"context_line":"objects belong to an active backup service or not.  If no other backup"},{"line_number":200,"context_line":"service owns the backup object (i.e., the ``host`` field corresponds"}],"source_content_type":"text/x-rst","patch_set":5,"id":"1a4dcd0f_8a16aa53","line":197,"updated":"2015-08-05 08:26:06.000000000","message":"Hi Tom,\nDo you mean there will be two fields: host and backup_host? \nWhy?","commit_id":"6330254005cc5e8513aaf0c58c62cf8ed86e0fdc"},{"author":{"_account_id":9003,"name":"Tom Barron","email":"tpb@dyncloud.net","username":"tbarron"},"change_message_id":"83f5b0ee5e04323a2e1d90dd095e8df4bf7922d0","unresolved":false,"context_lines":[{"line_number":194,"context_line":"than itself.  (If in the future multiple services are running on a"},{"line_number":195,"context_line":"single node, we can enrich the ``host`` field to distinguish among"},{"line_number":196,"context_line":"them.)  The service can then check the ``backup host`` field of the"},{"line_number":197,"context_line":"backup object for all in-flight or orphaned backup objects to"},{"line_number":198,"context_line":"distinguish whether they and their associated volume or snapshot"},{"line_number":199,"context_line":"objects belong to an active backup service or not.  If no other backup"},{"line_number":200,"context_line":"service owns the backup object (i.e., the ``host`` field corresponds"}],"source_content_type":"text/x-rst","patch_set":5,"id":"1a4dcd0f_ff711777","line":197,"in_reply_to":"1a4dcd0f_8a16aa53","updated":"2015-08-05 14:25:20.000000000","message":"No, that was just a wording mistake.  I intend one ``host`` field.\n\nThanks.","commit_id":"6330254005cc5e8513aaf0c58c62cf8ed86e0fdc"},{"author":{"_account_id":15961,"name":"lisali","email":"xiaoyan.li@intel.com","username":"lisali"},"change_message_id":"608e29b19d405fec5ee4ed051a8e98c43ba87a9d","unresolved":false,"context_lines":[{"line_number":201,"context_line":"to no active backup service or to one\u0027s own host), then the backup"},{"line_number":202,"context_line":"service can clean up the backups and over rpc to associated volume"},{"line_number":203,"context_line":"backend host clean up associated volumes, temporary volumes and"},{"line_number":204,"context_line":"snapshots."},{"line_number":205,"context_line":""},{"line_number":206,"context_line":"In this approach the ``host`` field for a backup object is set by API"},{"line_number":207,"context_line":"(or, down the road, a scheduler) when the backup or restore rpc is"}],"source_content_type":"text/x-rst","patch_set":5,"id":"1a4dcd0f_9ec71064","line":204,"updated":"2015-08-10 03:26:41.000000000","message":"Tom, about clean up corresponding volumes: \n1. clean up restoring-backup\u0026backing-up volumes;\nhttps://github.com/openstack/cinder/blob/master/cinder/backup/manager.py#L200\n\nbackup manager needs to find out whether it can send clean request to volumes service. \n\nI think here we need a mapping which indicates which backup a restoring-backup or backing-up volume is related to. \n\nFor example, we can add a related-volume in backups table. When a volume starts to restore with the backup, the field is filled with the volume id. \n\nif during the restoring period, backup-manage corrupts. When backup-manage starts, it at first find the backups which need to recover based on \u0027host\u0027 field,  and then it checks \u0027related-volume\u0027 field (this volume needs to clean ) and sends the clean request to volume service.","commit_id":"6330254005cc5e8513aaf0c58c62cf8ed86e0fdc"},{"author":{"_account_id":9003,"name":"Tom Barron","email":"tpb@dyncloud.net","username":"tbarron"},"change_message_id":"512fdf5b27572daccb90102e283e2d8b7e472b0b","unresolved":false,"context_lines":[{"line_number":201,"context_line":"to no active backup service or to one\u0027s own host), then the backup"},{"line_number":202,"context_line":"service can clean up the backups and over rpc to associated volume"},{"line_number":203,"context_line":"backend host clean up associated volumes, temporary volumes and"},{"line_number":204,"context_line":"snapshots."},{"line_number":205,"context_line":""},{"line_number":206,"context_line":"In this approach the ``host`` field for a backup object is set by API"},{"line_number":207,"context_line":"(or, down the road, a scheduler) when the backup or restore rpc is"}],"source_content_type":"text/x-rst","patch_set":5,"id":"1a4dcd0f_bd03a9b7","line":204,"in_reply_to":"1a4dcd0f_9ec71064","updated":"2015-08-11 10:51:15.000000000","message":"In the backup api create() method,  backup.volume_id is set when the volume status is set to \u0027backing-up\u0027.  So if we find a backup object in \u0027creating\u0027 state, we can look at the volume it is backing up (from backup.volume_id) and see if it is in \u0027backing-up\u0027 state and clean it up as necessary.  Similarly, we can check backup.temp_volume and backup.temp_snapshot.\n\nBut we don\u0027t currently set a volume field in the backup object when restoring, so as you suggest we will need to save it there.\n\nCan we just have a \u0027restoring_volume_id\u0027 field, or am I missing something and we need a more general \u0027related-volume\u0027 field?","commit_id":"6330254005cc5e8513aaf0c58c62cf8ed86e0fdc"},{"author":{"_account_id":15961,"name":"lisali","email":"xiaoyan.li@intel.com","username":"lisali"},"change_message_id":"d7e29ac14c439b0d04dbd10b6090999c5b1b70f8","unresolved":false,"context_lines":[{"line_number":201,"context_line":"to no active backup service or to one\u0027s own host), then the backup"},{"line_number":202,"context_line":"service can clean up the backups and over rpc to associated volume"},{"line_number":203,"context_line":"backend host clean up associated volumes, temporary volumes and"},{"line_number":204,"context_line":"snapshots."},{"line_number":205,"context_line":""},{"line_number":206,"context_line":"In this approach the ``host`` field for a backup object is set by API"},{"line_number":207,"context_line":"(or, down the road, a scheduler) when the backup or restore rpc is"}],"source_content_type":"text/x-rst","patch_set":5,"id":"1a4dcd0f_f09c8a0f","line":204,"in_reply_to":"1a4dcd0f_bd03a9b7","updated":"2015-08-13 06:39:54.000000000","message":"A field \u0027restoring_volume_id\u0027 is enough. Cinder only has backup and restore operataions related to volume status. volume-id, temp_volume_id, temp_snapshot_id for backup, and \u0027restoring_volume_id\u0027 for restore.\nWhen starting to restore, set the field, and after restoring, unset the field.","commit_id":"6330254005cc5e8513aaf0c58c62cf8ed86e0fdc"}]}