)]}'
{"/COMMIT_MSG":[{"author":{"_account_id":4240,"name":"Craig Vyvial","email":"cp16net@gmail.com","username":"cp16net"},"change_message_id":"22c784e05dc15c1751d6f938c045cc46c197acc6","unresolved":false,"context_lines":[{"line_number":7,"context_line":"Redis should perform backup using BGSAVE not SAVE"},{"line_number":8,"context_line":""},{"line_number":9,"context_line":"When Redis backup was implemented, using the save() command was"},{"line_number":10,"context_line":"chosen as it blocks, making the code simpler to read and understand."},{"line_number":11,"context_line":"Unfortunately SAVE and BGSAVE have differences beyond one being"},{"line_number":12,"context_line":"synchronous and the other asynchronous, in that SAVE also blocks"},{"line_number":13,"context_line":"client connections while it runs. This behaviour is not optimal,"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":2,"id":"7a5de9d1_ba0407c0","line":10,"range":{"start_line":10,"start_character":21,"end_line":10,"end_character":67},"updated":"2016-01-29 07:41:23.000000000","message":"This is not the only cases why this was chosen. There is a much bigger issue here when you choose to use one or the other. \n\nIdeally you should use BGSAVE[1] in a production environment[2], but this also forks the redis process (making a duplicate IN MEMORY). I capitalize the \u0027IN MEMORY\u0027 because this is crucial to how much memory redis is allowed to consume and based on how much data someone has added to the database. Redis doesnt use local disk other than to persist the data, most of the data is kept in memory which is why it is so fast. For example, if you have a 1g ram flavor and you give redis more than say 512MB of memory to use then you will NEVER be able to run a BGSAVE call or an internal save set in the configuration. This is because you will need another 512MB of memory to fork the process to continue to allow clients to connect read and write data. This amount of RAM will not exist with the guestagent and other services running on the instance.\n\nSAVE[2] blocks all client calls but allows you to be able to use as much ram on the instance as you can. i.e \u003e512MB but less than approximately 900MB depending on the other services running on the instance. (this is an estimate upper number.)\n\nSo with this information it really matters how the redis configuration is setup to allow saving for backup. There is a bunch of information on how redis persists its data automatically and how to configure this on the website as well[3].\n\n[1] http://redis.io/commands/bgsave\n[2] http://redis.io/commands/save\n[3] http://redis.io/topics/persistence","commit_id":"053812f50eb9586560cb458108202afd7b946c96"},{"author":{"_account_id":10215,"name":"Peter Stachowski","email":"peter@tesora.com","username":"peterstac"},"change_message_id":"c8b30117349fbb02949d4e8c7ace113992104591","unresolved":false,"context_lines":[{"line_number":7,"context_line":"Redis should perform backup using BGSAVE not SAVE"},{"line_number":8,"context_line":""},{"line_number":9,"context_line":"When Redis backup was implemented, using the save() command was"},{"line_number":10,"context_line":"chosen as it blocks, making the code simpler to read and understand."},{"line_number":11,"context_line":"Unfortunately SAVE and BGSAVE have differences beyond one being"},{"line_number":12,"context_line":"synchronous and the other asynchronous, in that SAVE also blocks"},{"line_number":13,"context_line":"client connections while it runs. This behaviour is not optimal,"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":2,"id":"7a5de9d1_4db09f8a","line":10,"range":{"start_line":10,"start_character":21,"end_line":10,"end_character":67},"in_reply_to":"7a5de9d1_ba0407c0","updated":"2016-01-29 17:12:47.000000000","message":"Well, actually it was ;)  The original spec described using BGSAVE: https://github.com/openstack/trove-specs/blob/master/specs/liberty/redis-backup-restore.rst#backup\n\nThat said, I don\u0027t think it\u0027s as big an issue as you imply, for two reasons.  One is that in practice, with overcommit_memory set, the actual memory footprint needed is typically much less.  In fact, if there is no activity on the server, the memory footprint is literally no different.  (Of course if there is no activity on the server, then using SAVE is perfectly fine, too. That\u0027s not the typical scenario, though.) From http://redis.io/topics/faq:\n\n\"Redis background saving schema relies on the copy-on-write semantic of fork in modern operating systems: Redis forks (creates a child process) that is an exact copy of the parent. The child process dumps the DB on disk and finally exits. In **theory** the child should use as much memory as the parent being a copy, but **actually** thanks to the copy-on-write semantic implemented by most modern operating systems the parent and child process will share the common memory pages. A page will be duplicated only when it changes in the child or in the parent. Since in theory all the pages may change while the child process is saving, Linux can\u0027t tell in advance how much memory the child will take, so if the overcommit_memory setting is set to zero fork will fail unless there is as much free RAM as required to really duplicate all the parent memory pages, with the result that if you have a Redis dataset of 3 GB and just 2 GB of free memory it will fail.\nSetting overcommit_memory to 1 says Linux to relax and perform the fork in a more optimistic allocation fashion, and **this is indeed what you want** for Redis.\" (emphasis added) - I will check our elements to see if we run \u0027echo 1 \u003e /proc/sys/vm/overcommit_memory\u0027 for Redis, and submit a changeset if we don\u0027t.\n\nThe second (and probably most relevant) reason is that Redis is already running BGSAVE periodically as part of the persistence strategy, since we have it set to do so in the configuration file.  That means that if you have a server that is so active that all the records change within the time it takes to write the data to disk, then yes, you\u0027ll need twice the memory of what your data requires.  However, in that high-use case, I suspect that having your cache block all those processes while the write goes on (possibly every few seconds) would be unacceptable, and you\u0027d probably be ok with allocating the extra memory to get the performance you needed.\n\nI like your idea of letting the user choose.  Unfortunately you know that Trove can\u0027t handle that (yet ;) ) and is way out of scope.  I can add another strategy if you like (and let the operator decide through the conf settings) or I can call save() if the call to bgsave() fails, but I think that\u0027s as far as we have the capability to do at the moment.  Let me know which you\u0027d prefer. :)","commit_id":"053812f50eb9586560cb458108202afd7b946c96"}],"trove/guestagent/datastore/experimental/redis/service.py":[{"author":{"_account_id":14576,"name":"Petr Malik","email":"pmalik@tesora.com","username":"pmalik"},"change_message_id":"419b76912587b1e853f950b5b8daf81317ec99c0","unresolved":false,"context_lines":[{"line_number":427,"context_line":"        if not self.__client.bgsave():"},{"line_number":428,"context_line":"            raise exception.BackupCreationError(_(\"Could not persist data\"))"},{"line_number":429,"context_line":"        while last_save \u003d\u003d self.__client.lastsave():"},{"line_number":430,"context_line":"            time.sleep(1)"},{"line_number":431,"context_line":"        LOG.debug(\"BGSAVE completed\")"},{"line_number":432,"context_line":""},{"line_number":433,"context_line":"    def set_master(self, host\u003dNone, port\u003dNone):"}],"source_content_type":"text/x-python","patch_set":1,"id":"9a68dd71_7eec828f","line":430,"range":{"start_line":430,"start_character":23,"end_line":430,"end_character":24},"updated":"2016-01-27 02:44:58.000000000","message":"Just wondering if we should use one of our numerous configurable times instead.\nOne that looks like a good match is: \u0027state_change_poll_time\u0027 - Interval between state change poll requests (seconds). (default \u003d 3s)\n\nIt is being used in database status polling, this is somewhat similar.","commit_id":"fd0d17105c17ed68eba830d6814484a84e02d436"},{"author":{"_account_id":10215,"name":"Peter Stachowski","email":"peter@tesora.com","username":"peterstac"},"change_message_id":"0644bd39fe15c781fa3ba402d46e916dea32d5c8","unresolved":false,"context_lines":[{"line_number":427,"context_line":"        if not self.__client.bgsave():"},{"line_number":428,"context_line":"            raise exception.BackupCreationError(_(\"Could not persist data\"))"},{"line_number":429,"context_line":"        while last_save \u003d\u003d self.__client.lastsave():"},{"line_number":430,"context_line":"            time.sleep(1)"},{"line_number":431,"context_line":"        LOG.debug(\"BGSAVE completed\")"},{"line_number":432,"context_line":""},{"line_number":433,"context_line":"    def set_master(self, host\u003dNone, port\u003dNone):"}],"source_content_type":"text/x-python","patch_set":1,"id":"7a5de9d1_e4f52bcb","line":430,"range":{"start_line":430,"start_character":23,"end_line":430,"end_character":24},"in_reply_to":"9a68dd71_7eec828f","updated":"2016-01-28 15:05:17.000000000","message":"Not sure why an operator would want to change the sleep time here.  Redis is pretty quick in persisting the file - making the sleep longer will just make the backup take longer IMO.  I will add a timeout though, in case the state never changes (otherwise the guest will stop responding).","commit_id":"fd0d17105c17ed68eba830d6814484a84e02d436"},{"author":{"_account_id":14576,"name":"Petr Malik","email":"pmalik@tesora.com","username":"pmalik"},"change_message_id":"ceec3640c659585c5ed80e65c4f9dc9060580562","unresolved":false,"context_lines":[{"line_number":431,"context_line":""},{"line_number":432,"context_line":"        try:"},{"line_number":433,"context_line":"            utils.poll_until(_timestamp_changed, sleep_time\u003d1,"},{"line_number":434,"context_line":"                             time_out\u003dTIME_OUT)"},{"line_number":435,"context_line":"        except exception.PollTimeOut:"},{"line_number":436,"context_line":"            raise RuntimeError(_(\"Timeout occurred waiting for Redis persist \""},{"line_number":437,"context_line":"                               \"to complete.\"))"}],"source_content_type":"text/x-python","patch_set":2,"id":"7a5de9d1_d5f08d00","line":434,"updated":"2016-01-29 21:46:06.000000000","message":"shouldn\u0027t we use a more appropriate (configurable) timeout for this?","commit_id":"053812f50eb9586560cb458108202afd7b946c96"}]}