Juan Hernandez has posted comments on this change.
Change subject: core: Locate data source in a loop
......................................................................
Patch Set 1: (2 inline comments)
....................................................
File
backend/manager/modules/dal/src/main/java/org/ovirt/engine/core/dal/dbbroker/DbFacadeLocator.java
Line 42: if (datasource != null) {
Line 43: break;
Line 44: }
Line 45:
Line 46: // Tell the user that the lookup failed but that we
will try
Yair, we can do that. But what should we do when we fail after unsuccessfully
retrying? The only reasonable thing I see we can do is a log message and
System.exit(whatever).
Line 47: // again in a few seconds:
Line 48: log.warn(
Line 49: "The datasource can't be located. This probably
means " +
Line 50: "that DNS is not working correctly or is slow,
please " +
Line 51: "check it. Will try again in a few seconds.");
Line 52:
Line 53: // Wait a bit before trying again:
Line 54: Thread.sleep(DEFAULT_INTERVAL_VALUE);
Line 55: }
Alissa, all that is true, and I mostly agree.
However the logic in that static block was not introduced by this patch, it was
already there, so the patch should not be judged according to the goodness or
badness of putting logic in static blocks.
Also, if this loop doesn't finish, that means that there is a severe
configuration problem, as there is no database connection, and no point (or
possibility) in doing any other useful task. The infinite loop is bad, but
letting the application start without the possibility to get database
connections is even worse.
So in my opinion this patch doesn't make the application worse when things work
as expected (when the data source is deployed before the application), it
improves the application a lot when things don't work as expected (when the
pool is deployed after the application) and improves the application a bit with
a better log message when there are severe configuration issues.
Line 56:
Line 57: // Create the facade and return it:
Line 58: dbFacade = new DbFacade();
Line 59: dbFacade.setDbEngineDialect(loadDbEngineDialect());
--
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: 1
Gerrit-Project: ovirt-engine
Gerrit-Branch: master
Gerrit-Owner: Juan Hernandez <juan.hernan...@redhat.com>
Gerrit-Reviewer: Alissa Bonas <abo...@redhat.com>
Gerrit-Reviewer: Barak Azulay <bazu...@redhat.com>
Gerrit-Reviewer: Juan Hernandez <juan.hernan...@redhat.com>
Gerrit-Reviewer: Roy Golan <rgo...@redhat.com>
Gerrit-Reviewer: Yair Zaslavsky <yzasl...@redhat.com>
_______________________________________________
Engine-patches mailing list
Engine-patches@ovirt.org
http://lists.ovirt.org/mailman/listinfo/engine-patches
