)]}'
{"/COMMIT_MSG":[{"author":{"_account_id":8290,"name":"liusheng","email":"liusheng2048@gmail.com","username":"LiuSheng"},"change_message_id":"640879155d5150b0b3b2c2f114e7636b9b8470d2","unresolved":false,"context_lines":[{"line_number":10,"context_line":"inner join sample table, and apply filters on it. However, when there"},{"line_number":11,"context_line":"is no timestamp filters, the inner join is unnecessary."},{"line_number":12,"context_line":""},{"line_number":13,"context_line":"Note that when sql_expire_samples_only is enabled, there will be"},{"line_number":14,"context_line":"some resources without any sample, in such case we should use inner"},{"line_number":15,"context_line":"join to avoid wrong result."},{"line_number":16,"context_line":""}],"source_content_type":"text/x-gerrit-commit-message","patch_set":3,"id":"3a29b11f_b22a4c7d","line":13,"updated":"2015-10-26 06:56:56.000000000","message":"thinking... is this situation reasonable ? should we clean the resource if all the samples of the resource have out of date.","commit_id":"860c8cdc509e990bab5c2f507f3b1cfb4abdabee"},{"author":{"_account_id":6676,"name":"zqfan","email":"aji.zqfan@gmail.com","username":"aji-zqfan"},"change_message_id":"623250aab2bb8c2b67c063d87b69f5ccb96b814e","unresolved":false,"context_lines":[{"line_number":10,"context_line":"inner join sample table, and apply filters on it. However, when there"},{"line_number":11,"context_line":"is no timestamp filters, the inner join is unnecessary."},{"line_number":12,"context_line":""},{"line_number":13,"context_line":"Note that when sql_expire_samples_only is enabled, there will be"},{"line_number":14,"context_line":"some resources without any sample, in such case we should use inner"},{"line_number":15,"context_line":"join to avoid wrong result."},{"line_number":16,"context_line":""}],"source_content_type":"text/x-gerrit-commit-message","patch_set":3,"id":"3a29b11f_f202040b","line":13,"in_reply_to":"3a29b11f_b22a4c7d","updated":"2015-10-26 07:55:04.000000000","message":"see: https://bugs.launchpad.net/ceilometer/+bug/1424871","commit_id":"860c8cdc509e990bab5c2f507f3b1cfb4abdabee"},{"author":{"_account_id":8290,"name":"liusheng","email":"liusheng2048@gmail.com","username":"LiuSheng"},"change_message_id":"ee50885fc40a8090e7be3f7d5a59370a398d69dc","unresolved":false,"context_lines":[{"line_number":10,"context_line":"inner join sample table, and apply filters on it. However, when there"},{"line_number":11,"context_line":"is no timestamp filters, the inner join is unnecessary."},{"line_number":12,"context_line":""},{"line_number":13,"context_line":"Note that when sql_expire_samples_only is enabled, there will be"},{"line_number":14,"context_line":"some resources without any sample, in such case we should use inner"},{"line_number":15,"context_line":"join to avoid wrong result."},{"line_number":16,"context_line":""}],"source_content_type":"text/x-gerrit-commit-message","patch_set":3,"id":"3a29b11f_3f5e0477","line":13,"in_reply_to":"3a29b11f_f202040b","updated":"2015-10-26 09:18:19.000000000","message":"got it, thanks","commit_id":"860c8cdc509e990bab5c2f507f3b1cfb4abdabee"},{"author":{"_account_id":13560,"name":"Rohit Jaiswal","email":"rohit.jaiswal@gmail.com","username":"rjaiswal"},"change_message_id":"35a975068a0afc580269adb1652996b6be6240a6","unresolved":false,"context_lines":[{"line_number":9,"context_line":"To get distinct resource ids, we do a query on resource table which"},{"line_number":10,"context_line":"inner join sample table, and apply filters on it. However, when there"},{"line_number":11,"context_line":"is no timestamp filters, the inner join is unnecessary."},{"line_number":12,"context_line":""},{"line_number":13,"context_line":"Change-Id: If85dbea15d42d42c6b0be7402c06f258e278b2eb"},{"line_number":14,"context_line":"Closes-Bug: #1509677"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":4,"id":"3a29b11f_44e76812","line":12,"updated":"2015-10-26 17:15:25.000000000","message":"Maybe change the commit header and message to include that perform inner join only when expire_samples_only is enabled or timestamp is specified.","commit_id":"5f4af65744c4acba76814210eb2b05b8979940af"}],"ceilometer/storage/impl_sqlalchemy.py":[{"author":{"_account_id":8290,"name":"liusheng","email":"liusheng2048@gmail.com","username":"LiuSheng"},"change_message_id":"640879155d5150b0b3b2c2f114e7636b9b8470d2","unresolved":false,"context_lines":[{"line_number":432,"context_line":"        session \u003d self._engine_facade.get_session()"},{"line_number":433,"context_line":"        # get list of resource_ids"},{"line_number":434,"context_line":"        has_timestamp \u003d start_timestamp or end_timestamp"},{"line_number":435,"context_line":"        if cfg.CONF.sql_expire_samples_only or has_timestamp:"},{"line_number":436,"context_line":"            res_q \u003d session.query(distinct(models.Resource.resource_id)).join("},{"line_number":437,"context_line":"                models.Sample,"},{"line_number":438,"context_line":"                models.Sample.resource_id \u003d\u003d models.Resource.internal_id)"}],"source_content_type":"text/x-python","patch_set":3,"id":"3a29b11f_d20268b4","line":435,"updated":"2015-10-26 06:56:56.000000000","message":"when we specified timestamp filter, can we use the following max_timestamp and min_timestamp (L447~L450) to check if there is sample in the time range of the resource ? if the sample_filter.start_timestamp \u003e max_timestamp or sample_filter.end_timestamp \u003c min_timestamp , the source should be skipped. otherwise the resource should be returned. if so we don\u0027t need inner join both of two cases.\n\nnot sure i am missing something.","commit_id":"860c8cdc509e990bab5c2f507f3b1cfb4abdabee"},{"author":{"_account_id":8290,"name":"liusheng","email":"liusheng2048@gmail.com","username":"LiuSheng"},"change_message_id":"ee50885fc40a8090e7be3f7d5a59370a398d69dc","unresolved":false,"context_lines":[{"line_number":432,"context_line":"        session \u003d self._engine_facade.get_session()"},{"line_number":433,"context_line":"        # get list of resource_ids"},{"line_number":434,"context_line":"        has_timestamp \u003d start_timestamp or end_timestamp"},{"line_number":435,"context_line":"        if cfg.CONF.sql_expire_samples_only or has_timestamp:"},{"line_number":436,"context_line":"            res_q \u003d session.query(distinct(models.Resource.resource_id)).join("},{"line_number":437,"context_line":"                models.Sample,"},{"line_number":438,"context_line":"                models.Sample.resource_id \u003d\u003d models.Resource.internal_id)"}],"source_content_type":"text/x-python","patch_set":3,"id":"3a29b11f_ff088c2f","line":435,"in_reply_to":"3a29b11f_0d68c5b9","updated":"2015-10-26 09:18:19.000000000","message":"Agree, the time is more expensive.\n\nnon-blocking nit: it\u0027s better to add a note about cfg.CONF.sql_expire_samples_only in code, not commit message, looks weird if we don\u0027t find the commit message :)","commit_id":"860c8cdc509e990bab5c2f507f3b1cfb4abdabee"},{"author":{"_account_id":6676,"name":"zqfan","email":"aji.zqfan@gmail.com","username":"aji-zqfan"},"change_message_id":"623250aab2bb8c2b67c063d87b69f5ccb96b814e","unresolved":false,"context_lines":[{"line_number":432,"context_line":"        session \u003d self._engine_facade.get_session()"},{"line_number":433,"context_line":"        # get list of resource_ids"},{"line_number":434,"context_line":"        has_timestamp \u003d start_timestamp or end_timestamp"},{"line_number":435,"context_line":"        if cfg.CONF.sql_expire_samples_only or has_timestamp:"},{"line_number":436,"context_line":"            res_q \u003d session.query(distinct(models.Resource.resource_id)).join("},{"line_number":437,"context_line":"                models.Sample,"},{"line_number":438,"context_line":"                models.Sample.resource_id \u003d\u003d models.Resource.internal_id)"}],"source_content_type":"text/x-python","patch_set":3,"id":"3a29b11f_0d68c5b9","line":435,"in_reply_to":"3a29b11f_d20268b4","updated":"2015-10-26 07:55:04.000000000","message":"The performance bottle neck is the for loop rather than the inner join (it actually very fast, even with a full table join, which contains 5M rows. Although I think the RAM/DISK cost is a bit high, the buffer will be drained out.)\n\nFor example, a 5M samples and 100K resources database, the distinct sentence for a full table join will response in less than 2 seconds, then there will be 100K rows, and each row there will be two queries, even if each query will be response in 0.005 second, the total response time will be  more than 16 minutes (trigger timeout exception by default).\n\nSo what we need to do is doing a inner join firstly to reduce resource id rows, not checking resource is needed or not in the loop.","commit_id":"860c8cdc509e990bab5c2f507f3b1cfb4abdabee"},{"author":{"_account_id":6676,"name":"zqfan","email":"aji.zqfan@gmail.com","username":"aji-zqfan"},"change_message_id":"2ab3160a2fb83274c08c492b5b3c5a7ca39e15e3","unresolved":false,"context_lines":[{"line_number":432,"context_line":"        session \u003d self._engine_facade.get_session()"},{"line_number":433,"context_line":"        # get list of resource_ids"},{"line_number":434,"context_line":"        has_timestamp \u003d start_timestamp or end_timestamp"},{"line_number":435,"context_line":"        if cfg.CONF.sql_expire_samples_only or has_timestamp:"},{"line_number":436,"context_line":"            res_q \u003d session.query(distinct(models.Resource.resource_id)).join("},{"line_number":437,"context_line":"                models.Sample,"},{"line_number":438,"context_line":"                models.Sample.resource_id \u003d\u003d models.Resource.internal_id)"}],"source_content_type":"text/x-python","patch_set":3,"id":"3a29b11f_535bb72e","line":435,"in_reply_to":"3a29b11f_ff088c2f","updated":"2015-10-26 11:17:23.000000000","message":"reasonable","commit_id":"860c8cdc509e990bab5c2f507f3b1cfb4abdabee"}]}