Alissa Bonas has posted comments on this change.

Change subject: core: Locate data source in a loop
......................................................................


Patch Set 5: (4 inline comments)

....................................................
File 
backend/manager/modules/dal/src/main/java/org/ovirt/engine/core/dal/dbbroker/DbFacadeLocator.java
Line 33: 
Line 34:     // The facade singleton:
Line 35:     private static DbFacade dbFacade;
Line 36: 
Line 37:     public synchronized static DbFacade getDbFacade() {
This method became synchronized. That means that from now on expensive locking 
(performance decrease) is involved every time entering it, even when just 
getting dbFacade after its initialization.
Better to do double locking inside method to improve this situation. (with 
using volatile)
Line 38:         // Do nothing if the facade has already been created:
Line 39:         if (dbFacade != null) {
Line 40:             return dbFacade;
Line 41:         }


Line 51:         // to wait:
Line 52:         long timeStarted = System.currentTimeMillis();
Line 53: 
Line 54:         DataSource datasource = null;
Line 55:         for (;;) {
How about instead of endless loop use " do while" or "while" on a clear 
condition - like the timewaited<=connectionTimeOut?
Line 56:             // Try to locate the data source:
Line 57:             datasource = 
EjbUtils.findResource(ContainerManagedResourceType.DATA_SOURCE);
Line 58:             if (datasource != null) {
Line 59:                 break;


Line 63:             long timeNow = System.currentTimeMillis();
Line 64:             long timeWaited = timeNow - timeStarted;
Line 65:             if (timeWaited > connectionTimeout) {
Line 66:                 log.errorFormat(
Line 67:                     "The data source can't be located after wating for 
more " +
wating->waiting
Line 68:                     "than {0} seconds, giving up.", connectionTimeout 
/ 1000
Line 69:                 );
Line 70:                 return null;
Line 71:             }


Line 82:                 Thread.sleep(checkInterval);
Line 83:             }
Line 84:             catch (InterruptedException exception) {
Line 85:                 log.warnFormat(
Line 86:                     "Interrupted while waiting for data source.", 
exception
what happens when it's interrupted? there is no handling except for logging.
Line 87:                 );
Line 88:             }
Line 89:         }
Line 90: 


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I72c99c61d05e8a1619c7d1fb70af956d1050eb3a
Gerrit-PatchSet: 5
Gerrit-Project: ovirt-engine
Gerrit-Branch: master
Gerrit-Owner: Juan Hernandez <[email protected]>
Gerrit-Reviewer: Alissa Bonas <[email protected]>
Gerrit-Reviewer: Allon Mureinik <[email protected]>
Gerrit-Reviewer: Barak Azulay <[email protected]>
Gerrit-Reviewer: Juan Hernandez <[email protected]>
Gerrit-Reviewer: Roy Golan <[email protected]>
Gerrit-Reviewer: Yair Zaslavsky <[email protected]>
_______________________________________________
Engine-patches mailing list
[email protected]
http://lists.ovirt.org/mailman/listinfo/engine-patches

Reply via email to