resource-list doesn't need to join sample table if no timestamp is specified (mysql backend)

Bug #1509677 reported by ZhiQiang Fan
12
This bug affects 1 person
Affects Status Importance Assigned to Milestone
Ceilometer
Fix Released
Undecided
ZhiQiang Fan
Kilo
Fix Released
Undecided
Unassigned

Bug Description

for sqlalchemy implementation, currently, even there is no timestamp is specified, we still inner join sample table, this is actually not needed

https://github.com/openstack/ceilometer/blob/b34865f80818165187552e7feca4ead2e61a30d3/ceilometer/storage/impl_sqlalchemy.py#L434

ZhiQiang Fan (aji-zqfan)
Changed in ceilometer:
assignee: nobody → ZhiQiang Fan (aji-zqfan)
Revision history for this message
OpenStack Infra (hudson-openstack) wrote : Fix proposed to ceilometer (master)

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

Changed in ceilometer:
status: New → In Progress
Revision history for this message
Rohit Jaiswal (rohit-jaiswal-3) wrote :

IIUC the inner join is still needed to filter out resources that do not have any samples, otherwise [1] will loop over resources with no samples and generate 2 queries per resource that dont return anything..

[1] https://github.com/openstack/ceilometer/blob/b34865f80818165187552e7feca4ead2e61a30d3/ceilometer/storage/impl_sqlalchemy.py#L440

Revision history for this message
ZhiQiang Fan (aji-zqfan) wrote :

@Rohit

Yes, that would happend when cfg.CONF.sql_expire_samples_only is true, then there will be resources without any samples, I will update the patch to reflect such scenario

Thanks!

Revision history for this message
OpenStack Infra (hudson-openstack) wrote : Fix merged to ceilometer (master)

Reviewed: https://review.openstack.org/239183
Committed: https://git.openstack.org/cgit/openstack/ceilometer/commit/?id=a4f442940551a8a38e26e132c7efa766226daa15
Submitter: Jenkins
Branch: master

commit a4f442940551a8a38e26e132c7efa766226daa15
Author: ZhiQiang Fan <email address hidden>
Date: Sat Oct 24 09:30:53 2015 -0600

    avoid unnecessary inner join in get_resources() for SQL backend

    To get distinct resource ids, we do a query on resource table which
    inner join sample table, and apply filters on it.

    Note that when sql_expire_samples_only is enabled, there will be
    some resources without any sample, in such case we must use inner
    join to avoid wrong result, no matter if there is a timestamp filter
    or not.

    But that option is disabled by default, so when there is no timestamp
    filters, the inner join is unnecessary, we should avoid it to save
    some RAM/CPU cost.

    Change-Id: If85dbea15d42d42c6b0be7402c06f258e278b2eb
    Closes-Bug: #1509677

Changed in ceilometer:
status: In Progress → Fix Committed
Revision history for this message
OpenStack Infra (hudson-openstack) wrote : Fix proposed to ceilometer (stable/liberty)

Fix proposed to branch: stable/liberty
Review: https://review.openstack.org/242959

Revision history for this message
OpenStack Infra (hudson-openstack) wrote : Fix proposed to ceilometer (stable/kilo)

Fix proposed to branch: stable/kilo
Review: https://review.openstack.org/242962

Revision history for this message
OpenStack Infra (hudson-openstack) wrote : Fix merged to ceilometer (stable/kilo)

Reviewed: https://review.openstack.org/242962
Committed: https://git.openstack.org/cgit/openstack/ceilometer/commit/?id=14586b2eac909dd76431e4eb9ff00b8f5a4c2d0c
Submitter: Jenkins
Branch: stable/kilo

commit 14586b2eac909dd76431e4eb9ff00b8f5a4c2d0c
Author: ZhiQiang Fan <email address hidden>
Date: Sat Oct 24 09:30:53 2015 -0600

    avoid unnecessary inner join in get_resources() for SQL backend

    To get distinct resource ids, we do a query on resource table which
    inner join sample table, and apply filters on it.

    Note that when sql_expire_samples_only is enabled, there will be
    some resources without any sample, in such case we must use inner
    join to avoid wrong result, no matter if there is a timestamp filter
    or not.

    But that option is disabled by default, so when there is no timestamp
    filters, the inner join is unnecessary, we should avoid it to save
    some RAM/CPU cost.

    Change-Id: If85dbea15d42d42c6b0be7402c06f258e278b2eb
    Closes-Bug: #1509677
    (cherry picked from commit a4f442940551a8a38e26e132c7efa766226daa15)

tags: added: in-stable-kilo
Revision history for this message
OpenStack Infra (hudson-openstack) wrote : Fix merged to ceilometer (stable/liberty)

Reviewed: https://review.openstack.org/242959
Committed: https://git.openstack.org/cgit/openstack/ceilometer/commit/?id=81dded73ce81b9c62e196003f72451338db65649
Submitter: Jenkins
Branch: stable/liberty

commit 81dded73ce81b9c62e196003f72451338db65649
Author: ZhiQiang Fan <email address hidden>
Date: Sat Oct 24 09:30:53 2015 -0600

    avoid unnecessary inner join in get_resources() for SQL backend

    To get distinct resource ids, we do a query on resource table which
    inner join sample table, and apply filters on it.

    Note that when sql_expire_samples_only is enabled, there will be
    some resources without any sample, in such case we must use inner
    join to avoid wrong result, no matter if there is a timestamp filter
    or not.

    But that option is disabled by default, so when there is no timestamp
    filters, the inner join is unnecessary, we should avoid it to save
    some RAM/CPU cost.

    Change-Id: If85dbea15d42d42c6b0be7402c06f258e278b2eb
    Closes-Bug: #1509677
    (cherry picked from commit a4f442940551a8a38e26e132c7efa766226daa15)

tags: added: in-stable-liberty
Revision history for this message
Thierry Carrez (ttx) wrote : Fix included in openstack/ceilometer 6.0.0.0b1

This issue was fixed in the openstack/ceilometer 6.0.0.0b1 development milestone.

Thierry Carrez (ttx)
Changed in ceilometer:
status: Fix Committed → Fix Released
Revision history for this message
Doug Hellmann (doug-hellmann) wrote : Fix included in openstack/ceilometer 5.0.1

This issue was fixed in the openstack/ceilometer 5.0.1 release.

Liusheng (liusheng)
Changed in ceilometer:
milestone: none → mitaka-1
To post a comment you must log in.
This report contains Public information  
Everyone can see this information.

Duplicates of this bug

Other bug subscribers

Remote bug watches

Bug watches keep track of this bug in other bug trackers.