ceilometer.storage.hbase.utils.prepare_key fail on 32-bits system

Bug #1388181 reported by ZhiQiang Fan
6
This bug affects 1 person
Affects Status Importance Assigned to Milestone
Ceilometer
Fix Released
Medium
ZhiQiang Fan

Bug Description

when run tox on 32-bits ubuntu 12.04 system, it fails

for i.e:

tox -edebug -- ceilometer.tests.api.v2.test_alarm_scenarios.TestAlarms.test_alarms_sends_notification

see: http://paste.openstack.org/show/127699/

the problem is that:
ceilometer.storage.hbase.utils.prepare_key thinks parameters are string or int object,
def prepare_key(*args):
    """Prepares names for rows and columns with correct separator.

    :param args: strings or numbers that we want our key construct of
    :return: key with quoted args that are separated with character ":"
    """
    key_quote = []
    for key in args:
        if isinstance(key, int):
            key = str(key)
        key_quote.append(quote(key))
    return ":".join(key_quote)

but ceilometer/alarm/storage/impl_hbase.py method record_alarm_change() will pass args:
def record_alarm_change(self, alarm_change):
        """Record alarm change event."""
        alarm_change_dict = hbase_utils.serialize_entry(alarm_change)
        ts = alarm_change.get('timestamp') or datetime.datetime.now()
        rts = hbase_utils.timestamp(ts)
        with self.conn_pool.connection() as conn:
            alarm_history_table = conn.table(self.ALARM_HISTORY_TABLE)
            alarm_history_table.put(
                hbase_utils.prepare_key(alarm_change.get('alarm_id'), rts),
                alarm_change_dict)

but
def timestamp(dt, reverse=True):
    """Timestamp is count of milliseconds since start of epoch.

    If reverse=True then timestamp will be reversed. Such a technique is used
    in HBase rowkey design when period queries are required. Because of the
    fact that rows are sorted lexicographically it's possible to vary whether
    the 'oldest' entries will be on top of the table or it should be the newest
    ones (reversed timestamp case).

    :param dt: datetime which is translated to timestamp
    :param reverse: a boolean parameter for reverse or straight count of
      timestamp in milliseconds
    :return: count or reversed count of milliseconds since start of epoch
    """
    epoch = datetime.datetime(1970, 1, 1)
    td = dt - epoch
    ts = td.microseconds + td.seconds * 1000000 + td.days * 86400000000
    return 0x7fffffffffffffff - ts if reverse else ts

the type of return value of timestamp is different on 32-bits system and 64-bits system, on 32-bits system, the return value is too large, which will finally be long object

Are we going to support 32-bits as well?
Since code are run on 64-bits server, this is not a problem, but for some poor developer like me who only has slow laptop, it is not convenient because my local test will fail forever.....

The fix is quite simple, add another instance check for long type, then it is ok to go

ZhiQiang Fan (aji-zqfan)
Changed in devstack:
assignee: nobody → ZhiQiang Fan (aji-zqfan)
ZhiQiang Fan (aji-zqfan)
affects: devstack → ceilometer
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/132387

Changed in ceilometer:
status: New → In Progress
Revision history for this message
OpenStack Infra (hudson-openstack) wrote : Fix merged to ceilometer (master)

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

commit fa982d1bf5fe6c9657fe4a2c5eb48c65bdac26da
Author: ZhiQiang Fan <email address hidden>
Date: Sat Nov 1 14:40:43 2014 +0800

    Fix storage.hbase.util.prepare_key() for 32-bits system

    storage.hbase.utils.timestamp() will return long type object on 32-bits
    system, which will cause exception in ceilometer/alarm/storage/impl_hbase.py:
    'long' object has no attribute 'encode'.
    Then developers on 32-bits system will never get local test passed.

    This patch changes type check in prepare_keys() from int to six.integer_types,
    which will work for both 32-bits and 64-bits system, python2 and python3
    environment.

    Note: no test code is added, because jenkins runs on 64-bits system. Reviewers
    can download code and verify it on 32-bits system.

    Change-Id: I57729ff67efe6d6036fe698e3d86491f9ed4600c
    Closes-Bug: #1388181

Changed in ceilometer:
status: In Progress → Fix Committed
Eoghan Glynn (eglynn)
Changed in ceilometer:
milestone: none → kilo-1
importance: Undecided → Medium
Thierry Carrez (ttx)
Changed in ceilometer:
status: Fix Committed → Fix Released
Thierry Carrez (ttx)
Changed in ceilometer:
milestone: kilo-1 → 2015.1.0
To post a comment you must log in.
This report contains Public information  
Everyone can see this information.

Other bug subscribers

Remote bug watches

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