I'll clean up the headers and java doc, that's no problem.  

On adding a configurable throttle strategy to the AsyncAppender, basically you 
already can by setting the buffer size, right?  Do you mean adding the ability 
to configure blocking and the buffer size based on log level?  Let me know if 
I'm not understanding you correctly.

I like the idea of reporting on discarded messages and making the throttle 
configurable based on level, great feedback.  Collapsing the two classes into 
one seems fine also.  

Funny you should mention renaming it, I actually submitted this code well over 
a year ago under the name TokenBucketFilter.  The feedback I got at that time 
was that I should rename it to something like BurstFilter to make it 
implementation agnostic.  It took me a long time to get around to it, but what 
you're looking at is my response to that initial feedback.  If the team would 
prefer I can name it back to TokenBucketFilter but I did kind of agree with the 
initial feedback about leaving the algorithm out of the name and just using a 
name that describes what the class is used for.  Just let me know what the 
consensus is.

I agree that the return value of getToken is confusing and will update it.

--- On Thu, 10/9/08, [EMAIL PROTECTED] <[EMAIL PROTECTED]> wrote:
From: [EMAIL PROTECTED] <[EMAIL PROTECTED]>
Subject: DO NOT REPLY [Bug 45753] Code contribution
To: [email protected]
Date: Thursday, October 9, 2008, 7:09 PM

https://issues.apache.org/bugzilla/show_bug.cgi?id=45753





--- Comment #1 from Curt Arnold <[EMAIL PROTECTED]>  2008-10-09 17:09:14
PST ---
First, some picky things:

The license headers in the source code should conform to
http://www.apache.org/legal/src-headers.html.  If you could fix the headers and
resubmit.

@author tags are no frowned upon in ASF since it may result in territoriality. 
We haven't removed them from log4j, but we are avoiding adding new ones.

Many methods are missing javadoc comments.

---

I guess the current AsyncAppender within blocking=false would be classified as
a leaky-bucket where the leak rate is the sustainable rate of underlying
appender.  It does have the advantage of summarizing the discard messages and
reporting the highest error level that was not reported.  What are your
thoughts on adding a configurable strategy for throttling on AsyncAppender?

The burst filter appears to discard events with no potential for summarizing or
being informed of event loss.

It might be desirable to make this level aware, so you would apply this to
DEBUG messages but INFO and higher might pass through unaffected.

I'd likely collapse BurstFilter and TokenBucket and rename it
TokenBucketFilter.

TokenBucket.getToken()'s has an unexpected return value.  You'd expect
that it
would return true if there were available tokens, but it returns true if empty.


-- 
Configure bugmail: https://issues.apache.org/bugzilla/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.

---------------------------------------------------------------------
To unsubscribe, e-mail: [EMAIL PROTECTED]
For additional commands, e-mail: [EMAIL PROTECTED]




      

Reply via email to