Salut,
apology for the delay.
Bongjae Chang wrote:
> 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.
OK That's great as the Mapper is a little over designed (but quite
performant)
>
> But, I think that the functions are enough for Grizzly RCM's use case. :)
Much better than what we had before :-)
>
> 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 "*".
ah great idea.
>
> 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.
Looks good!
Thanks
-- Jeanfrancois
>
> 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
>>>
>>>
>>>
>>>
>>>
>>> ------------------------------------------------------------------------
>>>
>>> ---------------------------------------------------------------------
>>> To unsubscribe, e-mail: dev-unsubscribe_at_grizzly.dev.java.net
>>> For additional commands, e-mail: dev-help_at_grizzly.dev.java.net