On 4/4/2013 11:18 AM, Jacopo Cappellato wrote:
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"
Unfortunately, that is not the correct interpretation. Look up the words
"tinker" or "fidget" to get a better understanding of what I meant.
Basically I was saying you can modify/adjust the following if statement
to get the results you want.
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.
No, please don't think I'm being adversarial or dismissive. This is a
good discussion, and I appreciate your input.
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.
