Skip to content

Conversation

@come-nc
Copy link
Contributor

@come-nc come-nc commented Dec 14, 2021

Backport #29523

come-nc and others added 13 commits December 14, 2021 16:49
Adds an ldap_full_dn column to store the dn, and only store a sha256
 hash in the ldap_dn which is shorter and can be indexed without
 trouble.
Migration still needs to be implemented.

Signed-off-by: Côme Chilliet <[email protected]>
Signed-off-by: Joas Schilling <[email protected]>
We cannot set ldap_dn_hash column as notnull because it is empty for
 existing users before postSchemaChange is called

Signed-off-by: Côme Chilliet <[email protected]>
This is to ensure new installations do not need to go through migration
 history.

Signed-off-by: Côme Chilliet <[email protected]>
The documentation says it can return false, and even if that is highly
 unlikely for sha256, better safe than sorry.

Signed-off-by: Côme Chilliet <[email protected]>
Signed-off-by: Côme Chilliet <[email protected]>
@come-nc come-nc added the 3. to review Waiting for reviews label Dec 14, 2021
@come-nc come-nc self-assigned this Dec 14, 2021
@come-nc come-nc requested review from a team, ArtificialOwl, juliusknorr and skjnldsv and removed request for a team December 16, 2021 11:42
@skjnldsv skjnldsv added 4. to release Ready to be released and/or waiting for tests to finish and removed 3. to review Waiting for reviews labels Dec 21, 2021
@skjnldsv
Copy link
Member

skjnldsv commented Dec 21, 2021

Failure seems related. LDAP query is too big

+ bash tests/drone-run-php-tests.sh || exit 0
=========================
= List of changed files =
=========================
apps/user_ldap/appinfo/info.xml
apps/user_ldap/composer/composer/autoload_classmap.php
apps/user_ldap/composer/composer/autoload_static.php
apps/user_ldap/lib/Mapping/AbstractMapping.php
apps/user_ldap/lib/Migration/Version1010Date20200630192842.php
apps/user_ldap/lib/Migration/Version1120Date20210917155206.php
apps/user_ldap/lib/Migration/Version1130Date20211102154716.php
=========================
info.xml files are modified
+ NOCOVERAGE=true TEST_SELECTION=DB ./autotest.sh mysql
Using PHP executable /usr/bin/php
Using database oc_autotest
Setup environment for mysql testing on local storage ...
Waiting for MySQL initialisation ...
..
Installing ....
Nextcloud was successfully installed
Testing with mysql ...
No coverage
/usr/local/bin/phpunit --configuration phpunit-autotest.xml --group DB,SLOWDB --log-junit autotest-results-mysql.xml  
PHPUnit 8.5.2 by Sebastian Bergmann and contributors.

Runtime:       PHP 7.2.27-5+0~20200202.35+debian8~1.gbp2925f8
Configuration: /drone/src/tests/phpunit-autotest.xml

........

SQLSTATE[08S01]: Communication link failure: 1153 Got a packet bigger than 'max_allowed_packet' bytes

/drone/src/3rdparty/doctrine/dbal/lib/Doctrine/DBAL/Driver/AbstractMySQLDriver.php:106
/drone/src/3rdparty/doctrine/dbal/lib/Doctrine/DBAL/DBALException.php:169
/drone/src/3rdparty/doctrine/dbal/lib/Doctrine/DBAL/DBALException.php:149
/drone/src/3rdparty/doctrine/dbal/lib/Doctrine/DBAL/Connection.php:914
/drone/src/lib/private/DB/Connection.php:202
/drone/src/3rdparty/doctrine/dbal/lib/Doctrine/DBAL/Query/QueryBuilder.php:206
/drone/src/lib/private/DB/QueryBuilder/QueryBuilder.php:248
/drone/src/apps/user_ldap/lib/Mapping/AbstractMapping.php:218
/drone/src/apps/user_ldap/lib/Mapping/AbstractMapping.php:259
/drone/src/apps/user_ldap/tests/Mapping/AbstractMappingTest.php:300

@skjnldsv skjnldsv added 2. developing Work in progress and removed 4. to release Ready to be released and/or waiting for tests to finish labels Dec 21, 2021
@come-nc
Copy link
Contributor Author

come-nc commented Dec 21, 2021

Thank you for the information.

So after a bit more investigation:
The test failing maps 3332 users with short generated DNs (like uid=as_12,dc=example,dc=com).
Then it asks the mappers for their uids, passing the list of all the DNs.
Because the sha256 hash is longer than these DNs, we hit a request size limit.

I am not sure how to avoid this, whether this test makes sense, and especially why this is only failing on mysql5.6.

@juliusknorr
Copy link
Member

MySQL 5.6 has a lower default value for the

https://dev.mysql.com/doc/refman/5.6/en/server-system-variables.html#sysvar_max_allowed_packet 4194304
https://dev.mysql.com/doc/refman/8.0/en/server-system-variables.html#sysvar_max_allowed_packet 67108864

Decreasing the chunking in

$sliceSize = 1000;
$maxSlices = $totalDBParamLimit / $sliceSize;
$results = [];
$slice = 1;
$fdns = array_map([$this, 'getDNHash'], $fdns);
$fdnsSlice = count($fdns) > $sliceSize ? array_slice($fdns, 0, $sliceSize) : $fdns;
$qb = $this->prepareListOfIdsQuery($fdnsSlice);
while (isset($fdnsSlice[999])) {
to use a shorter sql query might be an option, but I'm wondering if maybe it would be better to just add a higher max_allowed_packet size to our db recommendations.

Signed-off-by: Côme Chilliet <[email protected]>
@come-nc come-nc merged commit 84c4f85 into stable20 Jan 6, 2022
@come-nc come-nc deleted the backport/29523/stable20 branch January 6, 2022 10:28
@come-nc come-nc added 4. to release Ready to be released and/or waiting for tests to finish and removed 2. developing Work in progress labels Jan 27, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

4. to release Ready to be released and/or waiting for tests to finish

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants