db_plugin.delete_ports() can lead to long transaction if plugin.deleete_port talks with external system

[Akihiro Motoki]

db_plugin.delete_ports() can lead to long transaction if plugin.delete_port talks with external system.
it is observed first in nec plugin (bug 1282922), but it affects multiple plugins/drivers.

Note that it is about delete_ports and not about delete_port.

The detail is described in bug 1282922. Quoted from the original bug report.
----
The case I observed is that delete-port from dhcp-agent (release_dhcp_port RPC call) and delete-port from delete-network API request are run in parallel. plugin.delete-port in nec plugin calls REST API call to an external controller in addition to operates on neutron database.

After my investigation and testing, db_plugin.delete_ports() calls plugin.delete_port() under a transaction.
https://github.com/openstack/neutron/blob/master/neutron/db/db_base_plugin_v2.py#L1367
This means the transaction continues over API calls to external controller and it leads to a long transaction.
When plugin.delete_ports() and plugin.delete_port() are run at the same time, even if plugin.delete_port() avoid long transaction, db operations in plugin.delete_port() is blocked and they can fail with timeout.
----

[Akihiro Motoki (amotoki)]

The adhoc approach is to remove the transaction from db_plugin.delete_ports().
This removes long transaction even when plugin.delete_port talks with external systems.
At now I believe it works because only release_dhcp_port() in db/dhcp_rpc_base which handle dhcp-agent RPC and dhcp-port will be cleaned up in delete_network even if some dhcp ports are not deleted successfully in release_dhcp_port.

Better fix is to support bulk operations in plugin.delete_ports().
It will provide a more consistent way to handle multiple resources at one operation.

Thought?

[OpenStack Infra (hudson-openstack)]

Related fix proposed to branch: master
Review: https://review.openstack.org/75343

[OpenStack Infra (hudson-openstack)]

Reviewed: https://review.openstack.org/75343
Committed: https://git.openstack.org/cgit/openstack/neutron/commit/?id=64acc3bd63846a6e7da8d1136f946372c698cb76
Submitter: Jenkins
Branch: master

commit 64acc3bd63846a6e7da8d1136f946372c698cb76
Author: Akihiro Motoki <email address hidden>
Date: Fri Feb 21 17:42:46 2014 +0900

    nec plugin: Avoid long transaction in delete_ports

    db_plugin.delete_ports() can lead to long transaction
    if plugin.deleete_port talks with external system.
    This commit removes a transaction in delete_ports and
    allows NEC plugin to use more granular db transactions
    in delete_port. It greatly helps db race conditions and
    timeouts in delete_port operations.

    To avoid to impact other plugins/drivers by changing
    db_plugin.delete_ports directly and to land this patch soon,
    this commit overrides delete_ports() in NEC plugin.
    Further disssion on transaction in delete_ports will be
    discussed under bug 1282925.

    Closes-Bug: #1282922
    Related-Bug: #1282925

    Change-Id: I2c00694ad34eb2058bf7a0ff1c920ceded327d43

[Armando Migliaccio (armando-migliaccio)]

I have this one occur on delete_network operations too. Some stacktraces can be found here:

http://208.91.1.172/logs/neutron/52983/33/4011499/logs/screen-q-svc.txt.gz?level=TRACE#_2014-03-06_17_17_53_570
http://208.91.1.172/logs/neutron/78694/1/4011497/logs/screen-q-svc.txt.gz?level=TRACE#_2014-03-06_17_13_35_784

[Akihiro Motoki (amotoki)]

@armax

Thanks for sharing information.
The stacktraces are exactly same as what I experienced.

What do you think about removing transaction from delete_port"s"?
I think we don't need to delete all ports under one transaction.
It is used only by release_dhcp_ports at the moment, and each dhcp port can be deleted separately.

It is row risk and I believe we can get a good behavior.

[OpenStack Infra (hudson-openstack)]

Fix proposed to branch: master
Review: https://review.openstack.org/78880

[OpenStack Infra (hudson-openstack)]

Reviewed: https://review.openstack.org/78880
Committed: https://git.openstack.org/cgit/openstack/neutron/commit/?id=5e4b0c6fc6670ea036d801ce53444272bc311929
Submitter: Jenkins
Branch: master

commit 5e4b0c6fc6670ea036d801ce53444272bc311929
Author: Akihiro Motoki <email address hidden>
Date: Fri Mar 7 15:58:46 2014 +0900

    Avoid long transaction in plugin.delete_ports()

    db_plugin.delete_ports() called plugin.delete_port() under
    a transaction. It leads to long transaction if plugin.delete_port
    talks with external systems. This commit changes each delete_port
    outside of a transaction to avoid longer transaction.

    plugin.delete_ports is now called by release_dhcp_ports and
    dhcp-agent ports can be deleted separately, so this changes
    does not break the existing behavior.

    delete_ports is renamed to delete_ports_by_device_id
    to clarify the usage of this method.

    NEC plugin already has this change and it is no longer needed.

    _do_side_effect helper method in test_db_plugin is renamed
    to more self-descriptive name.

    Change-Id: Ied5883a57c7774c3b0778453d84c717b337f88c0
    Closes-Bug: #1282925
    Related-Bug: #1283522