Bug 43019 - Start of exam mode is extremely slow
Start of exam mode is extremely slow
Status: CLOSED FIXED
Product: UCS@school
Classification: Unclassified
Component: UMC - Exam mode
UCS@school 4.2
Other Linux
: P5 normal (vote)
: UCS@school 4.2 v2
Assigned To: Daniel Tröder
Florian Best
:
Depends on:
Blocks: 44157 44629
  Show dependency treegraph
 
Reported: 2016-11-21 15:15 CET by Jan Christoph Ebersbach
Modified: 2020-10-02 10:12 CEST (History)
3 users (show)

See Also:
What kind of report is it?: Bug Report
What type of bug is this?: 5: Major Usability: Impairs usability in key scenarios
Who will be affected by this bug?: 3: Will affect average number of installed domains
How will those affected feel about the bug?: 3: A User would likely not purchase the product
User Pain: 0.257
Enterprise Customer affected?:
School Customer affected?: Yes
ISV affected?:
Waiting Support:
Flags outvoted (downgraded) after PO Review:
Ticket number:
Bug group (optional):
Max CVSS v3 score:


Attachments
squashed diff (8.42 KB, patch)
2017-05-17 17:28 CEST, Florian Best
Details | Diff
patch (1.38 KB, patch)
2017-05-24 18:57 CEST, Florian Best
Details | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Jan Christoph Ebersbach univentionstaff 2016-11-21 15:15:05 CET
A customer reported that starting exam mode takes very long.  Especially the preparation of the room take a long time when done as a teacher.  However, preparation happens quickly when the Administrator starts exam mode.
Comment 1 Florian Best univentionstaff 2016-11-21 15:30:12 CET
We should analyze what is actually taking so long.
Maybe it is already fixed by fixing Bug #42997.
Comment 2 Florian Best univentionstaff 2016-11-21 16:36:50 CET
Another idea to improve performance I had was:
1. When cloning the users don't keep their group memberships.
2. After all users objects were created add them all once to the necessary groups.
This would not trigger the listener script for every object multiple times.
Comment 3 Sönke Schwardt-Krummrich univentionstaff 2016-11-22 10:45:07 CET
(In reply to Florian Best from comment #2)
> Another idea to improve performance I had was:
> 1. When cloning the users don't keep their group memberships.

Yes and no. If the access is restricted to the home share, we can drop the group memberships. If this option is not selected, the exam user should have access to the same shares the originally user has.

> 2. After all users objects were created add them all once to the necessary
> groups.
> This would not trigger the listener script for every object multiple times.

ACK

Raising the priority since the start of exam mode will fail if it takes longer than 5min. The customer reported a start time of 2.5-3 min.
Comment 4 Florian Best univentionstaff 2017-05-03 15:46:02 CEST
ucs-school-umc-exam (7.0.3-9):
r79049 | Bug #43019: add all exam users to their groups in one go
I see a security issue in this commit:
Now the DC Slave can put various users into various groups with a simple UMCP call. e.g. put user "foo" into group "Domain Admins". The changes are done with cn=admin, which has write access to everything. This was not so easily possible prior.
Comment 5 Florian Best univentionstaff 2017-05-03 15:48:28 CEST
ucs-school-umc-exam (7.0.3-9):
r79052 | Bug #43019: add user only to primary group if share access is restricted to home share

In this commit you are using Sanitizer(required=True). This will not work because you might have UCS@school environments with older UCS@school versions.
Also you have to keep in mind that everything which is added now needs to be supported forever. So this needs to be well designed.
Comment 6 Daniel Tröder univentionstaff 2017-05-03 17:32:57 CEST
r79049: add all exam users to their groups in one go
r79052: add user only to primary group if share access is restricted to home share
r79053: advisory
r79058: exam users can only gain groups the original users already (addresses comment4 and comment5)
r79059: success argument to Base.finished is deprecated
r79060: advisory update

Please be aware that if the exam was started with share access restricted to home (the default), and then the access mode is changed during the exam in the computer room module, the students will still not be able to access other shares.

I don't think that matters, as they'd have to logout and login anyway for such a change to take effect, and this will (hopefully) not happen during an exam.

Package: ucs-school-umc-exam
Version: 7.0.3-10A~4.2.0.201705031730
Branch: ucs_4.2-0
Scope: ucs-school-4.2
Comment 7 Sönke Schwardt-Krummrich univentionstaff 2017-05-08 10:02:01 CEST
(In reply to Daniel Tröder from comment #6)
> r79052: add user only to primary group if share access is restricted to home
> share

Hmmm... this might cause problems because the windows clients may also rely on these group memberships (e.g. for lokal files/directories/...). And also other services may use LDAP groups which would be directly affected.

How much time is spent in group handling? 

I would rather make the netlogon script generation asynchronous and more efficient, so user and group handling in the listener would not be slowed down.

> Please be aware that if the exam was started with share access restricted to
> home (the default), and then the access mode is changed during the exam in
> the computer room module, the students will still not be able to access
> other shares.

a) not ideal b) see above
 
