Alona Kaplan has posted comments on this change. Change subject: engine: Add NetworkAttachment dao ......................................................................
Patch Set 19: (1 comment) https://gerrit.ovirt.org/#/c/32581/19/packaging/dbscripts/upgrade/03_06_1170_add_network_attachment.sql File packaging/dbscripts/upgrade/03_06_1170_add_network_attachment.sql: Line 12: gateway CHARACTER VARYING(20), Line 13: custom_properties TEXT, Line 14: _create_date TIMESTAMP WITH TIME ZONE DEFAULT ('now'::text)::timestamp without time zone, Line 15: _update_date TIMESTAMP WITH TIME ZONE, Line 16: FOREIGN KEY (network_id) REFERENCES network(id) ON DELETE CASCADE, When removing a labeled network/detaching it from a cluster, we are trying to detach the network from the host (calling to setup networks). 1. In RemoveNetworkCommand you first remove the network and then calling the setupnetworks commands. It is problematic since you have ON DELETE CASCADE on the network. It means that ones the network will be removed the attachment will also be removed, which means that if the setup networks will try to get the attachment form the db it will fail, and probably the setup network command will fail on can do action. 2. Currently, both RemoveNetworkCommand and DetachNetworkToVdsGroupCommand are calling the old setup networks command so there is no bug (the old setup networks command doesn't care that the attachment was removed). But since our goal is to use the new command (I've added it to the TODO list) it should be fixed. I suggest leaving the on DELETE CASCADE, and refactoring the RemoveNetworkCommand so that it will first call the setup networks commands (since it is multiple action it will return once all the canDo completed) and just then removing the network from the db (which will cause the removal of the attachments). In this approach we don't need compensation. The only problem I see in this suggestion is that we have to assume there is no attempts to get the attachment from the db in the execution stage of the host setup network command- it should be well documented. Matrin, Moti- what do you think about the suggestion? Line 17: FOREIGN KEY (nic_id) REFERENCES vds_interface(id) ON DELETE SET NULL Line 18: ); Line 19: Line 20: CREATE INDEX IDX_network_attachments_nic_id ON network_attachments(nic_id); -- To view, visit https://gerrit.ovirt.org/32581 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I84187f3900b9e3f8a917fc4e4126de9e50e231b4 Gerrit-PatchSet: 19 Gerrit-Project: ovirt-engine Gerrit-Branch: master Gerrit-Owner: Moti Asayag <[email protected]> Gerrit-Reviewer: Alona Kaplan <[email protected]> Gerrit-Reviewer: Eli Mesika <[email protected]> Gerrit-Reviewer: Jenkins CI Gerrit-Reviewer: Martin Mucha <[email protected]> Gerrit-Reviewer: [email protected] Gerrit-HasComments: Yes _______________________________________________ Engine-patches mailing list [email protected] http://lists.ovirt.org/mailman/listinfo/engine-patches
