IDempiere/FullMeeting20121017

⇐ Table of Contents | Full Meeting Minutes | Full Meeting 2012-10-17

CarlosRuiz: Good morning
nmicoud: Bonjour
a42niem: hi all
nmicoud: I manage to fix the bug with table dir and commit a patch in my repo
nmicoud: hi
nmicoud: if you want to integrate it
CarlosRuiz: yes Nicolas - I saw your comment - good
nmicoud: ok, i play with the new master/detail layout. That's very nice. Just wondering if it could be possible to keep old version and this one. To easier transition for users
nmicoud: I mean, maybe adding a checkbon on window to determine if we want new design or old one
CarlosRuiz: sorry, I think the old approach was completely dropped
CarlosRuiz: two reasons: 1 - it was kind of underdeveloped and very prone to errors
CarlosRuiz: 2 - keeping both approaches make those classes very hard to maintain
nmicoud: i agree, i was just thinking from users point of view ; i fear they will be lost
nmicoud: that could have been done softly with a smooth transition. Maybe firstly 'reference' window and after some times, documents ones
nmicoud: so, we will see when starting migration
CarlosRuiz: well - for those using actual master-detail can be a problem
CarlosRuiz: in my case I basically didn't use that - like two years fixing bugs there and never worked fine :-)
CarlosRuiz: nmicoud, about some contributions from you pending for review - hengsin suggested if we create a ticket status to create a peer review queue
nmicoud: yes, no problem
CarlosRuiz: just that I don't know how to do that on JIRA :-) will need to research
nmicoud: don't know ; i only use jira superficially
CarlosRuiz: got it
CarlosRuiz: https://confluence.atlassian.com/display/JIRA/Configuring+Workflow
CarlosRuiz: sounds like adding an optional step "Peer Review" before "Resolved"
nmicoud: yes, so we would have Open -> Peer Review -> Resolved ?
CarlosRuiz: yes - or back to open in case the peer review didn't pass
nmicoud: yep
nmicoud: let's do this !
Edwin_Ang: hi everyone
CarlosRuiz: Hi Edwin_Ang
nmicoud: hi
Edwin_Ang: what is the current topic?
CarlosRuiz: just saying hi to Edwin :-)
CarlosRuiz: no, seriously - we have open agenda always
CarlosRuiz: so, we were discussing with Nicolas a new step on JIRA to mark "ready for peer review" tickets
CarlosRuiz: I have pending to review fixed assets - sorry I haven't find the time to integrate it
Edwin_Ang: ah.. i thought you guys are discussing the election in "the other project" :D
CarlosRuiz: :-) trying to avoid politics on this room
Edwin_Ang: no worries.. carlos.. i know you have a lot of tickets to review
Edwin_Ang: just take your time
Edwin_Ang: however may i discuss IDEMPIERE-385 now?
CarlosRuiz: sure
Edwin_Ang: i am working based on the comment on the ticket
Edwin_Ang: created M_StorageOnHand and M_StorageReservation
Edwin_Ang: refactor all codes that call M_Storage to M_StorageOnHand instead
Edwin_Ang: moving method getQtyAvailable to M_StorageReservation
CarlosRuiz: wow - sounds like you're almost done
Edwin_Ang: not really :D
Edwin_Ang: got to update views and db functions
Edwin_Ang: and i need some explanation about this: create reserve-location on warehouse? for reporting and info product purposes
CarlosRuiz: ah sure
CarlosRuiz: I guess you're aware of the actual problems having reservations on storage table - because sometimes reservation is done on a location and shipping is done in a different
CarlosRuiz: leading to negatives and positives
Edwin_Ang: yes..
CarlosRuiz: ok - when you split the table in onhand and reservation
CarlosRuiz: the reservation table won't have location column
CarlosRuiz: so - when you try to join those back again to create the M_Storage view
CarlosRuiz: you need a "default reservation" location to show the reservations on that view
CarlosRuiz: is intended just for reporting purposes and kind of backward compatibility - so it doesn't mind if the user change the "reservation location" in the middle
CarlosRuiz: it will just show reservations elsewhere in the reports
Edwin_Ang: i thought we retain m_warehouse_id in M_StorageReservation
CarlosRuiz: yep
CarlosRuiz: but dropped locator
CarlosRuiz: and M_Storage view (that must be equal to the actual M_Storage table) has locator
CarlosRuiz: so - the idea is to add a new column, something like M_Warehouse_ID.M_ReserveLocator_ID
CarlosRuiz: and the view and the db functions show the reservations just on that locator - the other locators will show reservations as zero
Edwin_Ang: ah.. i see
Edwin_Ang: but the consequences is: the reservedqty will only be calculated as subtractor for qtyavailable in the reserve locator
Edwin_Ang: for other locator in that warehouse, qtyavail will equal qtyonhand
CarlosRuiz: yes
CarlosRuiz: kind of what happens actually - asking for available on a locator doesn't make sense - it can throw anything
CarlosRuiz: you can ask for the available on a warehouse - summing up all locators
Edwin_Ang: actually i do not like the current qtyreservation implementation
Edwin_Ang: users tend to think that reserve means that the qty is "booked"
Edwin_Ang: for example onhand is 100
Edwin_Ang: user A make a sales order with qty 80
Edwin_Ang: user B move 30 units to another locator in another warehouse
Edwin_Ang: the available qty will become -10
Edwin_Ang: i find it hard to explain it to them
Edwin_Ang: reservation does not really mean "reservation"
Edwin_Ang: :D
CarlosRuiz: yes - even worst if you move that to another warehouse
CarlosRuiz: cos you'll have warehouse negative available
hengsin: that's a bug, the current reservation does means to be a revervation :)
CarlosRuiz: Edwin_Ang, is clearer now - do you want to assign that ticket to you?
Edwin_Ang: @CarlosRuiz: lemme give it a try :)
Edwin_Ang: @hengsin: if that is a bug then every inventory transaction must consider qtyavail instead of qtyonhand
fjvr: In the scenario of the movement of quantities: Block because you are breaking reservation or Go if your are moving to fufill reservation.
Edwin_Ang: incl movement and internal use inv
Edwin_Ang: physical inv should still use qtyonhand but then there will be a possibility of minus qtyavail caused by physical inv transaction
Edwin_Ang: @fjvr: can you please elaborate more on "Go if you are moving to fulfill reservation"?
fjvr: When you do the movement you would have to make a reference to (the sales order or the reservation) and so this reservation should carry over with the movemement. In th example you present, it is NOT carrying over and thus you have the - 10 as on hand quantity.
fjvr: So, if you move because of reason independent of the reservation, the system should block you because you are breaking your agreement with a customer OR you are moving to fufill the reservation in which case the system can only know if you make reference to that which it reserved it initially.
fjvr: *to that which reserved it initially
hengsin: @fjvr, yes, exactly. at the minimum, there should be a flag to say a particular warehouse or locator shouldn't allow negative inventory
CarlosRuiz: I have seen that use case - a company that reserve on a warehouse and fulfill the shipments from a different warehouse
CarlosRuiz: I would suggest to keep it simple at first - solve the IDEMPIERE-385 - and then we can discuss how to control more strictly the breaks on reservations
Edwin_Ang: my concern is if we don't consider it now, we would need to change the table structure again later to fix the problem
CarlosRuiz: ok - the flag for negative inventory on warehouse is there - but just for onhand
CarlosRuiz: it doesn't check available
CarlosRuiz: maybe we can add another flag to disallow negative available inventory?
CarlosRuiz: just thinking draft - if that could be a solution
Edwin_Ang: what i am thinking now is elaborating on the M_Warehouse_ID.M_ReserveLocator_ID
Edwin_Ang: when completing the sales order, the system should compare orderqty with qtyavailable in M_Warehouse_ID.M_ReserveLocator_ID
hengsin: yeah, that could work - you can use a logical warehouse to control you don't actually accept order more than what you have
Edwin_Ang: so even thought if there are sufficient qty in other locator in the same warehouse, they wouldn't be consider as available
Edwin_Ang: but if going this way, then the M_StorageReservation should have M_Locator_ID
Edwin_Ang: so that any movement that use confirmation will also generate reservation on original locator
CarlosRuiz: no, sounds like we go back to our original approach
CarlosRuiz: I mean back to the original approach with the known problems
Edwin_Ang: well we actually improved the original approach with M_Warehouse_ID.M_ReserveLocator_ID
CarlosRuiz: hengsin, "that could work" refers to Edwin suggestion or "disallow negative available" ?
Edwin_Ang: and i think the flaw in the original design is mixing m_storage with reservation
CarlosRuiz: still reservations by locator doesn't sound good
hengsin: Carlos, I means adding the disallow negative available flag to warehouse
CarlosRuiz: yes - that would mean that you cannot move reserved items out of that warehouse
CarlosRuiz: or "internal use" them
CarlosRuiz: maybe we could complement that with a "move reservation to another warehouse" process
Edwin_Ang: in movement, the user will directly choose the locator from
Edwin_Ang: it is wrong if the qtyreserved is then allocated to M_Warehouse_ID.M_ReserveLocator_ID
hengsin: for better or worst - http://wiki.idempiere.org/wiki/File:ToolbarProcessButton.png :)
CarlosRuiz: sorry Edwin_Ang - I'm not following that line
CarlosRuiz: in movement - user is moving onhand qtys
Edwin_Ang: but there is that movement with confirmation
Edwin_Ang: although it is currently buggy
Edwin_Ang: but i think movement with confirmation should reserve the stock
CarlosRuiz: I saw that recently
CarlosRuiz: reserving on movement prepare - and releasing the reserve on confirmation
CarlosRuiz: well - it was not reservation - but more like allocation
CarlosRuiz: and it was kind of hard to control all the other possible events - i.e reversing - not confirming
CarlosRuiz: I think actual approach if you want to make such movements is to move to a transit warehouse
Edwin_Ang: well.. moving to transit warehouse is another case
Edwin_Ang: i mean we have the move confirmation inherited from Compiere
Edwin_Ang: it serves a certain inventory movement scenario
Edwin_Ang: it is become more and more complex :D
Edwin_Ang: i will try to work on the initial work first
CarlosRuiz: yes
Edwin_Ang: then elaborate later
CarlosRuiz: that's what happened on every forum thread where we discussed this :-)
Edwin_Ang: :D
CarlosRuiz: great - you can contact me on skype - or here - or on JIRA - in case new doubts arise
Edwin_Ang: one more thing that i would like to ask you
CarlosRuiz: sure
Edwin_Ang: in one of my previous installation
Edwin_Ang: i noticed many m_storage records with negative qty
Edwin_Ang: if you sum up by m_product_id and m_locator_id, the qty will be +
Edwin_Ang: however the negative qty records surely make me feel uneasy
Edwin_Ang: do you have any idea what cause that?
CarlosRuiz: do you mean on onhand?
hengsin: qtyonhand ?
Edwin_Ang: yes
Edwin_Ang: qtyonhand
CarlosRuiz: I think it can happen
CarlosRuiz: if you don't have the check "disallow negative inventory"
CarlosRuiz: which is recent and not present on adempiere "official"
Edwin_Ang: you haven't committed that feature yet at the time
CarlosRuiz: it must be so common that Jorg Janke made a process to fix that :-)
Edwin_Ang: oh really? which one? :D :D
Edwin_Ang: i don't know there's such process
CarlosRuiz: http://adempiere.com/ManPageP_StorageCleanup
hengsin: "if you sum up by m_product_id and m_locator_id, the qty will be +" - it is + but is it correct ?
CarlosRuiz: but I don't like it - it seems kind of "random"
Edwin_Ang: @hengsin: i can't be sure if it is 100% correct
Edwin_Ang: cos at that time we heavily customized the inventory codes
Edwin_Ang: my last implementation incl inventory
Edwin_Ang: been avoiding inventory ever since
Edwin_Ang: :D
Edwin_Ang: until now
Edwin_Ang: hahaha
CarlosRuiz: ok guys - I need to move out - thanks for attending the meeting
hengsin: one issue that have been there since day one is the lack of concurrency control
Edwin_Ang: ok.. bye Carlos
Edwin_Ang: thanks for the discussion :)
nmicoud: bye
hengsin: in many place, it is reading mstorage record without "for update" and then use it for calculation and further update, i.e you could be working with stale data and end up sending the wrong update to db.
Edwin_Ang: @hengsin: any idea how we can improve that area?
hengsin: for scenario where you need to perform update with figures from m_storage, we should read it from m_storage with the for update clause
hengsin: that should ensure we get the "current data" since the lock will prevent other transaction to mess with it.
Edwin_Ang: can you give an example of implementation in the idempiere code?
hengsin: for example, in MInOut.completeIt, in checkMaterialPolicy, it call MStorage.getWarehouse to retrieve storage records. it then use that for delivery calculate and later use MStorage.add to modify the m_storage.qtyonhand
hengsin: in a busy environment, some other transactions might have retrieve/modify the same storage record during that interval.
hengsin: i means the interval between mstorage.getwarehouse and mstore.add
hengsin: typo - mstorage.add