Re: [Freeipa-devel] [PATCH] 1083 improve migration performance
On 02/04/2013 08:05 PM, Rob Crittenden wrote:
Martin Kosek wrote:
On 02/01/2013 04:21 PM, Rob Crittenden wrote:
I did some analysis on migration and found several areas impacting
performance:
1. We were calling user_mod to reset the magic value in description to not
create a UPG. This caused a lot of unnecessary queries to display the user.
2. We check the remote LDAP server to make sure that the GID is valid and
added
a cache. We lacked a negative cache.
3. The biggest drag on performance was managing the default users group.
After
about 1000 users it would take about half a second to calculate the modlist
and
another half second for 389-ds to apply the change.
This patch addresses all of these.
For the last what I do is only do the group addition every 100 records. A
query
is run to find all users who aren't in the default users group and those are
added.
I also added a bit of logging so one can better track the progress of
migration.
I migrated 12.5k users with compat enabled in 3 1/2 hours.
I migrated the same 12.5k users and 2k groups with compat disabled in 30
minutes.
By contrast when I started, with compat enabled, I migrated:
1000 users in 7 minutes
2000 users in 27 minutes
3000 users in 1 hour
rob
Good job, this should improve the migration plugin perfomance a lot. Just few
minor remarks:
1) I am not native speaker, but this looks strange to me:
+_krb_failed_msg = _('Unable to determine Kerberos principal %s already
exists.
Use \'ipa user-mod\' to set it manually.')
Yup, typo on my part.
Shouldn't it read Unable to determine IF Kerberos principal...?
2) In:
+searchfilter = ((objectclass=posixAccount)(!(memberof=%s))) %
group_dn
+(result, truncated) = ldap.find_entries(searchfilter,
+['member'], api.env.container_user, scope=_ldap.SCOPE_SUBTREE,
+time_limit = -1)
Shouldn't we search with empty attrs_list (attrs_list=[''])? We do not need
nor use the member attribute anyway.
Sure. I had it in my head that asking for an empty list returned everything. I
did some quick testing and asking for a blank attribute returns no attributes,
so I think will work.
3) In
+if migrate_cnt 0 and migrate_cnt % 100:
+api.log.info(%d %ss migrated. %s elapsed. %
(migrate_cnt, ldap_obj_name, total_dur))
I think you wanted to do this condition:
if migrate_cnt 0 and migrate_cnt % 100 == 0:
Yup, this is what I get for adding something right before submitting the
patch.
Fixed.
4) In _update_default_group:
+api.log.debug('Adding users to group duration %s' % d)
I would improve it this way:
+mode = (forced) if force else
+api.log.debug('Adding %d users to group%s, duration %s',
migrate_cnt,
mode, d)
Sure, added.
5) We now print a lot of interesting migration-related information to IPA
server httpd error_log. I think it may be useful to also add a note about it
to
ipa help migration I think that regular admins may not have a clue that we
log information like this to this error log.
I added a short logging section.
rob
Almost there!
1) Few new typos:
+recommended that this be evaluated post-migration to correct or
^^
...
+/etc/ipa/defult.conf or /etc/ipa/server.conf, then an entry will be printed
^^^
2) ipausers group update may report error for the whole migration if there is
no user to update (e.g. if all migrated users already exist in IPA):
# ipa migrate-ds --with-compat...
ipa: ERROR: no such entry
This is the misbehaving LDAP search:
+searchfilter = ((objectclass=posixAccount)(!(memberof=%s))) %
group_dn
+(result, truncated) = ldap.find_entries(searchfilter,
+[''], api.env.container_user, scope=_ldap.SCOPE_SUBTREE,
+time_limit = -1)
Martin
___
Freeipa-devel mailing list
Freeipa-devel@redhat.com
https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCH] 1083 improve migration performance
Martin Kosek wrote:
On 02/04/2013 08:05 PM, Rob Crittenden wrote:
Martin Kosek wrote:
On 02/01/2013 04:21 PM, Rob Crittenden wrote:
I did some analysis on migration and found several areas impacting performance:
1. We were calling user_mod to reset the magic value in description to not
create a UPG. This caused a lot of unnecessary queries to display the user.
2. We check the remote LDAP server to make sure that the GID is valid and added
a cache. We lacked a negative cache.
3. The biggest drag on performance was managing the default users group. After
about 1000 users it would take about half a second to calculate the modlist and
another half second for 389-ds to apply the change.
This patch addresses all of these.
For the last what I do is only do the group addition every 100 records. A query
is run to find all users who aren't in the default users group and those are
added.
I also added a bit of logging so one can better track the progress of
migration.
I migrated 12.5k users with compat enabled in 3 1/2 hours.
I migrated the same 12.5k users and 2k groups with compat disabled in 30
minutes.
By contrast when I started, with compat enabled, I migrated:
1000 users in 7 minutes
2000 users in 27 minutes
3000 users in 1 hour
rob
Good job, this should improve the migration plugin perfomance a lot. Just few
minor remarks:
1) I am not native speaker, but this looks strange to me:
+_krb_failed_msg = _('Unable to determine Kerberos principal %s already exists.
Use \'ipa user-mod\' to set it manually.')
Yup, typo on my part.
Shouldn't it read Unable to determine IF Kerberos principal...?
2) In:
+searchfilter = ((objectclass=posixAccount)(!(memberof=%s))) %
group_dn
+(result, truncated) = ldap.find_entries(searchfilter,
+['member'], api.env.container_user, scope=_ldap.SCOPE_SUBTREE,
+time_limit = -1)
Shouldn't we search with empty attrs_list (attrs_list=[''])? We do not need
nor use the member attribute anyway.
Sure. I had it in my head that asking for an empty list returned everything. I
did some quick testing and asking for a blank attribute returns no attributes,
so I think will work.
3) In
+if migrate_cnt 0 and migrate_cnt % 100:
+api.log.info(%d %ss migrated. %s elapsed. %
(migrate_cnt, ldap_obj_name, total_dur))
I think you wanted to do this condition:
if migrate_cnt 0 and migrate_cnt % 100 == 0:
Yup, this is what I get for adding something right before submitting the patch.
Fixed.
4) In _update_default_group:
+api.log.debug('Adding users to group duration %s' % d)
I would improve it this way:
+mode = (forced) if force else
+api.log.debug('Adding %d users to group%s, duration %s', migrate_cnt,
mode, d)
Sure, added.
5) We now print a lot of interesting migration-related information to IPA
server httpd error_log. I think it may be useful to also add a note about it to
ipa help migration I think that regular admins may not have a clue that we
log information like this to this error log.
I added a short logging section.
rob
Almost there!
1) Few new typos:
+recommended that this be evaluated post-migration to correct or
^^
I think this was correct but it was definitely awkward so I re-worded
the sentence.
...
+/etc/ipa/defult.conf or /etc/ipa/server.conf, then an entry will be printed
^^^
Fixed
2) ipausers group update may report error for the whole migration if there is
no user to update (e.g. if all migrated users already exist in IPA):
# ipa migrate-ds --with-compat...
ipa: ERROR: no such entry
This is the misbehaving LDAP search:
+searchfilter = ((objectclass=posixAccount)(!(memberof=%s))) %
group_dn
+(result, truncated) = ldap.find_entries(searchfilter,
+[''], api.env.container_user, scope=_ldap.SCOPE_SUBTREE,
+time_limit = -1)
Ok, wrapped in try/except.
I also moved the ldap.update_entry() block so we don't try the update if
we know we have no new members.
rob
From 27d2df49e1e17e7171b3654b68e69ddf536f1969 Mon Sep 17 00:00:00 2001
From: Rob Crittenden rcrit...@redhat.com
Date: Tue, 29 Jan 2013 11:19:30 -0500
Subject: [PATCH] Improve migration performance
Add new users to the default users group in batches of 100. The
biggest overhead of migration is in calculating the modlist when
managing the default user's group and applying the changes. A
significant amount of time can be saved by not doing this on every
add operation.
Some other minor improvements include:
Add a negative cache for groups not found in the remote LDAP server.
Replace call to user_mod with a direct LDAP update.
Catch some occurances of LimitError and handle more gracefully.
I also added some debug logging to report on migration status and
performance.
https://fedorahosted.org/freeipa/ticket/3386
---
ipalib/plugins/migration.py | 96
Re: [Freeipa-devel] [PATCH] 1083 improve migration performance
On 02/05/2013 03:27 PM, Rob Crittenden wrote:
Martin Kosek wrote:
On 02/04/2013 08:05 PM, Rob Crittenden wrote:
Martin Kosek wrote:
On 02/01/2013 04:21 PM, Rob Crittenden wrote:
I did some analysis on migration and found several areas impacting
performance:
1. We were calling user_mod to reset the magic value in description to not
create a UPG. This caused a lot of unnecessary queries to display the
user.
2. We check the remote LDAP server to make sure that the GID is valid and
added
a cache. We lacked a negative cache.
3. The biggest drag on performance was managing the default users group.
After
about 1000 users it would take about half a second to calculate the
modlist and
another half second for 389-ds to apply the change.
This patch addresses all of these.
For the last what I do is only do the group addition every 100 records. A
query
is run to find all users who aren't in the default users group and those
are
added.
I also added a bit of logging so one can better track the progress of
migration.
I migrated 12.5k users with compat enabled in 3 1/2 hours.
I migrated the same 12.5k users and 2k groups with compat disabled in 30
minutes.
By contrast when I started, with compat enabled, I migrated:
1000 users in 7 minutes
2000 users in 27 minutes
3000 users in 1 hour
rob
Good job, this should improve the migration plugin perfomance a lot. Just
few
minor remarks:
1) I am not native speaker, but this looks strange to me:
+_krb_failed_msg = _('Unable to determine Kerberos principal %s already
exists.
Use \'ipa user-mod\' to set it manually.')
Yup, typo on my part.
Shouldn't it read Unable to determine IF Kerberos principal...?
2) In:
+searchfilter = ((objectclass=posixAccount)(!(memberof=%s))) %
group_dn
+(result, truncated) = ldap.find_entries(searchfilter,
+['member'], api.env.container_user, scope=_ldap.SCOPE_SUBTREE,
+time_limit = -1)
Shouldn't we search with empty attrs_list (attrs_list=[''])? We do not
need
nor use the member attribute anyway.
Sure. I had it in my head that asking for an empty list returned
everything. I
did some quick testing and asking for a blank attribute returns no
attributes,
so I think will work.
3) In
+if migrate_cnt 0 and migrate_cnt % 100:
+api.log.info(%d %ss migrated. %s elapsed. %
(migrate_cnt, ldap_obj_name, total_dur))
I think you wanted to do this condition:
if migrate_cnt 0 and migrate_cnt % 100 == 0:
Yup, this is what I get for adding something right before submitting the
patch.
Fixed.
4) In _update_default_group:
+api.log.debug('Adding users to group duration %s' % d)
I would improve it this way:
+mode = (forced) if force else
+api.log.debug('Adding %d users to group%s, duration %s',
migrate_cnt,
mode, d)
Sure, added.
5) We now print a lot of interesting migration-related information to IPA
server httpd error_log. I think it may be useful to also add a note about
it to
ipa help migration I think that regular admins may not have a clue that
we
log information like this to this error log.
I added a short logging section.
rob
Almost there!
1) Few new typos:
+recommended that this be evaluated post-migration to correct or
^^
I think this was correct but it was definitely awkward so I re-worded the
sentence.
...
+/etc/ipa/defult.conf or /etc/ipa/server.conf, then an entry will be printed
^^^
Fixed
2) ipausers group update may report error for the whole migration if there is
no user to update (e.g. if all migrated users already exist in IPA):
# ipa migrate-ds --with-compat...
ipa: ERROR: no such entry
This is the misbehaving LDAP search:
+searchfilter = ((objectclass=posixAccount)(!(memberof=%s))) %
group_dn
+(result, truncated) = ldap.find_entries(searchfilter,
+[''], api.env.container_user, scope=_ldap.SCOPE_SUBTREE,
+time_limit = -1)
Ok, wrapped in try/except.
I also moved the ldap.update_entry() block so we don't try the update if we
know we have no new members.
rob
ACK. Pushed to master, ipa-3-1, ipa-3-0.
Martin
___
Freeipa-devel mailing list
Freeipa-devel@redhat.com
https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCH] 1083 improve migration performance
On 02/01/2013 04:21 PM, Rob Crittenden wrote:
I did some analysis on migration and found several areas impacting
performance:
1. We were calling user_mod to reset the magic value in description to not
create a UPG. This caused a lot of unnecessary queries to display the user.
2. We check the remote LDAP server to make sure that the GID is valid and
added
a cache. We lacked a negative cache.
3. The biggest drag on performance was managing the default users group. After
about 1000 users it would take about half a second to calculate the modlist
and
another half second for 389-ds to apply the change.
This patch addresses all of these.
For the last what I do is only do the group addition every 100 records. A
query
is run to find all users who aren't in the default users group and those are
added.
I also added a bit of logging so one can better track the progress of
migration.
I migrated 12.5k users with compat enabled in 3 1/2 hours.
I migrated the same 12.5k users and 2k groups with compat disabled in 30
minutes.
By contrast when I started, with compat enabled, I migrated:
1000 users in 7 minutes
2000 users in 27 minutes
3000 users in 1 hour
rob
Good job, this should improve the migration plugin perfomance a lot. Just few
minor remarks:
1) I am not native speaker, but this looks strange to me:
+_krb_failed_msg = _('Unable to determine Kerberos principal %s already exists.
Use \'ipa user-mod\' to set it manually.')
Shouldn't it read Unable to determine IF Kerberos principal...?
2) In:
+searchfilter = ((objectclass=posixAccount)(!(memberof=%s))) %
group_dn
+(result, truncated) = ldap.find_entries(searchfilter,
+['member'], api.env.container_user, scope=_ldap.SCOPE_SUBTREE,
+time_limit = -1)
Shouldn't we search with empty attrs_list (attrs_list=[''])? We do not need
nor use the member attribute anyway.
3) In
+if migrate_cnt 0 and migrate_cnt % 100:
+api.log.info(%d %ss migrated. %s elapsed. %
(migrate_cnt, ldap_obj_name, total_dur))
I think you wanted to do this condition:
if migrate_cnt 0 and migrate_cnt % 100 == 0:
Otherwise, this logs INFO log level entry for every migrated user. AFAIU, this
logging density should only be enabled when httpd is run with debug=true.
4) In _update_default_group:
+api.log.debug('Adding users to group duration %s' % d)
I would improve it this way:
+mode = (forced) if force else
+api.log.debug('Adding %d users to group%s, duration %s', migrate_cnt,
mode, d)
5) We now print a lot of interesting migration-related information to IPA
server httpd error_log. I think it may be useful to also add a note about it to
ipa help migration I think that regular admins may not have a clue that we
log information like this to this error log.
Martin
___
Freeipa-devel mailing list
Freeipa-devel@redhat.com
https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCH] 1083 improve migration performance
Martin Kosek wrote:
On 02/01/2013 04:21 PM, Rob Crittenden wrote:
I did some analysis on migration and found several areas impacting performance:
1. We were calling user_mod to reset the magic value in description to not
create a UPG. This caused a lot of unnecessary queries to display the user.
2. We check the remote LDAP server to make sure that the GID is valid and added
a cache. We lacked a negative cache.
3. The biggest drag on performance was managing the default users group. After
about 1000 users it would take about half a second to calculate the modlist and
another half second for 389-ds to apply the change.
This patch addresses all of these.
For the last what I do is only do the group addition every 100 records. A query
is run to find all users who aren't in the default users group and those are
added.
I also added a bit of logging so one can better track the progress of migration.
I migrated 12.5k users with compat enabled in 3 1/2 hours.
I migrated the same 12.5k users and 2k groups with compat disabled in 30
minutes.
By contrast when I started, with compat enabled, I migrated:
1000 users in 7 minutes
2000 users in 27 minutes
3000 users in 1 hour
rob
Good job, this should improve the migration plugin perfomance a lot. Just few
minor remarks:
1) I am not native speaker, but this looks strange to me:
+_krb_failed_msg = _('Unable to determine Kerberos principal %s already exists.
Use \'ipa user-mod\' to set it manually.')
Yup, typo on my part.
Shouldn't it read Unable to determine IF Kerberos principal...?
2) In:
+searchfilter = ((objectclass=posixAccount)(!(memberof=%s))) %
group_dn
+(result, truncated) = ldap.find_entries(searchfilter,
+['member'], api.env.container_user, scope=_ldap.SCOPE_SUBTREE,
+time_limit = -1)
Shouldn't we search with empty attrs_list (attrs_list=[''])? We do not need
nor use the member attribute anyway.
Sure. I had it in my head that asking for an empty list returned
everything. I did some quick testing and asking for a blank attribute
returns no attributes, so I think will work.
3) In
+if migrate_cnt 0 and migrate_cnt % 100:
+api.log.info(%d %ss migrated. %s elapsed. %
(migrate_cnt, ldap_obj_name, total_dur))
I think you wanted to do this condition:
if migrate_cnt 0 and migrate_cnt % 100 == 0:
Yup, this is what I get for adding something right before submitting the
patch. Fixed.
4) In _update_default_group:
+api.log.debug('Adding users to group duration %s' % d)
I would improve it this way:
+mode = (forced) if force else
+api.log.debug('Adding %d users to group%s, duration %s', migrate_cnt,
mode, d)
Sure, added.
5) We now print a lot of interesting migration-related information to IPA
server httpd error_log. I think it may be useful to also add a note about it to
ipa help migration I think that regular admins may not have a clue that we
log information like this to this error log.
I added a short logging section.
rob
From 0917369aab3a9f50ae993cb22e51326648dfda19 Mon Sep 17 00:00:00 2001
From: Rob Crittenden rcrit...@redhat.com
Date: Tue, 29 Jan 2013 11:19:30 -0500
Subject: [PATCH] Improve migration performance
Add new users to the default users group in batches of 100. The
biggest overhead of migration is in calculating the modlist when
managing the default user's group and applying the changes. A
significant amount of time can be saved by not doing this on every
add operation.
Some other minor improvements include:
Add a negative cache for groups not found in the remote LDAP server.
Replace call to user_mod with a direct LDAP update.
Catch some occurances of LimitError and handle more gracefully.
I also added some debug logging to report on migration status and
performance.
https://fedorahosted.org/freeipa/ticket/3386
---
ipalib/plugins/migration.py | 92 +
1 file changed, 84 insertions(+), 8 deletions(-)
diff --git a/ipalib/plugins/migration.py b/ipalib/plugins/migration.py
index 81df59a23ff46b770076cb56a7d78fed8db26029..8dcc4c56d9dcf62e9e35dcbcd3f0202b6954b503 100644
--- a/ipalib/plugins/migration.py
+++ b/ipalib/plugins/migration.py
@@ -31,6 +31,7 @@ if api.env.in_server and api.env.context in ['lite', 'server']:
raise e
from ipalib import _
from ipapython.dn import DN
+import datetime
__doc__ = _(
Migration to IPA
@@ -72,6 +73,12 @@ If a base DN is not provided with --basedn then IPA will use either
the value of defaultNamingContext if it is set or the first value
in namingContexts set in the root of the remote LDAP server.
+Users are added as members to the default user group. This can be a
+time-intensive task so during migration this is done in a batch
+mode for every 100 users. As a result there will be a window in which
+users will be added to IPA but will not be members of the default
+user group.
+
EXAMPLES:
The simplest
[Freeipa-devel] [PATCH] 1083 improve migration performance
I did some analysis on migration and found several areas impacting
performance:
1. We were calling user_mod to reset the magic value in description to
not create a UPG. This caused a lot of unnecessary queries to display
the user.
2. We check the remote LDAP server to make sure that the GID is valid
and added a cache. We lacked a negative cache.
3. The biggest drag on performance was managing the default users group.
After about 1000 users it would take about half a second to calculate
the modlist and another half second for 389-ds to apply the change.
This patch addresses all of these.
For the last what I do is only do the group addition every 100 records.
A query is run to find all users who aren't in the default users group
and those are added.
I also added a bit of logging so one can better track the progress of
migration.
I migrated 12.5k users with compat enabled in 3 1/2 hours.
I migrated the same 12.5k users and 2k groups with compat disabled in 30
minutes.
By contrast when I started, with compat enabled, I migrated:
1000 users in 7 minutes
2000 users in 27 minutes
3000 users in 1 hour
rob
From f0b8bf2473d63af81b5dc5bef17499fa5e04aa85 Mon Sep 17 00:00:00 2001
From: Rob Crittenden rcrit...@redhat.com
Date: Tue, 29 Jan 2013 11:19:30 -0500
Subject: [PATCH] Improve migration performance
Add new users to the default users group in batches of 100. The
biggest overhead of migration is in calculating the modlist when
managing the default user's group and applying the changes. A
significant amount of time can be saved by not doing this on every
add operation.
Some other minor improvements include:
Add a negative cache for groups not found in the remote LDAP server.
Replace call to user_mod with a direct LDAP update.
Catch some occurances of LimitError and handle more gracefully.
I also added some debug logging to report on migration status and
performance.
https://fedorahosted.org/freeipa/ticket/3386
---
ipalib/plugins/migration.py | 70 +++--
1 file changed, 62 insertions(+), 8 deletions(-)
diff --git a/ipalib/plugins/migration.py b/ipalib/plugins/migration.py
index 81df59a23ff46b770076cb56a7d78fed8db26029..a254d60e742192d60f19c00a918a83df0326d49e 100644
--- a/ipalib/plugins/migration.py
+++ b/ipalib/plugins/migration.py
@@ -31,6 +31,7 @@ if api.env.in_server and api.env.context in ['lite', 'server']:
raise e
from ipalib import _
from ipapython.dn import DN
+import datetime
__doc__ = _(
Migration to IPA
@@ -107,6 +108,7 @@ EXAMPLES:
# USER MIGRATION CALLBACKS AND VARS
_krb_err_msg = _('Kerberos principal %s already exists. Use \'ipa user-mod\' to set it manually.')
+_krb_failed_msg = _('Unable to determine Kerberos principal %s already exists. Use \'ipa user-mod\' to set it manually.')
_grp_err_msg = _('Failed to add user to the default group. Use \'ipa group-add-member\' to add manually.')
_ref_err_msg = _('Migration of LDAP search reference is not supported.')
_dn_err_msg = _('Malformed DN')
@@ -123,13 +125,17 @@ def _pre_migrate_user(ldap, pkey, dn, entry_attrs, failed, config, ctx, **kwargs
has_upg = ctx['has_upg']
search_bases = kwargs.get('search_bases', None)
valid_gids = kwargs['valid_gids']
+invalid_gids = kwargs['invalid_gids']
if 'gidnumber' not in entry_attrs:
raise errors.NotFound(reason=_('%(user)s is not a POSIX user') % dict(user=pkey))
else:
# See if the gidNumber at least points to a valid group on the remote
# server.
-if entry_attrs['gidnumber'][0] not in valid_gids:
+if entry_attrs['gidnumber'][0] in invalid_gids:
+api.log.warn('GID number %s of migrated user %s does not point to a known group.' \
+ % (entry_attrs['gidnumber'][0], pkey))
+elif entry_attrs['gidnumber'][0] not in valid_gids:
try:
(remote_dn, remote_entry) = ds_ldap.find_entry_by_attr(
'gidnumber', entry_attrs['gidnumber'][0], 'posixgroup',
@@ -139,10 +145,13 @@ def _pre_migrate_user(ldap, pkey, dn, entry_attrs, failed, config, ctx, **kwargs
except errors.NotFound:
api.log.warn('GID number %s of migrated user %s does not point to a known group.' \
% (entry_attrs['gidnumber'][0], pkey))
+invalid_gids.append(entry_attrs['gidnumber'][0])
except errors.SingleMatchExpected, e:
# GID number matched more groups, this should not happen
api.log.warn('GID number %s of migrated user %s should match 1 group, but it matched %d groups' \
% (entry_attrs['gidnumber'][0], pkey, e.found))
+except errors.LimitsExceeded, e:
+api.log.warn('Search limit exceeded searching for GID %s' % entry_attrs['gidnumber'][0])
# We don't want to create a UPG so set the magic value in description
# to let
