Re: [Freeipa-devel] [PATCHES] 143-147 Improve performance with large groups

[Petr Viktorin]

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

[Petr Viktorin]

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

[Martin Kosek]

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

[Petr Vobornik]

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

[Jan Cholasta]

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

[Petr Vobornik]

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.
--
Petr Vobornik
From 0b793660e14324596fe7a36290dfd62f83d176be Mon Sep 17 00:00:00 2001
From: Petr Vobornik 
Date: Thu, 18 Jul 2013 16:17:40 +0200
Subject: [PATCH] Web UI search optimization

This patch optimizes options used in commands executed by search pages.

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. Members are not displayed on search pages and such change drastically improves performance. It reduces computations on server and amount of data transferred to Web UI.
---
 install/ui/src/freeipa/entity.js  | 14 ++
 install/ui/src/freeipa/facet.js   | 11 ++-
 install/ui/src/freeipa/hbac.js|  1 -
 install/ui/src/freeipa/search.js  |  5 +
 install/ui/src/freeipa/selinux.js |  1 -
 5 files changed, 21 insertions(+), 11 deletions(-)

diff --git a/install/ui/src/freeipa/entity.js b/install/ui/src/freeipa/entity.js
index 22efd47afaf5dfd7ae6e1c39a6608b1186de6fb7..eef58d1a40b0d8a32e99f61bdd3e239843c0c0aa 100644
--- a/install/ui/src/freeipa/entity.js
+++ b/install/ui/src/freeipa/entity.js
@@ -217,6 +217,20 @@ exp.entity = IPA.entity = function(spec) {
 return that;
 };
 
+that.has_members = function() {
+var members = that.metadata.attribute_members;
+var has = false;
+if (members) {
+for (var member in members) {
+if (members.hasOwnProperty(member)) {
+has = true;
+break;
+}
+}
+}
+return has;
+};
+
 that.builder = spec.builder || IPA.entity_builder(that);
 
 that.entity_init = that.init;
diff --git a/install/ui/src/freeipa/facet.js b/install/ui/src/freeipa/facet.js
index 166203a93cd539a36993cfed7816ca9ab616116a..37106e22f44b2fb50fc79b8183cc62e9eb35b6e4 100644
--- a/install/ui/src/freeipa/facet.js
+++ b/install/ui/src/freeipa/facet.js
@@ -1082,7 +1082,6 @@ exp.table_facet = IPA.table_facet = function(spec, no_init) {
 
 that.pagination = spec.pagination === undefined ? true : spec.pagination;
 that.search_all_entries = spec.search_all_entries;
-that.search_all_attributes = spec.search_all_attributes;
 that.sort_enabled = spec.sort_enabled === undefined ? true : spec.sort_enabled;
 that.selectable = spec.selectable === undefined ? true : spec.selectable;
 that.select_changed = IPA.observer();
@@ -1312,7 +1311,7 @@ exp.table_facet = IPA.table_facet = function(spec, no_init) {
 
 that.create_get_records_command = function(pkeys, on_success, on_error) {
 
- var batch = IPA.batch_command({
+var batch = IPA.batch_command({
 name: that.get_records_command_name(),
 on_success: on_success,
 on_error: on_error
@@ -1324,10 +1323,13 @@ exp.table_facet = IPA.table_fa

Re: [Freeipa-devel] [PATCHES] 143-147 Improve performance with large groups

[Jan Cholasta]

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

[Martin Kosek]

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

[Jan Cholasta]

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

[Martin Kosek]

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 Jan&Petr 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

[Martin Kosek]

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 Jan&Petr 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

[Jan Cholasta]

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 Jan&Petr 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

[Martin Kosek]

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
>>> .
>>>
>>>
>>>
>>
>> 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 Jan&Petr 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

[Alexander Bokovoy]

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
.




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

[Jan Cholasta]

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
.




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

[Alexander Bokovoy]

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
.



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

[Alexander Bokovoy]

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
.



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

Re: [Freeipa-devel] [PATCHES] 143-147 Improve performance with large groups

[Jan Cholasta]

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
.



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

Re: [Freeipa-devel] [PATCHES] 143-147 Improve performance with large groups

[Jan Cholasta]

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 
.




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

--
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

[Rich Megginson]

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 
.


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

[Martin Kosek]

On 06/27/2013 04:55 PM, Jan Cholasta wrote:
> Hi,
> 
> the attached patches are an attempt to solve
>  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 .
> 
> 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