[jboss-jira] [JBoss JIRA] Commented: (JGRP-736) GMS.ViewHandler.run() view bundling not correctly checking if requests can be processed together...

[Bela Ban (JIRA)]

    [ http://jira.jboss.com/jira/browse/JGRP-736?page=comments#action_12409994 ] 
            
Bela Ban commented on JGRP-736:
-------------------------------

OK, got it. I modified and committed the code to 2.6, can you take a look and see whether you like them ?

> GMS.ViewHandler.run() view bundling not correctly checking if requests can be processed together...
> ---------------------------------------------------------------------------------------------------
>
>                 Key: JGRP-736
>                 URL: http://jira.jboss.com/jira/browse/JGRP-736
>             Project: JGroups
>          Issue Type: Bug
>            Reporter: Michael Newcomb
>         Assigned To: Bela Ban
>             Fix For: 2.6.3, 2.7
>
>
> Consider:
>  1356                     do {
>  1357                         Request firstRequest=(Request)q.remove(INTERVAL); // throws a TimeoutException if it runs into timeout
>  1358                         requests.add(firstRequest);
>  1359                         if(q.size() > 0) {
>  1360                             Request nextReq=(Request)q.peek();
>  1361                             keepGoing=view_bundling && firstRequest.canBeProcessedTogether(nextReq);
>  1362                         }
>  1363                         else {
>  1364                             stop=System.currentTimeMillis();
>  1365                             wait_time=max_bundling_time - (stop-start);
>  1366                             if(wait_time > 0)
>  1367                                 Util.sleep(wait_time);
>  1368                             keepGoing=q.size() > 0;
>  1369                         }
>  1370                     }
>  1371                     while(keepGoing);
> The faulty logic occurs on line 1368. At this point there was only 1 object on the queue when it was removed, so it waited max_bundling_time. After the wait_time expires, it checks to see if q.size() > 0, if so, it keeps going.
> This actually creates 2 problems.
> 1. if view_bundling is false, it will still bundle the next request because it goes right back up and removes it and adds it to requests
> 2. if the request in the queue can't be processed with the request that just came in, it will still be added to requests
> So, the simple solution (for released versions) is to use the checks from 1360-1361:
>  1367.5                          Request nextReq=(Request)q.peek();
>  1368                             keepGoing=view_bundling && ((Request) q.getLast()).canBeProcessedTogether(nextReq);
> I think the ViewHandler could be rewritten for later releases so that as requests come in, it notifies the ViewHandler thread and he can check all queued requests to see if they could be processed together. Therefore, if you had view_bundling enabled, but a request comes in that can't be processed with existing requests, it will start processing immediately instead of waiting the rest of max_bundling_time (not long, but could be if configured that way).
> Also, if view_bundling is disabled, it should not sleep at all.

-- 
This message is automatically generated by JIRA.
-
If you think it was sent incorrectly contact one of the administrators: http://jira.jboss.com/jira/secure/Administrators.jspa
-
For more information on JIRA, see: http://www.atlassian.com/software/jira