We’re going to provide some improvements for the Stock Management performance. One of the issues is that the Stock Management service currently doesn’t store any information about current stock on hand. Instead, the current stock on hand needs to be calculated from all of the transactions that happened on a stock card. In order to tackle that, we would like to introduce the fact table, that would store the stock on hand information as of any given date. That should make it faster to determine both the current stock and stock as of any given date. The design of the database can be found here: https://www.db-fiddle.com/f/qRbv6dCFnBbLXmP63vYjRv/1.
The idea is to store one entry for each day when the event has occurred, but:
- if there are more events in one day, the stockonhand value would get updated for an existing record in the database;
- if there are no entries for a given day, we can consider the latest one up-to-date;
As you can see in the fiddle, we’re planning to add an index only on the stockcardid column, which will speed up the queries a lot. According to the test we’ve run, there no need to add additional indexes as it won’t improve the performance in this case.
Please let us know what do you think about this design?
Looks quite good for me and I have few questions:
Do we need an additional table to store that information? From what I see the StockCardLineItem class has the stockOnHand field - which currently is set as transient so the value will be ignored by Hibernate - could we use this field instead of introducing a new table?
When we would like to fill the table? Probably when an event is sent to the service.
How we would like to fill the table? The easiest way is to simply take all line items to calculate stock on hand and save values to a database but this will probably have poor performance when there will be a lot of line items. Do we have other options for a huge amount of data?
How we fill this table for legacy data? Is it possible to do this by single migration in the stock management service?
Thanks for the reply @llewczynski!
According to your questions:
- To calculate the value of the stock on hand for one StockCard, we’re using all related StockCardLineItems. If we would store the value of the stock on hand for each StockCardLineItem, we would still have to fetch them all and sum, which would not improve the performance as much as introducing a new table.
- We would like to fill the table when the event occurs.
- I think the simplest solution could be applied there. The huge amount of data would be calculated only once at the beginning, as it will be updated regularly when a new event occurs, but we’re still investigating the best solution for this. The details about the logic will be introduced soon as a separate post.
- Single migrations seem to be the best option - we would have to iterate all stock events that already occurred.
Please let me know if you’ll have any other concerns.
Thanks for bringing this forward to the community. I had a couple of questions/comments.
First, we should pluralize the table calculated_stock_on_hand, to follow our style guide
The other thing I am curious about is the date for the calculated SOH table. Since it is just a date with no time portion and no timezone, how will it be managed in relation to stock events? Stock events are recorded at a particular instant (date, time and timezone), while this date is just at the day level. Perhaps we can assume the translation from instant to this date is for UTC, but then some questions arise. Does an end user ever interact with these dates directly? Because a user might specify a date and be in a timezone that is not in UTC, and their SOH for 2019-05-30 might be different from UTC SOH for 2019-05-30.
I am wondering if this “as of date” just being a date for calculated SOH is the right approach, since our stock events/transactions represent instants in time.
Good point @Chongsun_Ahn!
I thought the word stock is used for plural form, but it seems like stocks is accepted as well. I guess calculated_stocks_on_hand will be indeed more intuitive.
Actually, the date is not always stored in the UTC - it’s configurable and depends on where the system is going to be used, but it will be constant once established. When users are specifying the dates, they have to choose them corresponding to what’s set on the server, so using additional table shouldn’t change the values for any users.
I hope it will clarify your concerns. Please let me know if you’ll have any questions.
Thanks for continuing this discussion. This is always kind of a difficult topic, and we’ve had discussions about it before.
We generally store dates that have timezones to be stored in UTC, for simplicity. That way, the system, when it is storing, retrieving and using datetimes, there is no confusion; we know the datetime represents an instant in UTC. The only time we begin to think of a datetime in another timezone is when it is shown to an end user; like in the UI, or in a PDF/report.
The “system timezone” that we have in OpenLMIS is sort of a compromise. Ideally, we were thinking that each user could specify a user-based timezone in the user profile, and all datetimes would be shown with that timezone, but it was a lower level of effort to just have one timezone for the system and have all end user components show datetimes using that. That is why we see the User model have a timezone property.
The system timezone idea has limitations. What if the system is “located” where there is more than one timezone? We wouldn’t want to the user to be thinking of a different timezone in order to specify the right date.
Anyway, my input is that I think it’s not an issue that we are storing asOfDate as just a date; it doesn’t seem like this asOfDate is specified by end users for now. However, we may run into issues into the future when trying to convert this date to an instant, when comparing to other instants. Also, I’m not sure we should think of it in the “system timezone”.
@Sebastian_Brudzinski, I think you said in the Tech Committee call that we’re running into an issue with timezones and processing period dates?
Thanks @ohinc, this is looking good.
On the topic of date-times, dates, SoH and stock events it might help to think:
- Stock is held for a Location (Facility), which is at a mostly-static timezone (okay this is a huge lie, time zones change all the time).
- A stock event, and therefore resulting line items, are processed (aka entered) at an instance in time (a point on a time line). This is the stockevent.processeddate (timestamp).
- A stock event’s line items are described as occurring not at an instance in time, but rather on a semantic day. e.g. an orderable might be marked as being lost on June 2, 2018. It’s not recorded as an instant in time at all. If you were to interpret it as an instance in time, you could just as validly say it happened at 3pm UTC as 4pm UTC - it’s an interpretation that’s applied to the semantic day and not a fact.
- The Stock on Hand, as described here, is for a semantic day: June 2, 2018.
We’ve discussed the multiple ways the question “what’s the soh” can be asked. Here we’re only looking to solve for the most frequent question that’s asked: what’s my SoH as of June 2, 2018 (okay any day). The task of bucketing the change to SoH from any Line Item into the SoH table always stays at the level of a day, and never dips into timezones / instances in time.
Side note: there is a point where we need to consider instances in time, and that’s in answering questions involving the stockevent.processeddate (timestamp). One such question is the: what’s the SoH as reported on June 1, 2018. If we get this far we’ll need to take a step back into the points @Chongsun_Ahn raised about timezones, system time and Location.
Thanks for answers @Chongsun_Ahn and @joshzamor!
Like @joshzamor said we’re currently considering SoH at the level of a day, so we’ll proceed with creating the discussed table. I guess we’ll have to discuss the topic of timezones once again when it’ll be needed.
Yes, the processing period start and end dates are also currently stored as local dates, but at some point a time zone is injected into them, what makes them display incorrect dates on UI (shifted one day).
The ticket is https://openlmis.atlassian.net/browse/OLMIS-6301