Re: [Freeipa-devel] [PATCHES] 143-147 Improve performance with large groups
On 07/19/2013 01:10 PM, Petr Vobornik wrote:
On 07/18/2013 05:29 PM, Jan Cholasta wrote:
On 18.7.2013 17:26, Martin Kosek wrote:
On 07/18/2013 05:22 PM, Jan Cholasta wrote:
On 18.7.2013 17:07, Martin Kosek wrote:
On 07/18/2013 04:53 PM, Jan Cholasta wrote:
Added patch which adds new hidden option no_members to suppress
membership
processing for commands of all objects that have member attributes.
This can be
used by the WebUI to prevent member lookups where they are not
necessary (as we
discussed off-line with Martin and Petr).
Honza
1) Should the new option really have exclude='webui'? I thought
that Web UI is
the main and only consumer of this option.
The 'webui' context doesn't actually exist and the only meaning of
this stanza
is that the option is not shown in the output of the show_mappings
command.
2) I would clearly state this is an internal option only, e.g.
+ doc=_('INTERNAL: suppress processing of membership attributes.'),
No other internal option has this kind of thing in its doc and nobody
will see
it anyway, so we might just leave it like that IMHO.
OK.
3) It would be nice to state that this option is mutually exclusive
with --all,
but given it is internal anyway and there is no central place to
define it, we
do not need to do that.
The options are not really mutually exclusive at this point, they can be
specified together, --all takes precedence.
Well, they can be specified together, but the option is then NOOP
which could
confuse users which may have different expectations. Being explicit
helps.
I agree.
But
as I said, in this case this is not a requirement.
I agree as well :-)
Honza
Functional ACK for Honza's patch (didn't break Web UI test suite)
Attaching Web UI patch.
It:
1) Removed --all from _find and _show commands used by search pages. All
displayed attributes should be already included in default attributes.
2) Removed search_all_attributes - Not needed since introduction of paging.
3) Added --no-members options to search _show commmands.
ACK. Pushed both Petr's and Honza's patch to master and ipa-3-2.
Martin
___
Freeipa-devel mailing list
Freeipa-devel@redhat.com
https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCHES] 143-147 Improve performance with large groups
On 07/23/2013 01:30 PM, Martin Kosek wrote:
On 07/19/2013 01:10 PM, Petr Vobornik wrote:
On 07/18/2013 05:29 PM, Jan Cholasta wrote:
On 18.7.2013 17:26, Martin Kosek wrote:
On 07/18/2013 05:22 PM, Jan Cholasta wrote:
On 18.7.2013 17:07, Martin Kosek wrote:
On 07/18/2013 04:53 PM, Jan Cholasta wrote:
Added patch which adds new hidden option no_members to suppress
membership
processing for commands of all objects that have member attributes.
This can be
used by the WebUI to prevent member lookups where they are not
necessary (as we
discussed off-line with Martin and Petr).
Honza
1) Should the new option really have exclude='webui'? I thought
that Web UI is
the main and only consumer of this option.
The 'webui' context doesn't actually exist and the only meaning of
this stanza
is that the option is not shown in the output of the show_mappings
command.
2) I would clearly state this is an internal option only, e.g.
+ doc=_('INTERNAL: suppress processing of membership attributes.'),
No other internal option has this kind of thing in its doc and nobody
will see
it anyway, so we might just leave it like that IMHO.
OK.
3) It would be nice to state that this option is mutually exclusive
with --all,
but given it is internal anyway and there is no central place to
define it, we
do not need to do that.
The options are not really mutually exclusive at this point, they can be
specified together, --all takes precedence.
Well, they can be specified together, but the option is then NOOP
which could
confuse users which may have different expectations. Being explicit
helps.
I agree.
But
as I said, in this case this is not a requirement.
I agree as well :-)
Honza
Functional ACK for Honza's patch (didn't break Web UI test suite)
Attaching Web UI patch.
It:
1) Removed --all from _find and _show commands used by search pages. All
displayed attributes should be already included in default attributes.
2) Removed search_all_attributes - Not needed since introduction of paging.
3) Added --no-members options to search _show commmands.
ACK. Pushed both Petr's and Honza's patch to master and ipa-3-2.
Martin
These patches sometimes add the attribute no_members to groups, which
results in 7 tests being broken like this:
==
FAIL: test_cli.TestCLIParsing.test_group_add
--
Traceback (most recent call last):
File /usr/lib/python2.7/site-packages/nose/case.py, line 197, in
runTest
self.test(*self.arg)
File
/var/lib/jenkins/workspace/freeipa-unittests-f19-devel/ipatests/test_cmdline/test_cli.py,
line 73, in test_group_add
version=API_VERSION)
File
/var/lib/jenkins/workspace/freeipa-unittests-f19-devel/ipatests/test_cmdline/test_cli.py,
line 24, in check_command
util.assert_deepequal(kw_expected, kw_got)
File
/var/lib/jenkins/workspace/freeipa-unittests-f19-devel/ipatests/util.py,
line 338, in assert_deepequal
doc, sorted(missing), sorted(extra), expected, got, stack
AssertionError: assert_deepequal: dict keys mismatch.
missing keys = []
extra keys = ['no_members']
expected = {'all': False, 'cn': u'tgroup1', 'raw': False, 'version':
u'2.62', 'external': False, 'nonposix': False, 'description': u'Test group'}
got = {'all': False, 'description': u'Test group', 'no_members':
False, 'raw': False, 'version': u'2.62', 'external': False, 'nonposix':
False, 'cn': u'tgroup1'}
path = ()
--
Petr³
___
Freeipa-devel mailing list
Freeipa-devel@redhat.com
https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCHES] 143-147 Improve performance with large groups
On 07/23/2013 04:54 PM, Petr Viktorin wrote:
On 07/23/2013 01:30 PM, Martin Kosek wrote:
On 07/19/2013 01:10 PM, Petr Vobornik wrote:
On 07/18/2013 05:29 PM, Jan Cholasta wrote:
On 18.7.2013 17:26, Martin Kosek wrote:
On 07/18/2013 05:22 PM, Jan Cholasta wrote:
On 18.7.2013 17:07, Martin Kosek wrote:
On 07/18/2013 04:53 PM, Jan Cholasta wrote:
Added patch which adds new hidden option no_members to suppress
membership
processing for commands of all objects that have member attributes.
This can be
used by the WebUI to prevent member lookups where they are not
necessary (as we
discussed off-line with Martin and Petr).
Honza
1) Should the new option really have exclude='webui'? I thought
that Web UI is
the main and only consumer of this option.
The 'webui' context doesn't actually exist and the only meaning of
this stanza
is that the option is not shown in the output of the show_mappings
command.
2) I would clearly state this is an internal option only, e.g.
+ doc=_('INTERNAL: suppress processing of membership attributes.'),
No other internal option has this kind of thing in its doc and nobody
will see
it anyway, so we might just leave it like that IMHO.
OK.
3) It would be nice to state that this option is mutually exclusive
with --all,
but given it is internal anyway and there is no central place to
define it, we
do not need to do that.
The options are not really mutually exclusive at this point, they
can be
specified together, --all takes precedence.
Well, they can be specified together, but the option is then NOOP
which could
confuse users which may have different expectations. Being explicit
helps.
I agree.
But
as I said, in this case this is not a requirement.
I agree as well :-)
Honza
Functional ACK for Honza's patch (didn't break Web UI test suite)
Attaching Web UI patch.
It:
1) Removed --all from _find and _show commands used by search pages. All
displayed attributes should be already included in default attributes.
2) Removed search_all_attributes - Not needed since introduction of
paging.
3) Added --no-members options to search _show commmands.
ACK. Pushed both Petr's and Honza's patch to master and ipa-3-2.
Martin
These patches sometimes add the attribute no_members to groups, which
results in 7 tests being broken like this:
==
FAIL: test_cli.TestCLIParsing.test_group_add
--
Traceback (most recent call last):
File /usr/lib/python2.7/site-packages/nose/case.py, line 197, in
runTest
self.test(*self.arg)
File
/var/lib/jenkins/workspace/freeipa-unittests-f19-devel/ipatests/test_cmdline/test_cli.py,
line 73, in test_group_add
version=API_VERSION)
File
/var/lib/jenkins/workspace/freeipa-unittests-f19-devel/ipatests/test_cmdline/test_cli.py,
line 24, in check_command
util.assert_deepequal(kw_expected, kw_got)
File
/var/lib/jenkins/workspace/freeipa-unittests-f19-devel/ipatests/util.py,
line 338, in assert_deepequal
doc, sorted(missing), sorted(extra), expected, got, stack
AssertionError: assert_deepequal: dict keys mismatch.
missing keys = []
extra keys = ['no_members']
expected = {'all': False, 'cn': u'tgroup1', 'raw': False, 'version':
u'2.62', 'external': False, 'nonposix': False, 'description': u'Test
group'}
got = {'all': False, 'description': u'Test group', 'no_members':
False, 'raw': False, 'version': u'2.62', 'external': False, 'nonposix':
False, 'cn': u'tgroup1'}
path = ()
Correction: they don't add the attribute to output. It's just that the
CLI tests need to be updated with the new option.
--
Petr³
___
Freeipa-devel mailing list
Freeipa-devel@redhat.com
https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCHES] 143-147 Improve performance with large groups
On 19.7.2013 13:10, Petr Vobornik wrote:
On 07/18/2013 05:29 PM, Jan Cholasta wrote:
On 18.7.2013 17:26, Martin Kosek wrote:
On 07/18/2013 05:22 PM, Jan Cholasta wrote:
On 18.7.2013 17:07, Martin Kosek wrote:
On 07/18/2013 04:53 PM, Jan Cholasta wrote:
Added patch which adds new hidden option no_members to suppress
membership
processing for commands of all objects that have member attributes.
This can be
used by the WebUI to prevent member lookups where they are not
necessary (as we
discussed off-line with Martin and Petr).
Honza
1) Should the new option really have exclude='webui'? I thought
that Web UI is
the main and only consumer of this option.
The 'webui' context doesn't actually exist and the only meaning of
this stanza
is that the option is not shown in the output of the show_mappings
command.
2) I would clearly state this is an internal option only, e.g.
+ doc=_('INTERNAL: suppress processing of membership attributes.'),
No other internal option has this kind of thing in its doc and nobody
will see
it anyway, so we might just leave it like that IMHO.
OK.
3) It would be nice to state that this option is mutually exclusive
with --all,
but given it is internal anyway and there is no central place to
define it, we
do not need to do that.
The options are not really mutually exclusive at this point, they
can be
specified together, --all takes precedence.
Well, they can be specified together, but the option is then NOOP
which could
confuse users which may have different expectations. Being explicit
helps.
I agree.
But
as I said, in this case this is not a requirement.
I agree as well :-)
Honza
Functional ACK for Honza's patch (didn't break Web UI test suite)
Attaching Web UI patch.
It:
1) Removed --all from _find and _show commands used by search pages. All
displayed attributes should be already included in default attributes.
2) Removed search_all_attributes - Not needed since introduction of paging.
3) Added --no-members options to search _show commmands.
Hmm, is the has_members function used to distinguish between objects
with members and without members? If so, would it be helpful to add the
no_members option to all commands (just like all and raw), so that
you don't have to do this check?
Honza
--
Jan Cholasta
___
Freeipa-devel mailing list
Freeipa-devel@redhat.com
https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCHES] 143-147 Improve performance with large groups
On 07/19/2013 03:49 PM, Jan Cholasta wrote:
On 19.7.2013 13:10, Petr Vobornik wrote:
On 07/18/2013 05:29 PM, Jan Cholasta wrote:
On 18.7.2013 17:26, Martin Kosek wrote:
On 07/18/2013 05:22 PM, Jan Cholasta wrote:
On 18.7.2013 17:07, Martin Kosek wrote:
On 07/18/2013 04:53 PM, Jan Cholasta wrote:
Added patch which adds new hidden option no_members to suppress
membership
processing for commands of all objects that have member attributes.
This can be
used by the WebUI to prevent member lookups where they are not
necessary (as we
discussed off-line with Martin and Petr).
Honza
1) Should the new option really have exclude='webui'? I thought
that Web UI is
the main and only consumer of this option.
The 'webui' context doesn't actually exist and the only meaning of
this stanza
is that the option is not shown in the output of the show_mappings
command.
2) I would clearly state this is an internal option only, e.g.
+ doc=_('INTERNAL: suppress processing of membership attributes.'),
No other internal option has this kind of thing in its doc and nobody
will see
it anyway, so we might just leave it like that IMHO.
OK.
3) It would be nice to state that this option is mutually exclusive
with --all,
but given it is internal anyway and there is no central place to
define it, we
do not need to do that.
The options are not really mutually exclusive at this point, they
can be
specified together, --all takes precedence.
Well, they can be specified together, but the option is then NOOP
which could
confuse users which may have different expectations. Being explicit
helps.
I agree.
But
as I said, in this case this is not a requirement.
I agree as well :-)
Honza
Functional ACK for Honza's patch (didn't break Web UI test suite)
Attaching Web UI patch.
It:
1) Removed --all from _find and _show commands used by search pages. All
displayed attributes should be already included in default attributes.
2) Removed search_all_attributes - Not needed since introduction of
paging.
3) Added --no-members options to search _show commmands.
Hmm, is the has_members function used to distinguish between objects
with members and without members? If so, would it be helpful to add the
no_members option to all commands (just like all and raw), so that
you don't have to do this check?
Honza
Yes, that's its purpose. IMO it doesn't matter now, the function is
implemented. The only benefits would be small, not noticeable,
performance gain and tolerance to incorrect use of --no-members. Doesn't
look like a reason for polluting all commands with it.
--
Petr Vobornik
___
Freeipa-devel mailing list
Freeipa-devel@redhat.com
https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCHES] 143-147 Improve performance with large groups
On 07/18/2013 04:53 PM, Jan Cholasta wrote:
On 15.7.2013 17:21, Martin Kosek wrote:
On 07/15/2013 03:43 PM, Jan Cholasta wrote:
On 15.7.2013 15:16, Martin Kosek wrote:
On 07/11/2013 12:15 PM, Alexander Bokovoy wrote:
On Thu, 11 Jul 2013, Jan Cholasta wrote:
We can add WebUI improvements later. I have some WIP, but I need to
discuss
it with Petr first (he's away this week).
Ok.
The patchset is in ipa-3-2 as well now.
Just checking:
1) Jan, did you check size of these new indexes on IPA master with such a
high
number of users? How big are they? I want to make sure that this won't
create
an issue on upgrades to new 3.2.x.
With 10k users, the indices ate roughly 250 MB of disk space.
Scratch that, my measurement method was stupid. It is just 2 MB of extra
space.
2) Does the patch set also fix the problem for Web UI? Currently, I think
it
will still grab and process all member attributes even though it does not
need
it. If the Web UI performance is still not sharp, I would rather leave this
ticket opened and let JanPetr cooperate on the Web UI part.
I agree on keeping the ticket open.
Honza
Ok, I reopened the ticket. Petr and Jan, please cooperate on this one.
Martin
Added patch which adds new hidden option no_members to suppress membership
processing for commands of all objects that have member attributes. This can
be
used by the WebUI to prevent member lookups where they are not necessary (as
we
discussed off-line with Martin and Petr).
Honza
1) Should the new option really have exclude='webui'? I thought that Web UI is
the main and only consumer of this option.
2) I would clearly state this is an internal option only, e.g.
+ doc=_('INTERNAL: suppress processing of membership attributes.'),
3) It would be nice to state that this option is mutually exclusive with --all,
but given it is internal anyway and there is no central place to define it, we
do not need to do that.
Martin
___
Freeipa-devel mailing list
Freeipa-devel@redhat.com
https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCHES] 143-147 Improve performance with large groups
On 18.7.2013 17:07, Martin Kosek wrote:
On 07/18/2013 04:53 PM, Jan Cholasta wrote:
Added patch which adds new hidden option no_members to suppress membership
processing for commands of all objects that have member attributes. This can be
used by the WebUI to prevent member lookups where they are not necessary (as we
discussed off-line with Martin and Petr).
Honza
1) Should the new option really have exclude='webui'? I thought that Web UI is
the main and only consumer of this option.
The 'webui' context doesn't actually exist and the only meaning of this
stanza is that the option is not shown in the output of the
show_mappings command.
2) I would clearly state this is an internal option only, e.g.
+ doc=_('INTERNAL: suppress processing of membership attributes.'),
No other internal option has this kind of thing in its doc and nobody
will see it anyway, so we might just leave it like that IMHO.
3) It would be nice to state that this option is mutually exclusive with --all,
but given it is internal anyway and there is no central place to define it, we
do not need to do that.
The options are not really mutually exclusive at this point, they can be
specified together, --all takes precedence.
Honza
--
Jan Cholasta
___
Freeipa-devel mailing list
Freeipa-devel@redhat.com
https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCHES] 143-147 Improve performance with large groups
On 07/18/2013 05:22 PM, Jan Cholasta wrote:
On 18.7.2013 17:07, Martin Kosek wrote:
On 07/18/2013 04:53 PM, Jan Cholasta wrote:
Added patch which adds new hidden option no_members to suppress membership
processing for commands of all objects that have member attributes. This
can be
used by the WebUI to prevent member lookups where they are not necessary
(as we
discussed off-line with Martin and Petr).
Honza
1) Should the new option really have exclude='webui'? I thought that Web UI
is
the main and only consumer of this option.
The 'webui' context doesn't actually exist and the only meaning of this stanza
is that the option is not shown in the output of the show_mappings command.
2) I would clearly state this is an internal option only, e.g.
+ doc=_('INTERNAL: suppress processing of membership attributes.'),
No other internal option has this kind of thing in its doc and nobody will see
it anyway, so we might just leave it like that IMHO.
OK.
3) It would be nice to state that this option is mutually exclusive with
--all,
but given it is internal anyway and there is no central place to define it,
we
do not need to do that.
The options are not really mutually exclusive at this point, they can be
specified together, --all takes precedence.
Well, they can be specified together, but the option is then NOOP which could
confuse users which may have different expectations. Being explicit helps. But
as I said, in this case this is not a requirement.
Martin
___
Freeipa-devel mailing list
Freeipa-devel@redhat.com
https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCHES] 143-147 Improve performance with large groups
On 18.7.2013 17:26, Martin Kosek wrote:
On 07/18/2013 05:22 PM, Jan Cholasta wrote:
On 18.7.2013 17:07, Martin Kosek wrote:
On 07/18/2013 04:53 PM, Jan Cholasta wrote:
Added patch which adds new hidden option no_members to suppress membership
processing for commands of all objects that have member attributes. This can be
used by the WebUI to prevent member lookups where they are not necessary (as we
discussed off-line with Martin and Petr).
Honza
1) Should the new option really have exclude='webui'? I thought that Web UI is
the main and only consumer of this option.
The 'webui' context doesn't actually exist and the only meaning of this stanza
is that the option is not shown in the output of the show_mappings command.
2) I would clearly state this is an internal option only, e.g.
+ doc=_('INTERNAL: suppress processing of membership attributes.'),
No other internal option has this kind of thing in its doc and nobody will see
it anyway, so we might just leave it like that IMHO.
OK.
3) It would be nice to state that this option is mutually exclusive with --all,
but given it is internal anyway and there is no central place to define it, we
do not need to do that.
The options are not really mutually exclusive at this point, they can be
specified together, --all takes precedence.
Well, they can be specified together, but the option is then NOOP which could
confuse users which may have different expectations. Being explicit helps.
I agree.
But
as I said, in this case this is not a requirement.
I agree as well :-)
Honza
--
Jan Cholasta
___
Freeipa-devel mailing list
Freeipa-devel@redhat.com
https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCHES] 143-147 Improve performance with large groups
On 07/11/2013 12:15 PM, Alexander Bokovoy wrote: On Thu, 11 Jul 2013, Jan Cholasta wrote: On 11.7.2013 11:58, Alexander Bokovoy wrote: On Mon, 08 Jul 2013, Alexander Bokovoy wrote: On Thu, 27 Jun 2013, Jan Cholasta wrote: On 27.6.2013 17:34, Rich Megginson wrote: On 06/27/2013 09:31 AM, Jan Cholasta wrote: The search is hard-coded in the referint plugin, see https://git.fedorahosted.org/cgit/389/ds.git/tree/ldap/servers/plugins/referint/referint.c#n745. Not sure if it makes sense to do a wildcard/substr search here - please file a ticket with 389 to investigate. https://fedorahosted.org/389/ticket/47411 So, should we merge this patchset or wait until 389-ds analyzes 47411? To me it looks like we can use this one as an interim solution, once Web UI performance is checked through. I've commited the patchset to master. Web UI works just fine for me and with a VM limited to 1GB RAM I seem to get snappier response even when running whole IPA stack and Firefox in the same VM. We can add WebUI improvements later. I have some WIP, but I need to discuss it with Petr first (he's away this week). Ok. The patchset is in ipa-3-2 as well now. Just checking: 1) Jan, did you check size of these new indexes on IPA master with such a high number of users? How big are they? I want to make sure that this won't create an issue on upgrades to new 3.2.x. 2) Does the patch set also fix the problem for Web UI? Currently, I think it will still grab and process all member attributes even though it does not need it. If the Web UI performance is still not sharp, I would rather leave this ticket opened and let JanPetr cooperate on the Web UI part. Martin ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCHES] 143-147 Improve performance with large groups
On 15.7.2013 15:16, Martin Kosek wrote: On 07/11/2013 12:15 PM, Alexander Bokovoy wrote: On Thu, 11 Jul 2013, Jan Cholasta wrote: We can add WebUI improvements later. I have some WIP, but I need to discuss it with Petr first (he's away this week). Ok. The patchset is in ipa-3-2 as well now. Just checking: 1) Jan, did you check size of these new indexes on IPA master with such a high number of users? How big are they? I want to make sure that this won't create an issue on upgrades to new 3.2.x. With 10k users, the indices ate roughly 250 MB of disk space. 2) Does the patch set also fix the problem for Web UI? Currently, I think it will still grab and process all member attributes even though it does not need it. If the Web UI performance is still not sharp, I would rather leave this ticket opened and let JanPetr cooperate on the Web UI part. I agree on keeping the ticket open. Honza -- Jan Cholasta ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCHES] 143-147 Improve performance with large groups
On 07/15/2013 03:43 PM, Jan Cholasta wrote: On 15.7.2013 15:16, Martin Kosek wrote: On 07/11/2013 12:15 PM, Alexander Bokovoy wrote: On Thu, 11 Jul 2013, Jan Cholasta wrote: We can add WebUI improvements later. I have some WIP, but I need to discuss it with Petr first (he's away this week). Ok. The patchset is in ipa-3-2 as well now. Just checking: 1) Jan, did you check size of these new indexes on IPA master with such a high number of users? How big are they? I want to make sure that this won't create an issue on upgrades to new 3.2.x. With 10k users, the indices ate roughly 250 MB of disk space. 2) Does the patch set also fix the problem for Web UI? Currently, I think it will still grab and process all member attributes even though it does not need it. If the Web UI performance is still not sharp, I would rather leave this ticket opened and let JanPetr cooperate on the Web UI part. I agree on keeping the ticket open. Honza Ok, I reopened the ticket. Petr and Jan, please cooperate on this one. Martin ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCHES] 143-147 Improve performance with large groups
On Mon, 08 Jul 2013, Alexander Bokovoy wrote: On Thu, 27 Jun 2013, Jan Cholasta wrote: On 27.6.2013 17:34, Rich Megginson wrote: On 06/27/2013 09:31 AM, Jan Cholasta wrote: The search is hard-coded in the referint plugin, see https://git.fedorahosted.org/cgit/389/ds.git/tree/ldap/servers/plugins/referint/referint.c#n745. Not sure if it makes sense to do a wildcard/substr search here - please file a ticket with 389 to investigate. https://fedorahosted.org/389/ticket/47411 So, should we merge this patchset or wait until 389-ds analyzes 47411? To me it looks like we can use this one as an interim solution, once Web UI performance is checked through. I've commited the patchset to master. Web UI works just fine for me and with a VM limited to 1GB RAM I seem to get snappier response even when running whole IPA stack and Firefox in the same VM. -- / Alexander Bokovoy ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCHES] 143-147 Improve performance with large groups
On 11.7.2013 11:58, Alexander Bokovoy wrote: On Mon, 08 Jul 2013, Alexander Bokovoy wrote: On Thu, 27 Jun 2013, Jan Cholasta wrote: On 27.6.2013 17:34, Rich Megginson wrote: On 06/27/2013 09:31 AM, Jan Cholasta wrote: The search is hard-coded in the referint plugin, see https://git.fedorahosted.org/cgit/389/ds.git/tree/ldap/servers/plugins/referint/referint.c#n745. Not sure if it makes sense to do a wildcard/substr search here - please file a ticket with 389 to investigate. https://fedorahosted.org/389/ticket/47411 So, should we merge this patchset or wait until 389-ds analyzes 47411? To me it looks like we can use this one as an interim solution, once Web UI performance is checked through. I've commited the patchset to master. Web UI works just fine for me and with a VM limited to 1GB RAM I seem to get snappier response even when running whole IPA stack and Firefox in the same VM. We can add WebUI improvements later. I have some WIP, but I need to discuss it with Petr first (he's away this week). Honza -- Jan Cholasta ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCHES] 143-147 Improve performance with large groups
On Thu, 11 Jul 2013, Jan Cholasta wrote: On 11.7.2013 11:58, Alexander Bokovoy wrote: On Mon, 08 Jul 2013, Alexander Bokovoy wrote: On Thu, 27 Jun 2013, Jan Cholasta wrote: On 27.6.2013 17:34, Rich Megginson wrote: On 06/27/2013 09:31 AM, Jan Cholasta wrote: The search is hard-coded in the referint plugin, see https://git.fedorahosted.org/cgit/389/ds.git/tree/ldap/servers/plugins/referint/referint.c#n745. Not sure if it makes sense to do a wildcard/substr search here - please file a ticket with 389 to investigate. https://fedorahosted.org/389/ticket/47411 So, should we merge this patchset or wait until 389-ds analyzes 47411? To me it looks like we can use this one as an interim solution, once Web UI performance is checked through. I've commited the patchset to master. Web UI works just fine for me and with a VM limited to 1GB RAM I seem to get snappier response even when running whole IPA stack and Firefox in the same VM. We can add WebUI improvements later. I have some WIP, but I need to discuss it with Petr first (he's away this week). Ok. The patchset is in ipa-3-2 as well now. -- / Alexander Bokovoy ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCHES] 143-147 Improve performance with large groups
On Thu, 27 Jun 2013, Jan Cholasta wrote: On 27.6.2013 17:34, Rich Megginson wrote: On 06/27/2013 09:31 AM, Jan Cholasta wrote: The search is hard-coded in the referint plugin, see https://git.fedorahosted.org/cgit/389/ds.git/tree/ldap/servers/plugins/referint/referint.c#n745. Not sure if it makes sense to do a wildcard/substr search here - please file a ticket with 389 to investigate. https://fedorahosted.org/389/ticket/47411 So, should we merge this patchset or wait until 389-ds analyzes 47411? To me it looks like we can use this one as an interim solution, once Web UI performance is checked through. -- / Alexander Bokovoy ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
[Freeipa-devel] [PATCHES] 143-147 Improve performance with large groups
Hi,
the attached patches are an attempt to solve
https://fedorahosted.org/freeipa/ticket/3706 without actually removing
ipausers.
I have done some basic timing on IPA with 10k users, the results are:
ipa user-add: 18 s originally, 4 s with the patches
ipa user-del: 54 s originally, 7 s with the patches
Other commands should be affected as well, especially del commands
(deleting an entry triggers a originally unindexed search in the
referint plugin) and member manipulation commands (full member list is
no longer fetched and stored back when adding/removing members).
Patch 147 fixes https://fedorahosted.org/freeipa/ticket/3743.
Honza
--
Jan Cholasta
From ddca9fbf73e985fb8a6e5ea43b0e2e68c957377b Mon Sep 17 00:00:00 2001
From: Jan Cholasta jchol...@redhat.com
Date: Tue, 25 Jun 2013 12:58:37 +
Subject: [PATCH 1/5] Use LDAP search instead of *group_show to check if a
group exists.
https://fedorahosted.org/freeipa/ticket/3706
---
ipalib/plugins/aci.py | 9 +
ipalib/plugins/baseldap.py | 5 +
ipalib/plugins/config.py| 2 +-
ipalib/plugins/hostgroup.py | 4 ++--
ipalib/plugins/netgroup.py | 2 +-
ipalib/plugins/user.py | 2 +-
6 files changed, 15 insertions(+), 9 deletions(-)
diff --git a/ipalib/plugins/aci.py b/ipalib/plugins/aci.py
index dab209e..a7f85dd 100644
--- a/ipalib/plugins/aci.py
+++ b/ipalib/plugins/aci.py
@@ -252,7 +252,8 @@ def _make_aci(ldap, current, aciname, kw):
elif group:
# Not so friendly with groups. This will raise
try:
-entry_attrs = api.Command['group_show'](kw['group'])['result']
+group_dn = api.Object['group'].get_dn_if_exists(kw['group'])
+entry_attrs = {'dn': group_dn}
except errors.NotFound:
raise errors.NotFound(reason=_(Group '%s' does not exist) % kw['group'])
@@ -269,7 +270,7 @@ def _make_aci(ldap, current, aciname, kw):
a.set_target_attr(kw['attrs'])
if valid['memberof']:
try:
-api.Command['group_show'](kw['memberof'])
+api.Object['group'].get_dn_if_exists(kw['memberof'])
except errors.NotFound:
api.Object['group'].handle_not_found(kw['memberof'])
groupdn = _group_from_memberof(kw['memberof'])
@@ -291,8 +292,8 @@ def _make_aci(ldap, current, aciname, kw):
a.set_target(target)
if valid['targetgroup']:
# Purposely no try here so we'll raise a NotFound
-entry_attrs = api.Command['group_show'](kw['targetgroup'])['result']
-target = 'ldap:///%s' % entry_attrs['dn']
+group_dn = api.Object['group'].get_dn_if_exists(kw['targetgroup'])
+target = 'ldap:///%s' % group_dn
a.set_target(target)
if valid['subtree']:
# See if the subtree is a full URI
diff --git a/ipalib/plugins/baseldap.py b/ipalib/plugins/baseldap.py
index bb0de98..1312107 100644
--- a/ipalib/plugins/baseldap.py
+++ b/ipalib/plugins/baseldap.py
@@ -493,6 +493,11 @@ class LDAPObject(Object):
assert isinstance(parent_dn, DN)
return parent_dn
+def get_dn_if_exists(self, *keys, **kwargs):
+dn = self.get_dn(*keys, **kwargs)
+entry = self.backend.get_entry(dn, [''])
+return entry.dn
+
def get_primary_key_from_dn(self, dn):
assert isinstance(dn, DN)
try:
diff --git a/ipalib/plugins/config.py b/ipalib/plugins/config.py
index 33eb174..b9cf050 100644
--- a/ipalib/plugins/config.py
+++ b/ipalib/plugins/config.py
@@ -213,7 +213,7 @@ class config_mod(LDAPUpdate):
if 'ipadefaultprimarygroup' in entry_attrs:
group=entry_attrs['ipadefaultprimarygroup']
try:
-api.Command['group_show'](group)
+api.Object['group'].get_dn_if_exists(group)
except errors.NotFound:
raise errors.NotFound(message=_(The group doesn't exist))
kw = {}
diff --git a/ipalib/plugins/hostgroup.py b/ipalib/plugins/hostgroup.py
index 9fb1029..bc10994 100644
--- a/ipalib/plugins/hostgroup.py
+++ b/ipalib/plugins/hostgroup.py
@@ -122,7 +122,7 @@ class hostgroup_add(LDAPCreate):
assert isinstance(dn, DN)
try:
# check duplicity with hostgroups first to provide proper error
-netgroup = api.Command['hostgroup_show'](keys[-1])
+api.Object['hostgroup'].get_dn_if_exists(keys[-1])
self.obj.handle_duplicate_entry(*keys)
except errors.NotFound:
pass
@@ -130,7 +130,7 @@ class hostgroup_add(LDAPCreate):
try:
# when enabled, a managed netgroup is created for every hostgroup
# make sure that the netgroup can be created
-netgroup = api.Command['netgroup_show'](keys[-1])
+api.Object['netgroup'].get_dn_if_exists(keys[-1])
raise errors.DuplicateEntry(message=unicode(_(\
u'netgroup
Re: [Freeipa-devel] [PATCHES] 143-147 Improve performance with large groups
On 06/27/2013 04:55 PM, Jan Cholasta wrote: Hi, the attached patches are an attempt to solve https://fedorahosted.org/freeipa/ticket/3706 without actually removing ipausers. I have done some basic timing on IPA with 10k users, the results are: ipa user-add: 18 s originally, 4 s with the patches ipa user-del: 54 s originally, 7 s with the patches Other commands should be affected as well, especially del commands (deleting an entry triggers a originally unindexed search in the referint plugin) and member manipulation commands (full member list is no longer fetched and stored back when adding/removing members). Patch 147 fixes https://fedorahosted.org/freeipa/ticket/3743. Honza Thanks for this effort! I quickly went through the patches, they mostly look harmless. Except the following: Subject: [PATCH 4/5] Add missing substring indices for attributes managed by the referint plugin. AFAIK, sub index is a very expensive index - as we discussed offline - adding Rich to advise and confirm this. I think you added it because some plugin was doing substring/wildcard search when an LDAP entry was being deleted - did you identify which one it is? Because I would rather get rid of the bad search than adding so many sub indices. Secondly, did you also check Web UI performance? I think we could noticeable improve user/group lists performance if we added a new (hidden) option to suppress loading membership information which could then be utilized by Web UI. Adding Petr Vobornik to CC to consider this. Martin ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCHES] 143-147 Improve performance with large groups
On 06/27/2013 09:31 AM, Jan Cholasta wrote: On 27.6.2013 17:23, Martin Kosek wrote: Thanks for this effort! I quickly went through the patches, they mostly look harmless. Except the following: Subject: [PATCH 4/5] Add missing substring indices for attributes managed by the referint plugin. AFAIK, sub index is a very expensive index - as we discussed offline - adding Rich to advise and confirm this. I think you added it because some plugin was doing substring/wildcard search when an LDAP entry was being deleted - did you identify which one it is? Because I would rather get rid of the bad search than adding so many sub indices. The search is hard-coded in the referint plugin, see https://git.fedorahosted.org/cgit/389/ds.git/tree/ldap/servers/plugins/referint/referint.c#n745. Not sure if it makes sense to do a wildcard/substr search here - please file a ticket with 389 to investigate. sub index isn't necessarily a bad thing - in this case it may be more beneficial than harmful - if you have enough nsslapd-idlistscanlimit to hold the entire candidate list in a single id list without hurting performance (i.e. a list of 1 entries is probably ok - a list of 100 entries is not) Secondly, did you also check Web UI performance? I think we could noticeable improve user/group lists performance if we added a new (hidden) option to suppress loading membership information which could then be utilized by Web UI. Adding Petr Vobornik to CC to consider this. No, not yet. Honza ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCHES] 143-147 Improve performance with large groups
On 27.6.2013 17:34, Rich Megginson wrote: On 06/27/2013 09:31 AM, Jan Cholasta wrote: The search is hard-coded in the referint plugin, see https://git.fedorahosted.org/cgit/389/ds.git/tree/ldap/servers/plugins/referint/referint.c#n745. Not sure if it makes sense to do a wildcard/substr search here - please file a ticket with 389 to investigate. https://fedorahosted.org/389/ticket/47411 -- Jan Cholasta ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
