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

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.