Low datetime precision leads to inconsistent order of shares

[Dmitry Galkin]

Hi,

We have recently migrated Manila from Postgres to MariaDB and the side-effect of this migration is that the created_at | updated_at | deleted_at columns of all objects have only up-to-1 second precision.

In some big projects we have now multiple shares that have exactly the same created_at timestamp. Which, in turn, means we get inconsistent order of shares when those are listed via API:

Example Shares with the same timestamp:
MariaDB [manila]> SELECT created_at FROM shares GROUP BY created_at HAVING COUNT(*) >1;
+---------------------+
| created_at |
+---------------------+
| 2018-06-05 10:35:29 |
| 2018-06-07 15:22:40 |
| 2018-06-21 05:24:33 |
| 2018-06-26 10:14:05 |
...
| 2020-01-13 13:46:48 |
| 2020-01-13 13:49:25 |
+---------------------+
1713 rows in set (0.011 sec)

MariaDB [manila]> show columns from shares;
+---------------------------------------+--------------+------+-----+---------+-------+
| Field | Type | Null | Key | Default | Extra |
+---------------------------------------+--------------+------+-----+---------+-------+
| created_at | datetime | YES | | NULL | |
| updated_at | datetime | YES | | NULL | |
| deleted_at | datetime | YES | | NULL | |
...

The list sort_key is 'created_at' by default: https://github.com/openstack/manila/blob/stable/train/manila/db/sqlalchemy/api.py#L1845

-> that results that two shares with the same 'created_at' timestamp will often "flip" their positions in the list between two subsequent API calls with the same parameters.
This might not be a big issue at first glance, but when used with "--limit" that causes problems, since a share might "get lost" from the list if it is located on the edge of the limited list:

Sample script to reproduce:

$ for i in {01..10}; do curl -sH "X-Auth-Token: ${OS_AUTH_TOKEN}" https://your_manila_api/v2/980a837d27b04c58a71d506a45c365a9/shares'?project_id=7c0bc39982ba42b59dfa32093bebef1f&all_tenants=1&limit=10&offset=240' | jq -r '.shares[] | .id' | tee pass$i.txt; done

Easy to see by the diff:
(shares were not created or deleted during the test)

$ diff -u5 pass01.txt pass02.txt
--- pass01.txt 2019-12-11 14:59:15.089967267 +0100
+++ pass02.txt 2019-12-11 14:59:17.026663666 +0100
@@ -1,10 +1,10 @@
 2a27b629-a485-4507-8db0-c7cab22760c3
 5e4cd8bd-60ef-433d-893a-556160745f77
 613de97d-b9fa-4e47-99d7-67091055e430
 1b1a5309-8237-44ab-86e3-cb80e7de8e03
-4ae1c17a-a8a4-4f42-aadc-c8d49496be4f
 774af312-2513-47b1-aed7-8eb3a01d7f9c
+4ae1c17a-a8a4-4f42-aadc-c8d49496be4f
 fb7e5dde-93ed-4831-b006-f34ebfe8eedc
-0ac73939-c8d1-499b-ad7e-56fcdfb578ce
-0c3a57f4-8b7a-4f59-abe4-93af0d0b3d20
 20889446-3d23-4419-b28e-90f9cc6edba9
+158e73c2-643e-4652-883c-99e5cf7483aa
+0c3a57f4-8b7a-4f59-abe4-93af0d0b3d20

The workaround I did so far is altering the created_at|updated_at|deleted_at columns type for shares to be DATETIME(6) and all new shares are created with more-precise timestamps now:

Name testshare
Description
ID 589e24a8-36c4-4a5f-97b1-d5ec4bc82fec
....
Created At 2020-01-08T09:13:45.765670

The respective DB migration is attached as a file. Can be easily extended to all manila objects. Possible alternative is to change the default sort key to id, but this is an API change.

Please share your thoughts.

[Goutham Pacha Ravi (gouthamr)]

Hi Dmitry,

Thank you so much for reporting this bug! We did take a look at this a while ago, but for some reason we haven't updated this bug with the triage [1]

I think your fix is appropriate; would you be kind to propose the patch?

[1] http://eavesdrop.openstack.org/meetings/manila/2020/manila.2020-01-23-15.01.log.html#l-162

[Dmitry Galkin (galkindmitrii)]

Hi Goutham,

Sure, will do.

Any hints on which other objects that should apply?
 - that caused trouble for shares in our case, but might be applicable for snapshots, etc...

[OpenStack Infra (hudson-openstack)]

Fix proposed to branch: master
Review: https://review.opendev.org/721807

[Goutham Pacha Ravi (gouthamr)]

> Hi Goutham,

> Sure, will do.

> Any hints on which other objects that should apply?
> - that caused trouble for shares in our case, but might be applicable for snapshots, etc...

Thank you for proposing the fix! Sorry I didn't notice your reply here. I'll comment on the change, and assist with getting this in.

[Vida Haririan (vhariria)]

Additional comments http://eavesdrop.openstack.org/meetings/manila/2020/manila.2020-04-23-15.00.log.html

[OpenStack Infra (hudson-openstack)]

This issue was fixed in the openstack/manila 12.0.0.0rc1 release candidate.