Hi,
I attached the proposed patch again about (1), (2) and (3) which I said.
I wrote the getAppropriateThread() and getAppropriateThreadRatio() functions simply and I didn't use the Mapper.
But, I think that the functions are enough for Grizzly RCM's use case. :)
Here are use cases of the method.
Assuming that threadPools(or privilegedTokens) have 3 key entries as (a)"/examples", (b)"/examples/index.html"and (c)"/foo/bar",
If a given param is "/examples/index.jsp", returns (a)
If a given param is "/examples/index.html", returns (b)
If a given param is "/foo/bar", returns (c)
If a given param is "/foo/bar/index.html", returns (c)
If a given param is "/foo/bar2", returns null
Also, an user can set the policy metric like this.
For rule (a), "/examples" or "/examples/*" or "/examples/*/".
If an user set the rule to be "/*" or "/" or "*", the rule converts it into "*".
Would you please review this?
---
Index: com/sun/grizzly/rcm/ResourceAllocationFilter.java
===================================================================
--- com/sun/grizzly/rcm/ResourceAllocationFilter.java (revision 3411)
+++ com/sun/grizzly/rcm/ResourceAllocationFilter.java (working copy)
@@ -92,10 +92,11 @@
protected final static String RULE_TOKENS =
"com.sun.grizzly.rcm.policyMetric";
- private final static String DELAY_VALUE = "com.sun.grizzly.rcm.delay";
+ private final static String DELAY_VALUE = "com.sun.grizzly.rcm.delay"; // milli-seconds
protected final static String QUERY_STRING="?";
protected final static String PATH_STRING="/";
+ protected final static String ASTERISK_STRING="*";
public final static String BYTEBUFFER_INPUTSTREAM = "bbInputStream";
@@ -139,7 +140,7 @@
/**
* The time this thread will sleep when a rule is delayed.
*/
- private static long delayValue = 5 * 1000;
+ private static long delayValue = 5 * 1000; // milli-seconds
/**
@@ -163,9 +164,21 @@
while (privElement.hasMoreElements()){
tokens = privElement.nextToken();
int index = tokens.indexOf("|");
+ String tokenKey = tokens.substring(0, index);
+ // Remove last slash
+ boolean slash = tokenKey.endsWith(PATH_STRING);
+ if (slash){
+ tokenKey = tokenKey.substring(0,tokenKey.length() -1);
+ }
+ // Remove last asterisk. i.g. "/examples/*"
+ boolean asterisk = tokenKey.endsWith(PATH_STRING + ASTERISK_STRING);
+ if( asterisk )
+ tokenKey = tokenKey.substring(0,tokenKey.length() -2);
+ // such as "/" or "/*"
+ if(tokenKey.length() == 0)
+ tokenKey = ASTERISK_STRING; // default key
tokenValue = Double.valueOf(tokens.substring(index+1));
- privilegedTokens.put
- (tokens.substring(0, index),tokenValue);
+ privilegedTokens.put(tokenKey,tokenValue);
countRatio += tokenValue;
}
}
@@ -187,9 +200,21 @@
allocationPolicy = RESERVE;
}
}
+
+ String delayValueString = System.getProperty( DELAY_VALUE );
+ if( delayValueString != null ) {
+ long delayValueLong = -1;
+ try {
+ delayValueLong = Long.parseLong( delayValueString );
+ } catch( NumberFormatException e ) {
+ Controller.logger().info( "Invalid delay value:" + delayValueString );
+ }
+ if( delayValueLong > 0 )
+ delayValue = delayValueLong;
+ }
}
-
-
+
+
public ResourceAllocationFilter() {
}
@@ -235,7 +260,7 @@
String token = getContextRoot(byteBuffer);
int delayCount = 0;
- while (leftRatio == 0 && privilegedTokens.get(token) == null) {
+ while (leftRatio == 0 && getAppropriateThreadRatio(token) == null) {
if ( allocationPolicy.equals(RESERVE) ){
delay(ctx);
delayCount++;
@@ -258,7 +283,7 @@
}
// Lazy instanciation
- ExecutorService threadPool = threadPools.get(token);
+ ExecutorService threadPool = getAppropriateThreadPool(token);
if (threadPool == null){
threadPool = filterRequest(token, ctx.getThreadPool());
threadPools.put(token,threadPool);
@@ -285,8 +310,8 @@
ctx.setKeyRegistrationState(Context.KeyRegistrationState.NONE);
try{
NIOContext copyContext = (NIOContext) Cloner.clone(ctx);
+ copyContext.setThreadPool(threadPool);
copyContext.execute(copyContext.getProtocolChainContextTask());
- copyContext.setThreadPool(threadPool);
} catch (Throwable t){
ctx.setAttribute(Context.THROWABLE,t);
ctx.setKeyRegistrationState(
@@ -324,15 +349,18 @@
// Set the number of dispatcher thread to 1.
threadPool.setCorePoolSize(1);
- Double threadRatio = privilegedTokens.get(token);
+ Double threadRatio = getAppropriateThreadRatio(token);
boolean defaultThreadPool = false;
if (threadRatio == null) {
- es = threadPools.get("*");
+ es = threadPools.get(ASTERISK_STRING);
if (es != null){
return es;
}
-
- threadRatio = (leftRatio == 0 ? 0.5 : leftRatio);
+ // before setting the thread ratio to be leftRatio, checks the "*" ratio
+ threadRatio = getAppropriateThreadRatio(ASTERISK_STRING);
+ if(threadRatio == null) {
+ threadRatio = (leftRatio == 0 ? 0.5 : leftRatio);
+ }
defaultThreadPool = true;
}
@@ -341,7 +369,7 @@
es = newThreadPool(privilegedCount, p);
if (defaultThreadPool){
- threadPools.put("*", es);
+ threadPools.put(ASTERISK_STRING, es);
}
}
@@ -393,7 +421,7 @@
byteBuffer.get(chars);
String token = new String(chars);
-
+
// Remove query string.
int index = token.indexOf(QUERY_STRING);
if ( index != -1){
@@ -466,7 +494,7 @@
case 0: // Search for first ' '
if (c == 0x20){
state = 1;
- start = byteBuffer.position() + 1;
+ start = byteBuffer.position();
}
break;
case 1: // Search for next ' '
@@ -507,5 +535,52 @@
};
}
return protocolParser;
- }
+ }
+
+ /**
+ * Returns an appropriate thread pool with corresponding to a given token
+ *
+ * ex) Assuming that <code>threadPools</code> have 3 key entries as (1)"/examples", (2)"/examples/index.html"and (3)"/foo/bar",
+ * if a given param is "/examples/index.jsp", returns (1).
+ * if a given param is "/examples/index.html", returns (2)
+ * if a given param is "/foo/bar", returns (3)
+ * if a given param is "/foo/bar/index.html", returns (3)
+ * if a given param is "/foo/bar2", returns <code>null</code>
+ *
+ * @param token a request uri. i.g. /examples/index.html or /examples
+ * @return if found, the appropriate thread pool is returned. if not found, <code>null</code> is returned.
+ */
+ private static ExecutorService getAppropriateThreadPool( String token ) {
+ if( token == null || token.length() == 0 )
+ return null;
+ ExecutorService threadPool = null;
+ threadPool = threadPools.get( token );
+ if( threadPool == null ) {
+ int index = token.lastIndexOf( PATH_STRING );
+ if( index > 0 )
+ return getAppropriateThreadPool( token.substring( 0, index ) );
+ }
+ return threadPool;
+ }
+
+ /**
+ * Returns an appropriate thread ratio with corresponding to a given token
+ *
+ * ex) similar to {_at_link #getAppropriateThreadPool(String)}
+ *
+ * @param token a request uri. i.g. /examples/index.html or /examples
+ * @return if found, the appropriate thread ratio is returned. if not found, <code>null</code> is returned.
+ */
+ private static Double getAppropriateThreadRatio( String token ) {
+ if( token == null || token.length() == 0 )
+ return null;
+ Double threadRatio = null;
+ threadRatio = privilegedTokens.get( token );
+ if( threadRatio == null ) {
+ int index = token.lastIndexOf( PATH_STRING );
+ if( index > 0 )
+ return getAppropriateThreadRatio( token.substring( 0, index ) );
+ }
+ return threadRatio;
+ }
}
---
And please advice me again.
Thanks!
--
Bongjae Chang
----- Original Message -----
From: "Bongjae Chang" <carryel_at_korea.com>
To: <dev_at_grizzly.dev.java.net>
Sent: Saturday, July 18, 2009 11:56 AM
Subject: Re: About Grizzly's RCM
> Hi Jeanfrancois,
>
> Jeanfrancois wrote:
>> This is strange as it looks for ' ', which is why start needs to be
>> bumped by 1 so we start at /. Not sure we should remove the +1 unless
>> I'm missing something.
>
> About (1), I would like to explain more.
>
> Assuming that a user request "GET /a.html HTTP/1.1",
> ---
> Byte Index
> G 0
> E 1
> T 2
> (blank) 3
> / 4
> a 5
> . 6
> h 7
> t 8
> m 9
> l 10
> (blank) 11
> ...
> ---
> while(byteBuffer.hasRemaining()) {
> c = byteBuffer.get();
> switch(state) {
> case 0:
> if (c==0x20){
> state = 1;
> //----------------------------------(a) At this point, byteBuffer.position() is not 3 but 4
> start = byteBuffer.position() + 1;
> }
> break;
> case 1:
> if (c==0x20){
> //----------------------------------(b) At this point, byteBuffer.position() is not 11 but 12
> end = byteBuffer.position() - 1;
> byteBuffer.position(start);
> byteBuffer.limit(end);
> ...
> }
> }
> ...
> }
> ---
> Above (a), after byteBuffer.get(), byteBuffer's position is already increased. So we need not to increase 1.
>
> And also see the following:
> ---
> protected String getContextRoot(ByteBuffer byteBuffer) {
> byte[] chars = new byte[byteBuffer.limit() - byteBuffer.position()]; ------ (c) byteBuffer.position() is equal to 5 and byteBuffer.limit() is equal to 11
> byteBuffer.get(chars);
> ...
> }
> ---
> Above (c), "byteBuffer.limit() - byteBuffer.position()" is equal to 6 and byteBuffer.position() is equal to 5.
>
> So we will not obtain "/a.html" but "a.html".
>
> Jeanfrancois wrote:
>> Agree. But if we go that route we will need to improve the mapping of
>> the request. So I think we will need to inject the Mapper or (I can see
>> Hubert and Alexey smiling) write a new one to replace the over
>> complicated Mapper.
>
> Right. But I am afraid of the performance decline because of additional comparison logic. But I think Mapper is still needed for practical manner.
>
> Jeanfrancois wrote:
>> No you are totally right. I'm just curious about (1). For 3, do you want
>> to go ahead and write/use the Mapper?
>
> Sure. First of all, I would like to look into the Mapper. :)
>
> After that, I will reply again.
>
> Thanks!
> --
> Bongjae Chang
>
>
> ----- Original Message -----
> From: "Jeanfrancois Arcand" <Jeanfrancois.Arcand_at_Sun.COM>
> To: <dev_at_grizzly.dev.java.net>
> Sent: Friday, July 17, 2009 11:29 PM
> Subject: Re: About Grizzly's RCM
>
>
>> Salut,
>>
>> Bongjae Chang wrote:
>>> Hi,
>>>
>>> Today, I tried to review and test the ResourceAllocationFilter.
>>
>> Great!
>>
>>>
>>> But, I have some questions about ResourceAllocationFilter.
>>>
>>>
>>> 1. ResourceAllocationFilter#getContextRoot()'s return value
>>>
>>> If a client sends "GET /examples/index.html HTTP/1.1" request to the
>>> grizzly server, getContextRoot() returns "examples/index.html".
>>>
>>> But, I think that ResourceAllocationFilter#getContextRoot() should
>>> return "/examples/index.html" because a user's policyMetric is like
>>> "/myApplication|0.9".
>>
>> Agree
>>
>>
>>>
>>> Is it right?
>>>
>>> If my thoughts are correct, here is the proposed patch.
>>>
>>> Index: com/sun/grizzly/rcm/ResourceAllocationFilter.java
>>> ===================================================================
>>> --- com/sun/grizzly/rcm/ResourceAllocationFilter.java (revision 3411)
>>> +++ com/sun/grizzly/rcm/ResourceAllocationFilter.java (working copy)
>>> @@ -466,7 +466,7 @@
>>> case 0: // Search for first ' '
>>> if (c == 0x20){
>>> state = 1;
>>> - start = byteBuffer.position() + 1;
>>> + start = byteBuffer.position();
>>> }
>>> break;
>>> case 1: // Search for next ' '
>>>
>>
>> This is strange as it looks for ' ', which is why start needs to be
>> bumped by 1 so we start at /. Not sure we should remove the +1 unless
>> I'm missing something.
>>
>>>
>>>
>>> 2. ResourceAllocationFilter#invokeProtocolParser()'s strange code?
>>>
>>> Here is ResourceAllocationFilter#invokeProtocolParser()
>>>
>>> ---
>>> public boolean invokeProtocolParser(...) {
>>> ...
>>> try{
>>> NIOContext copyContext = (NIOContext) Cloner.clone(ctx);
>>> copyContext.execute(copyContext.getProtocolChainContextTask());
>>> *copyContext.setThreadPool(threadPool);*
>>> } catch(...){
>>> ...
>>> }
>>> ...
>>> }
>>> ---
>>>
>>> I couldn't understand the above code because the decided thread
>>> pool(threadPool) was set after NIOContext.execute().
>>>
>>> Then, NIOContext.execute() doesn't use the decided thread pool but
>>> original thread pool.
>>>
>>> Is it right?
>>
>> Wow this is seriously broken. You are right.
>>
>>
>>>
>>>
>>> 3. Implications of the policyMetric
>>>
>>> When I read the JavaDoc and
>>> http://weblogs.java.net/blog/jfarcand/archive/2006/04/resource_consum.html,
>>> it seems that an user can set the policyMetric like the following.
>>>
>>> ---
>>> -Dcom.sun.grizzly.rcm.policyMetric="/examples|0.9"
>>> ---
>>>
>>> Then, does it mean that the policy is applied to all sub requests of
>>> "examples" context? Or does it mean that the policy is applied to only
>>> "/examples" request?
>>>
>>> I think that it is better that the policy should be applied to
>>> "/examples", "/examples/index.html", "/examples/something" and etc...
>>>
>>> What do you think?
>>
>> Agree. But if we go that route we will need to improve the mapping of
>> the request. So I think we will need to inject the Mapper or (I can see
>> Hubert and Alexey smiling) write a new one to replace the over
>> complicated Mapper.
>>
>>>
>>>
>>> If I am missing some points, please advice me.
>>
>> No you are totally right. I'm just curious about (1). For 3, do you want
>> to go ahead and write/use the Mapper?
>>
>> A+
>>
>> - Jeanfrancois
>>
>>
>>>
>>> Thanks.
>>>
>>> --
>>> Bongjae Chang
>>
>> ---------------------------------------------------------------------
>> To unsubscribe, e-mail: dev-unsubscribe_at_grizzly.dev.java.net
>> For additional commands, e-mail: dev-help_at_grizzly.dev.java.net
>>
>>
>>
>>