Re: [Freeipa-devel] [PATCH] 0481 permission-find: Cache the root entry for legacy permissions

2014-03-11 Thread Martin Kosek
On 03/10/2014 12:05 PM, Petr Viktorin wrote:
> On 03/07/2014 04:45 PM, Martin Kosek wrote:
>> On 02/28/2014 03:51 PM, Petr Viktorin wrote:
>>> Hello,
>>> This reduces LDAP searches in permission-find when there are legacy
>>> permissions. The root entry (which contains all legacy permission ACIs) is 
>>> only
>>> looked up once.
>>>
>>>
>>
>> There is a conflict on one line. But when I manually resolved it, the patch
>> worked for me. We got from 176 OPS per "ipa permission-find" to ~96. This
>> should be OK for now.
>>
>> Martin
> 
> I don't see the conflict. Perhaps I mistakenly based this patch on something
> that's now pushed (though this applies cleanly to master from a week ago, 
> too...).
> Could you check again?
> 

Ok, I probably simply applied your permission fixes in wrong order.

ACK.

Pushed to master: 34c3d309d99d0ebe5eb0b935d356e30d8866c139

Martin

___
Freeipa-devel mailing list
Freeipa-devel@redhat.com
https://www.redhat.com/mailman/listinfo/freeipa-devel


Re: [Freeipa-devel] [PATCH] 0481 permission-find: Cache the root entry for legacy permissions

2014-03-10 Thread Petr Viktorin

On 03/07/2014 04:45 PM, Martin Kosek wrote:

On 02/28/2014 03:51 PM, Petr Viktorin wrote:

Hello,
This reduces LDAP searches in permission-find when there are legacy
permissions. The root entry (which contains all legacy permission ACIs) is only
looked up once.




There is a conflict on one line. But when I manually resolved it, the patch
worked for me. We got from 176 OPS per "ipa permission-find" to ~96. This
should be OK for now.

Martin


I don't see the conflict. Perhaps I mistakenly based this patch on 
something that's now pushed (though this applies cleanly to master from 
a week ago, too...).

Could you check again?

--
PetrĀ³

___
Freeipa-devel mailing list
Freeipa-devel@redhat.com
https://www.redhat.com/mailman/listinfo/freeipa-devel


Re: [Freeipa-devel] [PATCH] 0481 permission-find: Cache the root entry for legacy permissions

2014-03-07 Thread Martin Kosek
On 02/28/2014 03:51 PM, Petr Viktorin wrote:
> Hello,
> This reduces LDAP searches in permission-find when there are legacy
> permissions. The root entry (which contains all legacy permission ACIs) is 
> only
> looked up once.
> 
> 

There is a conflict on one line. But when I manually resolved it, the patch
worked for me. We got from 176 OPS per "ipa permission-find" to ~96. This
should be OK for now.

Martin

___
Freeipa-devel mailing list
Freeipa-devel@redhat.com
https://www.redhat.com/mailman/listinfo/freeipa-devel


[Freeipa-devel] [PATCH] 0481 permission-find: Cache the root entry for legacy permissions

2014-02-28 Thread Petr Viktorin

Hello,
This reduces LDAP searches in permission-find when there are legacy 
permissions. The root entry (which contains all legacy permission ACIs) 
is only looked up once.



--
PetrĀ³
From 34528e3fce17db1e4c2a23f091dc9d7fcd93b97f Mon Sep 17 00:00:00 2001
From: Petr Viktorin 
Date: Fri, 28 Feb 2014 13:38:12 +0100
Subject: [PATCH] permission-find: Cache the root entry for legacy permissions

This makes searching faster if there are many legacy permissions present.

The root entry (which contains all legacy permission ACIs) is only
looked up once.
---
 ipalib/plugins/permission.py | 31 +++
 1 file changed, 23 insertions(+), 8 deletions(-)

diff --git a/ipalib/plugins/permission.py b/ipalib/plugins/permission.py
index 670e3f1c65452fef26838558ad115ebc2aeda87a..31266adac0326d008f5c1e3f6a94d696edb0c489 100644
--- a/ipalib/plugins/permission.py
+++ b/ipalib/plugins/permission.py
@@ -518,13 +518,14 @@ def _replace_aci(self, permission_entry, old_name=None, new_acistring=None):
 return acientry, acistring
 
 def _get_aci_entry_and_string(self, permission_entry, name=None,
-  notfound_ok=False):
+  notfound_ok=False, cached_acientry=None):
 """Get the entry and ACI corresponding to the permission entry
 
 :param name: The name of the permission, or None for the cn
 :param notfound_ok:
 If true, (acientry, None) will be returned on missing ACI, rather
 than raising exception
+:param cached_acientry: See upgrade_permission()
 """
 ldap = self.api.Backend.ldap2
 if name is None:
@@ -533,10 +534,15 @@ def _get_aci_entry_and_string(self, permission_entry, name=None,
  self.api.env.basedn)
 wanted_aciname = 'permission:%s' % name
 
-try:
-acientry = ldap.get_entry(location, ['aci'])
-except errors.NotFound:
-acientry = ldap.make_entry(location)
+if (cached_acientry and
+cached_acientry.dn == location and
+'aci' in cached_acientry):
+acientry = cached_acientry
+else:
+try:
+acientry = ldap.get_entry(location, ['aci'])
+except errors.NotFound:
+acientry = ldap.make_entry(location)
 acis = acientry.get('aci', ())
 for acistring in acis:
 aci = ACI(acistring)
@@ -550,7 +556,7 @@ def _get_aci_entry_and_string(self, permission_entry, name=None,
  'in %(dn)s ') % {'name': name, 'dn': location})
 
 def upgrade_permission(self, entry, target_entry=None,
-   output_only=False):
+   output_only=False, cached_acientry=None):
 """Upgrade the given permission entry to V2, in-place
 
 The entry is only upgraded if it is a plain old-style permission,
@@ -563,11 +569,16 @@ def upgrade_permission(self, entry, target_entry=None,
 :param output_only:
 If true, the flags are not updated to V2.
 Used for the -find and -show commands.
+:param cached_acientry:
+Optional pre-retreived entry that contains the existing ACI.
+If it is None or its DN does not match the location DN,
+cached_acientry is ignored and the entry is retreived from LDAP.
 """
 if entry.get('ipapermissiontype'):
 # Only convert old-style, non-SYSTEM permissions -- i.e. no flags
 return
-base, acistring = self._get_aci_entry_and_string(entry)
+base, acistring = self._get_aci_entry_and_string(
+entry, cached_acientry=cached_acientry)
 
 if not target_entry:
 target_entry = entry
@@ -1071,8 +1082,11 @@ def post_callback(self, ldap, entries, truncated, *args, **options):
 base_dn=DN(self.obj.container_dn, self.api.env.basedn),
 filter=ldap.combine_filters(filters, rules=ldap.MATCH_ALL),
 attrs_list=attrs_list)
+# Retrieve the root entry (with all legacy ACIs) at once
+root_entry = ldap.get_entry(DN(api.env.basedn), ['aci'])
 except errors.NotFound:
 legacy_entries = ()
+cached_root_entry = None
 self.log.debug('potential legacy entries: %s', len(legacy_entries))
 nonlegacy_names = {e.single_value['cn'] for e in entries}
 for entry in legacy_entries:
@@ -1087,7 +1101,8 @@ def post_callback(self, ldap, entries, truncated, *args, **options):
 entries.pop()
 truncated = True
 break
-self.obj.upgrade_permission(entry, output_only=True)
+self.obj.upgrade_permission(entry, output_only=True,
+cached_acientry=root_