Notification sending sometimes hits the keystone API to get glance endpoints

Bug #1757407 reported by Balazs Gibizer on 2018-03-21
6
This bug affects 1 person
Affects Status Importance Assigned to Milestone
OpenStack Compute (nova)
Low
Balazs Gibizer

Bug Description

During the investigation of another bug [1] we noticed that notification sending could trigger keystone API call if the glance/api_server config is not present in the nova.conf . The notification sending code paths[2][3] calls info_from_instance [4] that leads to the glance client get_api_servers function that falls back to keystone to get the endpoints if the above config is not present.

The versioned notifications do not use the glance endpoint information. However even if the notifications/notification_format config options is set to versioned, nova still hits keystone via the instance.exists notification codepath [3] as that path is shared between versioned and unversioned notifications.

This leads to an unnecessary REST API call where the result is not used so the caused performance loss is totally unnecessary.

[1] https://bugs.launchpad.net/nova/+bug/1753550
[2] https://github.com/openstack/nova/blob/db0747591ce8df1b0ca62aac0648b7154fed1f86/nova/compute/utils.py#L305
[3] https://github.com/openstack/nova/blob/6eccfb7c01b7e984cb18c7b75bd20a589dfdfe3d/nova/notifications/base.py#L212
[4] https://github.com/openstack/nova/blob/6eccfb7c01b7e984cb18c7b75bd20a589dfdfe3d/nova/notifications/base.py#L381
[5] https://github.com/openstack/nova/blob/24379f1822e3ae1d4f7c8398e60af6e52b386c32/nova/image/glance.py#L126

tags: added: notifications
Matt Riedemann (mriedem) wrote :

(10:21:19 AM) mriedem: gibi: yeah so we could optimize to not even do that lookup if only using versioned notifications,
(10:21:40 AM) mriedem: also, efried said the glance endpoint / service catalog information should be cached in ksa, so we shouldn't be hitting the keystone API every time, only the first time,
(10:22:09 AM) mriedem: but it's curious that we could create a server (which would fetch the image on the compute) and then the periodic (without a token) would have problems stopping it
(10:22:32 AM) mriedem: unless you did something like had (1) cached images on the compute or (2) restarted nova-compute in between to invalidate the ksa cache
(10:22:51 AM) mriedem: unless the cache has a timer on it? or is somehow otherwise request-specific

Changed in nova:
status: New → Confirmed
importance: Undecided → Low
Matt Riedemann (mriedem) wrote :

(10:24:54 AM) efried: mriedem, gibi: The caching would be specific to the context. So you would be hitting the endpoint (to do version discovery) once per unique context (as opposed to just the first time overall).

Which explains why the context generated via the periodic task doesn't get to leverage the cache.

Balazs Gibizer (balazs-gibizer) wrote :

We have to consider the backport potential of this bug.

Changed in nova:
assignee: nobody → Balazs Gibizer (balazs-gibizer)

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

Changed in nova:
status: Confirmed → In Progress
Changed in nova:
assignee: Balazs Gibizer (balazs-gibizer) → Matt Riedemann (mriedem)

Reviewed: https://review.openstack.org/564528
Committed: https://git.openstack.org/cgit/openstack/nova/commit/?id=93b897348bde072969f67e43875ce08e5d420b8a
Submitter: Zuul
Branch: master

commit 93b897348bde072969f67e43875ce08e5d420b8a
Author: Balazs Gibizer <email address hidden>
Date: Thu Apr 26 16:55:15 2018 +0200

    Call generate_image_url only for legacy notification

    The legacy instance.exists notification includes the full url of the glance
    image of the given instance. But the versioned notification only includes
    the image uuid. Generating the full url can be a costly operation as it
    needs to talk to Keystone.

    So this patch makes sure that generate_image_url only called when the
    generated information will be used.

    Change-Id: I78c2a34b3d03438457cc968cd0a38b8131e4f6e6
    Closes-Bug: #1757407

Changed in nova:
status: In Progress → Fix Released
Matt Riedemann (mriedem) on 2018-07-17
Changed in nova:
assignee: Matt Riedemann (mriedem) → Balazs Gibizer (balazs-gibizer)

This issue was fixed in the openstack/nova 18.0.0.0b3 development milestone.

Reviewed: https://review.openstack.org/584969
Committed: https://git.openstack.org/cgit/openstack/nova/commit/?id=59075b8243e869774e29bd3b378723947c0b3a20
Submitter: Zuul
Branch: stable/queens

commit 59075b8243e869774e29bd3b378723947c0b3a20
Author: Balazs Gibizer <email address hidden>
Date: Thu Apr 26 16:55:15 2018 +0200

    Call generate_image_url only for legacy notification

    The legacy instance.exists notification includes the full url of the glance
    image of the given instance. But the versioned notification only includes
    the image uuid. Generating the full url can be a costly operation as it
    needs to talk to Keystone.

    So this patch makes sure that generate_image_url only called when the
    generated information will be used.

     Conflicts:
     nova/compute/utils.py
     nova/notifications/base.py
     nova/tests/unit/notifications/test_base.py

    NOTE(elod.illes): conflict caused by parameter system_metadata which is not
    there on master anymore.

    Change-Id: I78c2a34b3d03438457cc968cd0a38b8131e4f6e6
    Closes-Bug: #1757407
    (cherry picked from commit 93b897348bde072969f67e43875ce08e5d420b8a)

tags: added: in-stable-queens

This issue was fixed in the openstack/nova 17.0.6 release.

To post a comment you must log in.
This report contains Public information  Edit
Everyone can see this information.

Other bug subscribers