Yair Zaslavsky has posted comments on this change.

Change subject: core: Make connection timeout configurable
......................................................................


Patch Set 7: (4 inline comments)

....................................................
File backend/manager/dbscripts/upgrade/pre_upgrade/0000_config.sql
Line 530: select fn_db_add_config_value('VdsRefreshRate','2','general');
Line 531: --Handling Host Selection Algorithm default for cluster
Line 532: select 
fn_db_add_config_value('VdsSelectionAlgorithm','None','general');
Line 533: --Timeout Policy
Line 534: select fn_db_add_config_value('vdsTimeout','180','general');
Hi, we would like to keep alphabetical order in this script, please take care 
of that.
Line 535: select fn_db_add_config_value('vdsRetries','3','3.0');
Line 536: select fn_db_add_config_value('vdsRetries','3','3.1');
Line 537: select fn_db_add_config_value('vdsConnectionTimeout','180','3.0');
Line 538: select fn_db_add_config_value('vdsConnectionTimeout','180','3.1');


....................................................
File 
backend/manager/modules/common/src/main/java/org/ovirt/engine/core/common/config/ConfigValues.java
Line 39:     @DefaultValueAttribute("EXAMPLE.COM")
Line 40:     DomainName(10),
Line 41: 
Line 42:     /**
Line 43:      * Timeout in seconds for the completion of operations invoked in 
a host. It
i wonder - invoked in a host, or on a host?
Line 44:      * should be quite large as some host operations can take more 
than 3 minutes
Line 45:      * to complete.
Line 46:      */
Line 47:     @TypeConverterAttribute(Integer.class)


....................................................
File 
backend/manager/modules/vdsbroker/src/main/java/org/ovirt/engine/core/vdsbroker/irsbroker/IrsBrokerCommand.java
Line 555:                         String clusterVersion = 
Config.DefaultConfigurationVersion;
Line 556:                         if (mCurrentVdsId != null) {
Line 557:                             VDS vds = 
DbFacade.getInstance().getVdsDao().get(mCurrentVdsId);
Line 558:                             if (vds != null) {
Line 559:                                 clusterVersion = 
vds.getvds_group_compatibility_version().getValue();
Not sure I fully understand this.
If we get cluterVersion based on the vds we from db, and this code actually 
happens if host != null,
Is there a case in which we will not have VDS entity?
Why not just to use

 VDS vds = DbFacade.getInstance().getVdsDao().get(mCurrentVdsId);
 clusterVersion = vds.getvds_group_compatibility_version().getValue()

Instead of line 555.
Line 560:                             }
Line 561:                         }
Line 562: 
Line 563:                         // Find the version specific timeouts:


....................................................
File 
backend/manager/modules/vdsbroker/src/main/java/org/ovirt/engine/core/vdsbroker/VdsManager.java
Line 179:         log.infoFormat("vdsBroker({0},{1})", _vds.gethost_name(), 
_vds.getport());
Line 180: 
Line 181:         // Get the compatibility version of the cluster, as the 
values of the
Line 182:         // timeouts used depend on that:
Line 183:         String clusterVersion = Config.DefaultConfigurationVersion;
Hi, maybe I don't understand here something, if this is for 3.1 and 3.2 hosts 
(and I assume 3.0 as well), won't you get the same clusterVersion when you use 
Config.DefaultConfigurationVersion for all hosts , whether if 3.1, 3.2 ?
Line 184:         if (_vds != null) {
Line 185:             clusterVersion = 
_vds.getvds_group_compatibility_version().getValue();
Line 186:         }
Line 187: 


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I55a49ae5d655c2105c5840decec81ae712e40c32
Gerrit-PatchSet: 7
Gerrit-Project: ovirt-engine
Gerrit-Branch: master
Gerrit-Owner: Juan Hernandez <juan.hernan...@redhat.com>
Gerrit-Reviewer: Dan Kenigsberg <dan...@redhat.com>
Gerrit-Reviewer: Doron Fediuck <dfedi...@redhat.com>
Gerrit-Reviewer: Juan Hernandez <juan.hernan...@redhat.com>
Gerrit-Reviewer: Laszlo Hornyak <lhorn...@redhat.com>
Gerrit-Reviewer: Michael Kublin <mkub...@redhat.com>
Gerrit-Reviewer: Noam Slomianko <nslom...@redhat.com>
Gerrit-Reviewer: Yair Zaslavsky <yzasl...@redhat.com>
Gerrit-Reviewer: Yaniv Kaul <yk...@redhat.com>
_______________________________________________
Engine-patches mailing list
Engine-patches@ovirt.org
http://lists.ovirt.org/mailman/listinfo/engine-patches

Reply via email to