On 30.11.2015 14:15, Martin Basti wrote:
On 30.11.2015 08:19, Jan Cholasta wrote:
On 24.11.2015 10:15, Martin Basti wrote:
On 20.11.2015 09:00, Jan Cholasta wrote:
On 19.11.2015 14:13, Jan Cholasta wrote:
On 19.11.2015 14:09, Martin Babinsky wrote:
On 11/19/2015 01:08 PM, Martin Basti wrote:
On 18.11.2015 14:26, Martin Basti wrote:
On 18.11.2015 14:24, Martin Kosek wrote:
On 11/18/2015 02:18 PM, Martin Basti wrote:
On 18.11.2015 13:55, Martin Kosek wrote:
On 11/18/2015 01:30 PM, Martin Basti wrote:
+ DEFAULT_TIME_LIMIT = -1.0
+ DEFAULT_SIZE_LIMIT = 0
...
if time_limit is None or time_limit == 0:
- time_limit = -1.0
+ if self.time_limit is not None and self.time_limit
!= 0:
+ time_limit = self.time_limit
+ else:
+ time_limit = self.DEFAULT_TIME_LIMIT
+
I wonder why is the -1 default time limit a float number, I
would
expect that
some trouble may emerge out of it. Maybe Rob knows?
Python LDAP allows to have smaller granularity than seconds
and it
internally
convert time limit values to float.
Hm, ok. I was just curious since the value we expose in API is Int
and the
default does not use the smaller granularity, so I was thinking
"Why
bother?".
If it is fixed in your patch, good. If not, good also, no need to
bikeshed here
I suppose.
I just keep the original values there, I do not want to touch it
with
this patch
Updated patch attached, the original one has wrong default limit for
ldap2
ACK
Hold your horses.
1) The upgrade_ldapsearch_limit option is not very useful, either add
generic search_time_limit and search_records_limit options, or don't
add any options at all (which, incidentally, is exactly what the
ticket asks for).
2) The limits are not passed to the connection using __init__()
anywhere, so there is no need to have arguments for them.
3) Rather than adding layers of if statements everywhere, implement
the time_limit and size_limit attributes as properties.
4) Use config_entry.single_value when setting defaults in
ldap2.get_ipa_config().
BTW, add comments only when it's not obvious what the code does - a
comment saying "set defaults" above two .setdefault() calls is pretty
redundant.
Updated patch attached
In ldap2.create_connection(), you should set self.time_limit only if
time_limit is not None, ditto for size limit, instead of handling None
in the properties' setters.
You don't need to handle the limits in ldap2.find_entries() anymore,
since it is handled by LDAPClient.find_entries(), so you can remove
the related code completely from ldap2.find_entries().
Updated patch attached.
The defaults should be set in LDAPClient class attributes rather than in
__init__().
Besides that ACK.
To speed things up I have amended your patch myself, see attachment.
Pushed to master: 2a1a3c498a71e85193af76a25333ebe9011e6b2a
--
Jan Cholasta
From 097b59f49776c4842c4ff774541f9f101bbda753 Mon Sep 17 00:00:00 2001
From: Martin Basti <mba...@redhat.com>
Date: Wed, 18 Nov 2015 10:31:05 +0100
Subject: [PATCH] Upgrade: increase time limit for upgrades
Default ldap search limit is now 30 sec by default during upgrade.
Limits must be changed for the whole ldap2 connection, because this
connection is used inside update plugins and commands called from
upgrade.
Together with increasing the time limit, also size limit should be
unlimited during upgrade. With sizelimit=None we may get the
TimeExceeded exception from getting default value of the sizelimit from LDAP.
https://fedorahosted.org/freeipa/ticket/5267
---
ipalib/plugins/permission.py | 3 +-
ipapython/ipaldap.py | 11 +++++--
ipaserver/install/ldapupdate.py | 5 ++-
ipaserver/plugins/ldap2.py | 72 ++++++++++++++++++++++++-----------------
4 files changed, 57 insertions(+), 34 deletions(-)
diff --git a/ipalib/plugins/permission.py b/ipalib/plugins/permission.py
index b17b61e..cbc48f5 100644
--- a/ipalib/plugins/permission.py
+++ b/ipalib/plugins/permission.py
@@ -1292,8 +1292,7 @@ class permission_find(baseldap.LDAPSearch):
if 'sizelimit' in options:
max_entries = options['sizelimit']
else:
- config = ldap.get_ipa_config()
- max_entries = int(config.single_value['ipasearchrecordslimit'])
+ max_entries = self.api.Backend.ldap2.size_limit
filters = ['(objectclass=ipaPermission)',
'(!(ipaPermissionType=V2))']
diff --git a/ipapython/ipaldap.py b/ipapython/ipaldap.py
index 7e5bc04..bbd27ac 100644
--- a/ipapython/ipaldap.py
+++ b/ipapython/ipaldap.py
@@ -689,6 +689,9 @@ class LDAPClient(object):
'nsslapd-minssf-exclude-rootdse': True,
})
+ time_limit = -1.0 # unlimited
+ size_limit = 0 # unlimited
+
def __init__(self, ldap_uri, start_tls=False, force_schema_updates=False,
no_schema=False, decode_attrs=True):
"""Create LDAPClient object.
@@ -1294,10 +1297,14 @@ class LDAPClient(object):
res = []
truncated = False
- if time_limit is None or time_limit == 0:
+ if time_limit is None:
+ time_limit = self.time_limit
+ if time_limit == 0:
time_limit = -1.0
+
if size_limit is None:
- size_limit = 0
+ size_limit = self.size_limit
+
if not isinstance(size_limit, int):
size_limit = int(size_limit)
if not isinstance(time_limit, float):
diff --git a/ipaserver/install/ldapupdate.py b/ipaserver/install/ldapupdate.py
index 86c0110..2ed5e8f 100644
--- a/ipaserver/install/ldapupdate.py
+++ b/ipaserver/install/ldapupdate.py
@@ -46,6 +46,7 @@ from ipapython.ipa_log_manager import *
from ipapython.ipautil import wait_for_open_socket
UPDATES_DIR=paths.UPDATES_DIR
+UPDATE_SEARCH_TIME_LIMIT = 30 # seconds
def connect(ldapi=False, realm=None, fqdn=None, dm_password=None, pw_name=None):
@@ -867,7 +868,9 @@ class LDAPUpdate:
self.api.Backend.ldap2.connect(
bind_dn=DN(('cn', 'Directory Manager')),
bind_pw=self.dm_password,
- autobind=self.ldapi)
+ autobind=self.ldapi,
+ time_limit=UPDATE_SEARCH_TIME_LIMIT,
+ size_limit=0)
self.conn = self.api.Backend.ldap2
else:
raise RuntimeError("Offline updates are not supported.")
diff --git a/ipaserver/plugins/ldap2.py b/ipaserver/plugins/ldap2.py
index 4cdc561..a1f1e19 100644
--- a/ipaserver/plugins/ldap2.py
+++ b/ipaserver/plugins/ldap2.py
@@ -72,6 +72,39 @@ class ldap2(CrudBackend, LDAPClient):
LDAPClient.__init__(self, ldap_uri,
force_schema_updates=force_schema_updates)
+ self.__time_limit = None
+ self.__size_limit = None
+
+ @property
+ def time_limit(self):
+ if self.__time_limit is None:
+ return float(self.get_ipa_config().single_value.get(
+ 'ipasearchtimelimit', 2))
+ return self.__time_limit
+
+ @time_limit.setter
+ def time_limit(self, val):
+ self.__time_limit = float(val)
+
+ @time_limit.deleter
+ def time_limit(self):
+ self.__time_limit = None
+
+ @property
+ def size_limit(self):
+ if self.__size_limit is None:
+ return int(self.get_ipa_config().single_value.get(
+ 'ipasearchrecordslimit', 0))
+ return self.__size_limit
+
+ @size_limit.setter
+ def size_limit(self, val):
+ self.__size_limit = int(val)
+
+ @size_limit.deleter
+ def size_limit(self):
+ self.__size_limit = None
+
def _connect(self):
# Connectible.conn is a proxy to thread-local storage;
# do not set it
@@ -87,7 +120,7 @@ class ldap2(CrudBackend, LDAPClient):
def create_connection(self, ccache=None, bind_dn=None, bind_pw='',
tls_cacertfile=None, tls_certfile=None, tls_keyfile=None,
debug_level=0, autobind=AUTOBIND_AUTO, serverctrls=None,
- clientctrls=None):
+ clientctrls=None, time_limit=None, size_limit=None):
"""
Connect to LDAP server.
@@ -114,6 +147,11 @@ class ldap2(CrudBackend, LDAPClient):
if tls_keyfile is not None:
_ldap.set_option(_ldap.OPT_X_TLS_KEYFILE, tls_keyfile)
+ if time_limit is not None:
+ self.time_limit = time_limit
+ if size_limit is not None:
+ self.size_limit = size_limit
+
if debug_level:
_ldap.set_option(_ldap.OPT_DEBUG_LEVEL, debug_level)
@@ -175,31 +213,9 @@ class ldap2(CrudBackend, LDAPClient):
# ignore when trying to unbind multiple times
pass
- def find_entries(self, filter=None, attrs_list=None, base_dn=None,
- scope=_ldap.SCOPE_SUBTREE, time_limit=None,
- size_limit=None, search_refs=False, paged_search=False):
-
- def _get_limits():
- """Get configured global limits, caching them for more calls"""
- if not _lims:
- config = self.get_ipa_config()
- _lims['time'] = int(config.get('ipasearchtimelimit', [None])[0])
- _lims['size'] = int(config.get('ipasearchrecordslimit', [None])[0])
- return _lims
- _lims = {}
-
- if time_limit is None:
- time_limit = _get_limits()['time']
- if size_limit is None:
- size_limit = _get_limits()['size']
-
- res, truncated = super(ldap2, self).find_entries(
- filter=filter, attrs_list=attrs_list, base_dn=base_dn, scope=scope,
- time_limit=time_limit, size_limit=size_limit,
- search_refs=search_refs, paged_search=paged_search)
- return (res, truncated)
-
- config_defaults = {'ipasearchtimelimit': [2], 'ipasearchrecordslimit': [0]}
+ del self.time_limit
+ del self.size_limit
+
def get_ipa_config(self, attrs_list=None):
"""Returns the IPA configuration entry (dn, entry_attrs)."""
@@ -223,9 +239,7 @@ class ldap2(CrudBackend, LDAPClient):
config_entry = entries[0]
except errors.NotFound:
config_entry = self.make_entry(dn)
- for a in self.config_defaults:
- if a not in config_entry:
- config_entry[a] = self.config_defaults[a]
+
context.config_entry = config_entry
return config_entry
--
2.4.3
--
Manage your subscription for the Freeipa-devel mailing list:
https://www.redhat.com/mailman/listinfo/freeipa-devel
Contribute to FreeIPA: http://www.freeipa.org/page/Contribute/Code