> I don't think that matters, as they'd have to logout and login anyway for
> such a change to take effect, and this will (hopefully) not happen during an
> exam.

No. If the samba process for the affected windows client is stopped (computerroom module should kill the process upon share mode change), the new share mode comes immediately into effect.
Comment 8 Daniel Tröder univentionstaff 2017-05-08 18:54:35 CEST
r79212: simplify adding exam users to groups
r79213: add exam students to all groups regardless of share restrictions
r79214: update advisory

Package: ucs-school-umc-exam
Version: 7.0.4-3A~4.2.0.201705081850
Branch: ucs_4.2-0
Scope: ucs-school-4.2

As all commit and builds were done in the 4.2 scope, I changed the TM. If this should be backported to 4.1 R2, please clone this bug.
Comment 9 Florian Best univentionstaff 2017-05-17 17:28:42 CEST
Created attachment 8858 [details]
squashed diff
Comment 10 Florian Best univentionstaff 2017-05-17 17:31:08 CEST
(In reply to Florian Best from comment #9)
> Created attachment 8858 [details]
> squashed diff
I did a squashed diff of your changes. This looks fine so far.
Except for the unnecessary return of the uid:
+»  »   »   examuseruid=exam_user_uid,

REOPEN: Please answer to comment #7.
Comment 11 Daniel Tröder univentionstaff 2017-05-18 09:36:16 CEST
(In reply to Florian Best from comment #10)
> (In reply to Florian Best from comment #9)
> > Created attachment 8858 [details]
> > squashed diff
> I did a squashed diff of your changes. This looks fine so far.
> Except for the unnecessary return of the uid:
> +»  »   »   examuseruid=exam_user_uid,
Ah yes... some leftover - removed it in r79411 (+r79412).

> REOPEN: Please answer to comment #7.

(In reply to Sönke Schwardt-Krummrich from comment #7)
> (In reply to Daniel Tröder from comment #6)
> > r79052: add user only to primary group if share access is restricted to home
> > share
> 
> Hmmm... this might cause problems because the windows clients may also rely
> on these group memberships (e.g. for lokal files/directories/...). And also
> other services may use LDAP groups which would be directly affected.
Was just implementing Sönkes idea from comment3, but it has already been removed:
r79213: add exam students to all groups regardless of share restrictions

> How much time is spent in group handling? 
Depends on show much load the LDAP server has at that moment and how much replication is going on. Especially at the start of an exam more LDAP and replication load is probably not appreciated.

> I would rather make the netlogon script generation asynchronous and more
> efficient, so user and group handling in the listener would not be slowed
> down.
That is an additional and separate problem to tackle. I know there exist already ideas for that, so I created a new bug to address it separately: Bug #44629.

In the case of the exam mode, asynchronous logon script generation may be undesirable.
Comment 12 Florian Best univentionstaff 2017-05-24 18:56:57 CEST
OK: exam users are still part of the groups the original user belongs to after creation
OK: I could not find any listener which has problems with afterwards adding groups
OK: the performance benefit average is maybe 10-15%

~REOPEN: when logged in via SAML the credential pop up occurs and asks for the password while at the same time the progressbar already starts. also in case of errors during the request the progressbar cannot be closed anymore but is useless forever at 0% (attachment 8880 [details]). In my case I had an error then, probably caused by this behavior:

Die Ausführung des Kommandos schoolexam/exam/start ist fehlgeschlagen:

Traceback (most recent call last):
  File "%PY2.7%/notifier/threads.py", line 82, in _run
    tmp = self._function()
  File "%PY2.7%/univention/management/console/modules/schoolexam/__init__.py", line 354, in _thread
    room=request.options['room'],
  File "%PY2.7%/univention/lib/umc.py", line 272, in umc_command
    return self.request('POST', 'command/%s' % (path,), data, headers)
  File "%PY2.7%/univention/lib/umc.py", line 297, in request
    return self.send(request)
  File "%PY2.7%/univention/lib/umc.py", line 316, in send
    raise HTTPError(request, response, self.hostname)
HTTPError: 591 on xen3.school.local (command/computerroom/room/acquire): {"status": 591, "message": "Die Ausf\u00fchrung des Kommandos computerroom/room/acquire ist fehlgeschlagen:\n\nTraceback (most recent call last):\n  File \"%PY2.7%/univention/management/console/base.py\", line 249, in execute\n    function.__func__(self, request, *args, **kwargs)\n  File \"%PY2.7%/univention/management/console/modules/decorators.py\", line 192, in _response\n    return function(self, request)\n  File \"%PY2.7%/ucsschool/lib/schoolldap.py\", line 125, in wrapper_func\n    kwargs[USER_READ], po = get_user_connection(bind=__bind_callback, write=False)\n  File \"%PY2.7%/univention/management/console/ldap.py\", line 94, in get_user_connection\n    return connection()\n  File \"%PY2.7%/univention/management/console/ldap.py\", line 140, in _decorated\n    kwargs[loarg], kwargs[poarg] = lo, po = getter()\n  File \"%PY2.7%/univention/management/console/ldap.py\", line 130, in getter\n    conn = connection()\n  File \"%PY2.7%/univention/management/console/ldap.py\", line 53, in connection\n    bind(lo)\n  File \"%PY2.7%/ucsschool/lib/schoolldap.py\", line 384, in bind_user_connection\n    return super(SchoolBaseModule, self).bind_user_connection(lo)\n  File \"%PY2.7%/univention/management/console/base.py\", line 353, in bind_user_connection\n    lo.lo.bind(self._user_dn, self._password)\n  File \"%PY2.7%/univention/uldap.py\", line 165, in bind\n    self.lo.simple_bind_s(self.binddn, self.__encode_pwd(self.bindpw))\n  File \"/usr/lib/python2.7/dist-packages/ldap/ldapobject.py\", line 879, in simple_bind_s\n    res = self._apply_method_s(SimpleLDAPObject.simple_bind_s,*args,**kwargs)\n  File \"/usr/lib/python2.7/dist-packages/ldap/ldapobject.py\", line 860, in _apply_method_s\n    return func(self,*args,**kwargs)\n  File \"/usr/lib/python2.7/dist-packages/ldap/ldapobject.py\", line 215, in simple_bind_s\n    resp_type, resp_data, resp_msgid, resp_ctrls = self.result3(msgid,all=1,timeout=self.timeout)\n  File \"/usr/lib/python2.7/dist-packages/ldap/ldapobject.py\", line 476, in result3\n    resp_ctrl_classes=resp_ctrl_classes\n  File \"/usr/lib/python2.7/dist-packages/ldap/ldapobject.py\", line 483, in result4\n    ldap_result = self._ldap_call(self._l.result4,msgid,all,timeout,add_ctrls,add_intermediates,add_extop)\n  File \"/usr/lib/python2.7/dist-packages/ldap/ldapobject.py\", line 106, in _ldap_call\n    result = func(*args,**kwargs)\nINVALID_CREDENTIALS: {'desc': 'Invalid credentials'}\n", "location": "https://xen3.school.local/univention/command"}

→ we should move the start of the progressbar to the successful execution of the  request.
Comment 13 Florian Best univentionstaff 2017-05-24 18:57:15 CEST
Created attachment 8882 [details]
patch

untested patch.
Comment 14 Daniel Tröder univentionstaff 2017-05-26 11:46:33 CEST
r79684: move start of progressbar to the successful execution of the request
r79685: update advisory

Package: ucs-school-umc-exam
Version: 7.0.4-7A~4.2.0.201705261144
Comment 15 Daniel Tröder univentionstaff 2017-06-01 14:33:06 CEST
90_ucsschool/101_exam_* revealed a problem in the case of users under cn=users,$ldap_base / not in any school-OU.

r79982: fix for case of users not in any OU
r79983: advisory updates

Package: ucs-school-umc-exam           | Branch: ucs_4.2-0
Version: 7.0.4-11A~4.2.0.201706011428  | Scope: ucs-school-4.2

Package: ucs-school-l10n-fr            | Branch: ucs_4.2-0
Version: 2.0.0-8A~4.2.0.201706011431   | Scope: ucs-school-4.2
Comment 16 Florian Best univentionstaff 2017-06-01 18:07:19 CEST
OK: looks good now, even starting parallel exams.
OK: YAML
Comment 17 Sönke Schwardt-Krummrich univentionstaff 2017-06-23 13:32:28 CEST
UCS@school 4.2 v2 has been released.

http://docs.software-univention.de/changelog-ucsschool-4.2v2-de.html

If this error occurs again, please clone this bug.