CarlosRuiz: full meeting

2012-10-17T17:36:52Z

full meeting

New page

⇐
[[IDempiere|Table of Contents]] |
[[IDempiere/Full Meeting Minutes|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

IDempiere/FullMeeting20121017 - Revision history

CarlosRuiz: full meeting