Yair Zaslavsky has posted comments on this change.
Change subject: [WIP] core: setting proper timeout mechanism for GetRootDSE
(#844733)
......................................................................
Patch Set 2: (6 inline comments)
Some comments, general concept and CORRECT usage (for a change :) ) of timeout
value is good.
Good work.
....................................................
File backend/manager/dbscripts/upgrade/pre_upgrade/0000_config.sql
Line 180: select fn_db_add_config_value('keystorePass','NoSoup4U','general');
Line 181: --Handling Keystore URL
Line 182: select fn_db_add_config_value('keystoreUrl','.keystore','general');
Line 183: select fn_db_add_config_value('LdapQueryPageSize','1000','general');
Line 184: select fn_db_add_config_value('LDAPQueryTimeout','3','general');
This line should be moved to update section, I would also keep it something
like 30, see comment on LdapOperationTimeout.
Our problem is with the connect timeout, not with the query (read) timeout.
Line 185: select fn_db_add_config_value('LDAPConnectTimeout','3','general');
Line 186: select fn_db_add_config_value('LDAPOperationTimeout','3','general');
Line 187: --Handling LDAP Security Authentication Method
Line 188: select
fn_db_add_config_value('LDAPSecurityAuthentication','GSSAPI','general');
Line 182: select fn_db_add_config_value('keystoreUrl','.keystore','general');
Line 183: select fn_db_add_config_value('LdapQueryPageSize','1000','general');
Line 184: select fn_db_add_config_value('LDAPQueryTimeout','3','general');
Line 185: select fn_db_add_config_value('LDAPConnectTimeout','3','general');
Line 186: select fn_db_add_config_value('LDAPOperationTimeout','3','general');
LdapOperationTimeout should be more than 3, maybe keep it 30 - the reason for
this is that in AD you search might be reffered to another machine (this may
take time).
Line 187: --Handling LDAP Security Authentication Method
Line 188: select
fn_db_add_config_value('LDAPSecurityAuthentication','GSSAPI','general');
Line 189: select fn_db_add_config_value('LDAPServerPort','389','general');
Line 190: select fn_db_add_config_value('LdapServers','','general');
....................................................
File
backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/adbroker/LdapBrokerUtils.java
Line 390: * @param env hashtable of parameters for ldap configuration.
Line 391: * this method adds to hashtable specific ldap configuration.
Line 392: */
Line 393: public static void addLdapConfigValues(Hashtable<String, String>
env){
Line 394: env.put("com.sun.jndi.ldap.connect.timeout",
Long.toString(Config.<Integer> GetValue(ConfigValues.LDAPQueryTimeout) * 1000));
This should be LdapConnectTimeout
Line 395: env.put("com.sun.jndi.ldap.read.timeout",
Long.toString(Config.<Integer> GetValue(ConfigValues.LDAPConnectTimeout) *
1000));
Line 396: }
Line 397:
Line 391: * this method adds to hashtable specific ldap configuration.
Line 392: */
Line 393: public static void addLdapConfigValues(Hashtable<String, String>
env){
Line 394: env.put("com.sun.jndi.ldap.connect.timeout",
Long.toString(Config.<Integer> GetValue(ConfigValues.LDAPQueryTimeout) * 1000));
Line 395: env.put("com.sun.jndi.ldap.read.timeout",
Long.toString(Config.<Integer> GetValue(ConfigValues.LDAPConnectTimeout) *
1000));
This should be LdapQueryTimeout
Line 396: }
Line 397:
....................................................
File
backend/manager/modules/common/src/main/java/org/ovirt/engine/core/common/config/ConfigValues.java
Line 1005: FenceStartStatusDelayBetweenRetriesInSec(291),
Line 1006:
Line 1007: @Reloadable
Line 1008: @TypeConverterAttribute(Integer.class)
Line 1009: @DefaultValueAttribute("3")
Should be 30
Line 1010: LDAPQueryTimeout(292),
Line 1011:
Line 1012: @Reloadable
Line 1013: @TypeConverterAttribute(Boolean.class)
Line 1443: SSHKeyAlias(377),
Line 1444:
Line 1445: @Reloadable
Line 1446: @TypeConverterAttribute(Integer.class)
Line 1447: @DefaultValueAttribute("3")
Should be 30
Line 1448: LDAPOperationTimeout(378),
Line 1449:
Line 1450: @Reloadable
Line 1451: @TypeConverterAttribute(Integer.class)
--
To view, visit http://gerrit.ovirt.org/7479
To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I59e875997b904cbdc058a45efd640f5bf1fc6579
Gerrit-PatchSet: 2
Gerrit-Project: ovirt-engine
Gerrit-Branch: master
Gerrit-Owner: Yair Zaslavsky <[email protected]>
Gerrit-Reviewer: Oved Ourfali <[email protected]>
Gerrit-Reviewer: Yair Zaslavsky <[email protected]>
Gerrit-Reviewer: Yaniv Bronhaim <[email protected]>
Gerrit-Reviewer: oVirt Jenkins CI Server
_______________________________________________
Engine-patches mailing list
[email protected]
http://lists.ovirt.org/mailman/listinfo/engine-patches
