On Apr 4, 2013, at 11:27 AM, Adrian Crum <[email protected]> wrote:
> Isn't that what the next block of code does? If you want to have a message > that appears based on a set of conditions, then you can fuss around with the > if statement that follows the timing message. Yeah, of course everyone can easily add log messages to get the information required (at the moment that block doesn't show time related information); in fact it is not a big deal for me, this is not the kind of commit that concerns me. By the way, I had to check in the dictionary the meaning of "to fuss" and I found: "show unnecessary or excessive concern about something" In light of this, if you think that I am wasting your time with unrealistic use cases then you can ignore me :-) I don't care much about this commit and you don't have to try to convince me; as I said I am not asking you to revert your commit, I simply added some information to the context because I felt it was missing from your arguments. Jacopo > > -Adrian > > On 4/4/2013 9:58 AM, Jacopo Cappellato wrote: >> This would be resolved by changing the message into something that clarifies >> that the service took longer than a given threshold X; in this case the >> message would be clear and could be useful to the new user (even if of >> course a message based on a threshold is kind of arbitrary and doesn't tell >> you enough about performance) and may be ignored or not based on the nature >> of the service. >> >> Jacopo >> >> On Apr 4, 2013, at 10:17 AM, Adrian Crum >> <[email protected]> wrote: >> >>> Having a condition like that is unfair to end users. >>> >>> Let's say I'm a new OFBiz user. I just wrote my first service and I try it >>> out. I run the service, and check the logs. Weird - the service engine says >>> my service didn't run. I check my data - it appears the service ran. I put >>> some logging messages inside my service and run it again. Cool - my log >>> messages appear, but the service engine still says my service didn't run. >>> So I dig deep into the service engine source code and discover that someone >>> decided my service wasn't important enough to log. That would be very >>> frustrating. >>> >>> If you don't want to see timing messages, then turn them off. >>> >>> Simple. >>> >>> -Adrian >>> >>> On 4/4/2013 8:57 AM, Jacopo Cappellato wrote: >>>> I agree it is arbitrary and sub-optimal solution but maybe I still liked >>>> it more before the last commit: the risk now is that we have too much >>>> information and no one will ever look at it. >>>> The obvious workaround could be to set the threshold in a property file >>>> but there are probably better solutions that will require more work and >>>> design: for example it would be nice to enable usage statistical >>>> collections on services and entities (e.g. how many times a service was >>>> executed, the average time of execution, max time etc...) and then show >>>> this data in a screen; we could store this data in an entity or a file or, >>>> maybe better, keep it in memory (in a cache or similar) and let an admin >>>> enable/disable/reset this while the system is running (similarly to cache >>>> entries). >>>> >>>> Just my 2 cents, >>>> >>>> Jacopo >>>> >>>> >>>> On Apr 4, 2013, at 9:22 AM, Adrian Crum >>>> <[email protected]> wrote: >>>> >>>>> Why not 20 or 30 or 40? >>>>> >>>>> That's the problem with arbitrary values - they don't mean anything. >>>>> >>>>> From my perspective, if anyone has timing enabled, then they want to see >>>>> what's going on in the system. >>>>> >>>>> Feel free to change it. >>>>> >>>>> -Adrian >>>>> >>>>> On 4/3/2013 9:22 AM, Jacques Le Roux wrote: >>>>>> Hi Adrian, All, >>>>>> >>>>>> Should we really show the timing for all services? >>>>>> Maybe increasing from 50 to 75 or even 100 for the 1st case would be >>>>>> enough? >>>>>> >>>>>> Jacques >>>>>> >>>>>> From: <[email protected]> >>>>>>> Author: adrianc >>>>>>> Date: Wed Apr 3 07:57:24 2013 >>>>>>> New Revision: 1463863 >>>>>>> >>>>>>> URL: http://svn.apache.org/r1463863 >>>>>>> Log: >>>>>>> Log message cleanup in ServiceDispatcher.java. Removed confusing text >>>>>>> about services taking too long. >>>>>>> >>>>>>> Modified: >>>>>>> >>>>>>> ofbiz/trunk/framework/service/src/org/ofbiz/service/ServiceDispatcher.java >>>>>>> >>>>>>> Modified: >>>>>>> ofbiz/trunk/framework/service/src/org/ofbiz/service/ServiceDispatcher.java >>>>>>> URL: >>>>>>> http://svn.apache.org/viewvc/ofbiz/trunk/framework/service/src/org/ofbiz/service/ServiceDispatcher.java?rev=1463863&r1=1463862&r2=1463863&view=diff >>>>>>> ============================================================================== >>>>>>> --- >>>>>>> ofbiz/trunk/framework/service/src/org/ofbiz/service/ServiceDispatcher.java >>>>>>> (original) >>>>>>> +++ >>>>>>> ofbiz/trunk/framework/service/src/org/ofbiz/service/ServiceDispatcher.java >>>>>>> Wed Apr 3 07:57:24 2013 >>>>>>> @@ -571,10 +571,8 @@ public class ServiceDispatcher { >>>>>>> rs.setEndStamp(); >>>>>>> >>>>>>> long timeToRun = System.currentTimeMillis() - serviceStartTime; >>>>>>> - if (Debug.timingOn() && timeToRun > 50) { >>>>>>> - Debug.logTiming("Slow sync service execution detected: >>>>>>> service [" + localName + "/" + modelService.name + "] finished in [" + >>>>>>> timeToRun + "] milliseconds", module); >>>>>>> - } else if (Debug.infoOn() && timeToRun > 200) { >>>>>>> - Debug.logInfo("Very slow sync service execution detected: >>>>>>> service [" + localName + "/" + modelService.name + "] finished in [" + >>>>>>> timeToRun + "] milliseconds", module); >>>>>>> + if (Debug.timingOn()) { >>>>>>> + Debug.logTiming("Sync service [" + localName + "/" + >>>>>>> modelService.name + "] finished in [" + timeToRun + "] milliseconds", >>>>>>> module); >>>>>>> } >>>>>>> if ((Debug.verboseOn() || modelService.debug) && timeToRun > 50 >>>>>>> && !modelService.hideResultInLog) { >>>>>>> // Sanity check - some service results can be multiple MB >>>>>>> in size. Limit message size to 10K. >>>>>>> >>>>>>> >
