Kanagaraj M has posted comments on this change.

Change subject: webadmin: Avoid duplicate server queries
......................................................................


Patch Set 2: (4 inline comments)

....................................................
File 
frontend/webadmin/modules/frontend/src/main/java/org/ovirt/engine/ui/frontend/Frontend.java
Line 201:                         
pendingRequests.remove(callback.getQueryKey());
Line 202:                         RunQuery(wrapper.getQueryType(), 
wrapper.getParameters(), wrapper.getCallback());
Line 203:                     } else {
Line 204:                         
currentRequests.remove(callback.getQueryKey());
Line 205:                     }
There is a possibility of subsequent queries of same type will not be executed.

the Query which has completed should be removed from currentRequests, 
irrespective of whether there is any pendingRequests of the same type or not. 
Otherwise the next query of the same type will not be executed and goes to 
pendingRequests as it sees the same type is present in currentRequests. 

else block is not needed here, you could move 

 currentRequests.remove(callback.getQueryKey()); 

as the first line of finally block.same as below comment.
Line 206:                 }
Line 207:             }
Line 208: 
Line 209:             @Override


Line 225:                             }
Line 226:                         }
Line 227:                         if (callback.isHandleFailure()) {
Line 228:                             
callback.getDel().OnSuccess(callback.getModel(), result);
Line 229:                         }
Duplicate calls to OnSuccess.
Line 230:                     } else {
Line 231:                         callback.setOriginalReturnValue(result);
Line 232:                         if (callback.getConverter() != null) {
Line 233:                             
callback.getDel().OnSuccess(callback.getModel(),


Line 240: 
Line 241:                     raiseQueryCompleteEvent(queryType, 
callback.getContext());
Line 242:                 }
Line 243:                 finally {
Line 244:                     QueryWrapper wrapper = 
pendingRequests.get(callback.getQueryKey());
wrapper.getKey() will also work here, so you can avoid having a new field 
'queryKey' and setter/getter in AsyncQuery.
Line 245:                     if (wrapper != null) {
Line 246:                         
pendingRequests.remove(callback.getQueryKey());
Line 247:                         RunQuery(wrapper.getQueryType(), 
wrapper.getParameters(), wrapper.getCallback());
Line 248:                     } else {


Line 246:                         
pendingRequests.remove(callback.getQueryKey());
Line 247:                         RunQuery(wrapper.getQueryType(), 
wrapper.getParameters(), wrapper.getCallback());
Line 248:                     } else {
Line 249:                         
currentRequests.remove(callback.getQueryKey());
Line 250:                     }
same here.
Line 251:                 }
Line 252:             }
Line 253:         });
Line 254:     }


--
To view, visit http://gerrit.ovirt.org/9554
To unsubscribe, visit http://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I483a1c449f1eb2e9820464dbaf0a686f90923bb6
Gerrit-PatchSet: 2
Gerrit-Project: ovirt-engine
Gerrit-Branch: master
Gerrit-Owner: Gilad Chaplik <gchap...@redhat.com>
Gerrit-Reviewer: Alona Kaplan <alkap...@redhat.com>
Gerrit-Reviewer: Daniel Erez <de...@redhat.com>
Gerrit-Reviewer: Einav Cohen <eco...@redhat.com>
Gerrit-Reviewer: Gilad Chaplik <gchap...@redhat.com>
Gerrit-Reviewer: Kanagaraj M <kmayi...@redhat.com>
Gerrit-Reviewer: Vojtech Szocs <vsz...@redhat.com>
_______________________________________________
Engine-patches mailing list
Engine-patches@ovirt.org
http://lists.ovirt.org/mailman/listinfo/engine-patches

Reply via email to