| Posted | Nick | Remark | |
|---|---|---|---|
| #openstack-nova - 2019-02-06 | |||
| 07:55:32 | openstackgerrit | Jan Gutter proposed openstack/nova master: Ignore some PendingDeprecationWarnings for os-vif https://review.openstack.org/634928 | |
| 08:20:15 | openstackgerrit | Jan Gutter proposed openstack/os-vif master: Convert hardcoded regexes to raw strings for py36 https://review.openstack.org/635111 | |
| 10:10:34 | openstackgerrit | Surya Seetharaman proposed openstack/nova master: API microversion 2.68: Handles Down Cells https://review.openstack.org/591657 | |
| 10:10:34 | openstackgerrit | Surya Seetharaman proposed openstack/nova master: Modify InstanceMappingList.get_not_deleted_by_cell_and_project() https://review.openstack.org/635120 | |
| 10:10:35 | openstackgerrit | Surya Seetharaman proposed openstack/nova master: Plumbing for ignoring list_records_by_skipping_down_cells https://review.openstack.org/635121 | |
| 10:53:54 | openstackgerrit | Andrey Volkov proposed openstack/nova master: Check hosts have no instances for AZ rename https://review.openstack.org/509206 | |
| 11:18:06 | openstackgerrit | Stephen Finucane proposed openstack/python-novaclient master: Microversion 2.68: Remove 'forced' live migrations, evacuations https://review.openstack.org/635131 | |
| 11:22:03 | openstackgerrit | Stephen Finucane proposed openstack/nova master: API: Remove evacuate/live-migrate 'force' parameter https://review.openstack.org/634600 | |
| 12:41:53 | openstackgerrit | Surya Seetharaman proposed openstack/nova master: Plumbing for ignoring list_records_by_skipping_down_cells https://review.openstack.org/635121 | |
| 12:41:54 | openstackgerrit | Surya Seetharaman proposed openstack/nova master: API microversion 2.68: Handles Down Cells https://review.openstack.org/591657 | |
| 12:41:54 | openstackgerrit | Surya Seetharaman proposed openstack/nova master: Plumbing for allowing the all-tenants filter https://review.openstack.org/635145 | |
| 12:41:55 | openstackgerrit | Surya Seetharaman proposed openstack/nova master: Plumbing required in ViewBuilder to construct partial results https://review.openstack.org/635146 | |
| 12:41:55 | openstackgerrit | Surya Seetharaman proposed openstack/nova master: API microversion 2.68: Handles Down Cells Documentation https://review.openstack.org/635147 | |
| 13:10:30 | openstackgerrit | Artom Lifshitz proposed openstack/nova master: [WIP] Create claims when live migrating https://review.openstack.org/634606 | |
| 13:10:31 | openstackgerrit | Artom Lifshitz proposed openstack/nova master: [WIP] New objects to transmit NUMA config from dest to source https://review.openstack.org/634827 | |
| 13:10:31 | openstackgerrit | Artom Lifshitz proposed openstack/nova master: [WIP] Send dest NUMA info inside LiveMigrateData https://review.openstack.org/634828 | |
| 13:45:07 | bauzas | good afternoon Nova, now begins my upstream day | |
| 13:46:12 | sean-k-mooney | o/ hello upstream bauzas :) | |
| 13:46:26 | bauzas | sylvainb != bauzas :) | |
| 13:46:41 | bauzas | so 'upstream bauzas' is just a pleonasm | |
| 13:47:40 | sean-k-mooney | bauzas: the real question is who is the evil one and who is the good one sylvainb or bauzas | |
| 13:48:02 | bauzas | it's schizophrenic, I know | |
| 13:48:16 | bauzas | mwuaaahahahah | |
| 13:49:50 | mriedem | tssurya: i could be wrong but it seems the commit message and comment in https://review.openstack.org/#/c/635120/ are backward | |
| 13:50:53 | sean-k-mooney | bauzas: the real question is who is the evil one and who is the good one sylvainb or bauzas | |
| 13:51:26 | bauzas | I can't answer this question without the presence of my lawyer | |
| 14:26:56 | tssurya | mriedem: um, no I meant for the ones where the data migration is run, it would be False/True while for the new ones its None/NULL at least in the database.. | |
| 14:27:05 | tssurya | I'll update it on the review. | |
| 14:29:02 | mriedem | i'm not sure what "new ones" means | |
| 14:29:07 | mriedem | new instance mappings should have a default of false | |
| 14:29:11 | mriedem | existing records would have None | |
| 14:29:15 | mriedem | and those are what get migrated | |
| 14:29:22 | tssurya | like fresh instance_mappings from Stein | |
| 14:29:30 | tssurya | that is what I mean by new | |
| 14:29:47 | mriedem | and those should have queued_for_delete=False in the db by default | |
| 14:29:55 | tssurya | which they don't | |
| 14:30:05 | mriedem | ? | |
| 14:30:20 | tssurya | I thought they should have qfd=False by default too | |
| 14:30:30 | tssurya | but in the new devstack in which I tested they are NULL | |
| 14:30:32 | tssurya | by default | |
| 14:30:33 | mriedem | if they don't, then we have a problem because we'd never be able to complete that data migration | |
| 14:30:54 | tssurya | well I am assuming once we run the migration, we would not run it again | |
| 14:31:20 | mriedem | but if you're saying that new instance mappings records don't have queued_for_delete=false in the db, then we'd have to migrate those as well | |
| 14:31:33 | mriedem | b/c the migration is looking for queued_for_delete=None | |
| 14:31:33 | tssurya | would have to ask dansmith | |
| 14:31:57 | tssurya | but I am thinking for the newer ones None also means not queued_for_delete | |
| 14:32:17 | tssurya | and when they get deleted it would become True | |
| 14:32:32 | dansmith | why ask me? :) | |
| 14:33:08 | dansmith | if we're writing the qfd flag on object save, then new records should have =False, if not then it depends on whether or not we have a default in the schema | |
| 14:33:18 | tssurya | https://review.openstack.org/#/c/584504/5//COMMIT_MSG@16 | |
| 14:34:24 | dansmith | oh ;) | |
| 14:34:39 | dansmith | yeah, I remember this being some weird detail when writing that migration but I don't remember why | |
| 14:35:00 | tssurya | to catch all the None and change them to True/False | |
| 14:35:19 | tssurya | which made sense | |
| 14:35:34 | tssurya | so I am assuming we were ok with have new records that would be None by default right ? | |
| 14:35:38 | tssurya | having* | |
| 14:35:38 | mriedem | no | |
| 14:35:46 | mriedem | because then the data migration will never be complet | |
| 14:35:48 | mriedem | *complete | |
| 14:36:09 | mriedem | new records should have False in the db when we insert the record | |
| 14:36:17 | dansmith | oh right right | |
| 14:36:22 | tssurya | mriedem: yea the data migration will never be complete, but I am assuming it will be removed eventually and the goal of that migration was to update the deleted instances to True | |
| 14:36:28 | dansmith | because they need to be either true or false null means unmigrated | |
| 14:36:55 | dansmith | tssurya: no, I think the intent is to update them to true or false right? | |
| 14:36:56 | tssurya | heh ok I didn't think it was bug, but maybe it is ? :D | |
| 14:37:07 | tssurya | dansmith: yea for the old instances | |
| 14:37:21 | mriedem | the data migration also sets queued_for_delete=False on instance mappings for non-deleted instances | |
| 14:37:28 | mriedem | https://review.openstack.org/#/c/584504/5/nova/objects/instance_mapping.py@179 | |
| 14:37:41 | tssurya | but we don't care for the new ones right ? for them None = False () sort of and when they get deleted it would become True() from None that's all | |
| 14:38:06 | tssurya | mriedem: yea I understand | |
| 14:38:16 | tssurya | if we feel its a bug for the migration to be always incomplete | |
| 14:38:53 | mriedem | well generally data migrations are something you'd like to have an end state | |
| 14:39:01 | tssurya | then we can fix. | |
| 14:39:06 | tssurya | (anu suggestions ?) | |
| 14:39:08 | tssurya | any* | |
| 14:39:59 | dansmith | I'm confused about what the problem is | |
| 14:40:44 | mriedem | started with my confusion over the commit message and comments in https://review.openstack.org/#/c/635120/ | |
| 14:41:03 | openstackgerrit | Stephen Finucane proposed openstack/nova master: Remove get_config_vhostuser https://review.openstack.org/565471 | |
| 14:41:03 | openstackgerrit | Stephen Finucane proposed openstack/nova master: Set bandwidth limits for generic vHost User interfaces https://review.openstack.org/635170 | |
| 14:41:04 | mriedem | and tssurya said that with a new instance mapping in devstack today, the queued_for_delete in the DB is NULL even though we default to False in the object and sqla model code | |
| 14:41:12 | mriedem | which ^ doesn't make sense to me | |
| 14:41:24 | mriedem | i get that we don't set False for the server_default | |
| 14:41:36 | mriedem | but inserting new records should have a default value of False in the DB i'd think | |
| 14:41:41 | stephenfin | mriedem, jaypipes: I've split out that bandwidth limits fix to a separate patch, if you fancy taking a quick look over it again https://review.openstack.org/565471 https://review.openstack.org/635170 | |
| 14:42:03 | mriedem | stephenfin: only if i can raise my pinky while reviewing it | |
| 14:42:04 | tssurya | mriedem: we don't default to false in the api_models code right ? that was changed for the migration | |
| 14:42:16 | tssurya | which is why this is happening I think | |
| 14:42:22 | dansmith | tssurya: right but we should be passing it from the object always | |
| 14:42:29 | dansmith | if not, then that's a problem | |
| 14:42:32 | mriedem | ok then i missed https://review.openstack.org/#/c/584504/5/nova/db/sqlalchemy/api_models.py | |
| 14:43:11 | mriedem | we're not passing it from the object b/c the field isn't set | |
| 14:43:18 | mriedem | https://review.openstack.org/#/c/584504/5/nova/objects/instance_mapping.py@102 | |
| 14:43:59 | mriedem | https://review.openstack.org/#/c/584504/5/nova/compute/api.py@938 | |
| 14:44:00 | dansmith | we have a default on the field, but that doesn't mean it gets set, | |
| 14:44:05 | mriedem | right | |
| 14:44:06 | dansmith | so maybe whoever wrote that code thought it would be | |
| 14:44:07 | mriedem | so it's not in the changes | |
| 14:44:30 | mriedem | anyway it's a simple fix | |
| 14:44:51 | dansmith | yeah, so create() should always set that =False and destroy should set it =True | |
| 14:44:53 | mriedem | changes.setdefault('queued_for_delete', False) | |