Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

re-index contacts with social profiles #22222

Merged
merged 1 commit into from
Dec 22, 2020
Merged

re-index contacts with social profiles #22222

merged 1 commit into from
Dec 22, 2020

Conversation

call-me-matt
Copy link
Member

follow-up from #22085

Signed-off-by: call-me-matt nextcloud@matthiasheinisch.de

apps/dav/lib/Migration/BuildSocialSearchIndex.php Outdated Show resolved Hide resolved
apps/dav/lib/Migration/BuildSocialSearchIndex.php Outdated Show resolved Hide resolved
->andWhere($query->expr()->lte('c.id', $query->createNamedParameter($stopAt)))
->andWhere($query->expr()->gt('c.id', $query->createNamedParameter($offset)))
->orderBy('c.id', 'ASC');
$social_cards = $query->execute();
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Need a limit on this query and then loop the execution with different offsets instead.

Copy link
Member Author

@call-me-matt call-me-matt Aug 19, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What do you mean? Currently it stops after 15sec and schedules a follow-up. Or do you think this query could take too much memory?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

if you have 10k users you load 100k entries from the database, that's going to take a while and eat lots of memory, yeah.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would expect a deployment of that size to have enough memory for such a query. Or did you experience problems when executing these kind of tasks in past, for example here?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fix is in #22311

Well this is also about contacts, not only users, so it can explode rather quickly in size, so better to be save than having people with bug reports on update.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, better safe than sorry... I have pushed an update, can you check? It's limiting the query, but if there are thousands of users there will be many background jobs to work off anyways. I am particularly interested, because I have introduced a way more demanding background job in nextcloud/contacts#1745

$query = $this->db->getQueryBuilder();
// TODO: return contacts with multiple social profiles only once
// FIXME: distinct seems only to return the first parameter?
// $query->selectDistinct('c.addressbookid', 'c.uri', 'c.carddata')
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

any idea how the distinct query can be applied here?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why do you want to distinct? We need to update all entries?

Suggested change
// $query->selectDistinct('c.addressbookid', 'c.uri', 'c.carddata')
$query->select('c.addressbookid', 'c.uri', 'c.carddata')
->selectDistinct('c.id')

but since its the id column you dont select duplicates anyway, so should be fine without it.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Well, I am looking in the cards_properties for socialprofiles and then take the respective contacts from cards (so that I have only the ones with social profiles). But thinking about that now again, I guess the card-properties are in fact the indexed parameters, so that I cannot search in there and must take all contacts?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I have changed that now, see 69f3dca

@call-me-matt call-me-matt self-assigned this Aug 26, 2020
@call-me-matt call-me-matt added the 3. to review Waiting for reviews label Aug 26, 2020
@call-me-matt call-me-matt marked this pull request as ready for review August 26, 2020 21:55
@kesselb
Copy link
Contributor

kesselb commented Sep 11, 2020

The autoloaders are not up to date
Please run: bash build/autoloaderchecker.sh
And commit the result

@skjnldsv skjnldsv added this to the Nextcloud 21 milestone Sep 11, 2020
@call-me-matt
Copy link
Member Author

The autoloaders are not up to date

What exactly do I need to do?
If I run the mentioned script, it modifies the following files:

Filelist
  • apps/accessibility/composer/composer/ClassLoader.php
  • apps/admin_audit/composer/composer/ClassLoader.php
  • apps/cloud_federation_api/composer/composer/ClassLoader.php
  • apps/comments/composer/composer/ClassLoader.php
  • apps/contactsinteraction/composer/composer/ClassLoader.php
  • apps/dav/composer/composer/ClassLoader.php
  • apps/dav/composer/composer/autoload_classmap.php
  • apps/dav/composer/composer/autoload_static.php
  • apps/encryption/composer/composer/ClassLoader.php
  • apps/federatedfilesharing/composer/composer/ClassLoader.php
  • apps/federation/composer/composer/ClassLoader.php
  • apps/files/composer/composer/ClassLoader.php
  • apps/files_sharing/composer/composer/ClassLoader.php
  • apps/files_trashbin/composer/composer/ClassLoader.php
  • apps/files_versions/composer/composer/ClassLoader.php
  • apps/lookup_server_connector/composer/composer/ClassLoader.php
  • apps/oauth2/composer/composer/ClassLoader.php
  • apps/provisioning_api/composer/composer/ClassLoader.php
  • apps/settings/composer/composer/ClassLoader.php
  • apps/sharebymail/composer/composer/ClassLoader.php
  • apps/systemtags/composer/composer/ClassLoader.php
  • apps/testing/composer/composer/ClassLoader.php
  • apps/twofactor_backupcodes/composer/composer/ClassLoader.php
  • apps/updatenotification/composer/composer/ClassLoader.php
  • apps/user_ldap/composer/composer/ClassLoader.php
  • apps/workflowengine/composer/composer/ClassLoader.php
  • lib/composer/composer/ClassLoader.php

But the output is not very convincing. I seems to be trapped in a loop?

$ bash build/autoloaderchecker.sh
Composer found: checking for update
A preview release of the next major version of Composer is available (2.0.0-RC1), run "composer self-update --preview" to give it a try. See also https://github.com/composer/composer/releases for changelogs.
You are already using composer version 1.10.13 (stable channel).

Regenerating main autoloader
Generating optimized autoload files
Generated optimized autoload files containing 1306 classes

Regenerating autoloader for /var/www/nextcloud/apps/accessibility
Generating optimized autoload files (authoritative)
Generated optimized autoload files (authoritative) containing 7 classes

Regenerating autoloader for /var/www/nextcloud/apps/admin_audit
Generating optimized autoload files (authoritative)
Generated optimized autoload files (authoritative) containing 13 classes

Regenerating autoloader for /var/www/nextcloud/apps/cloud_federation_api
Generating optimized autoload files (authoritative)
Generated optimized autoload files (authoritative) containing 4 classes

Regenerating autoloader for /var/www/nextcloud/apps/comments
Generating optimized autoload files (authoritative)
Generated optimized autoload files (authoritative) containing 15 classes

Regenerating autoloader for /var/www/nextcloud/apps/contactsinteraction
Generating optimized autoload files (authoritative)
Generated optimized autoload files (authoritative) containing 10 classes

Regenerating autoloader for /var/www/nextcloud/apps/dav
Generating optimized autoload files (authoritative)
Generated optimized autoload files (authoritative) containing 223 classes

Regenerating autoloader for /var/www/nextcloud/apps/encryption
Generating optimized autoload files (authoritative)
Generated optimized autoload files (authoritative) containing 26 classes

Regenerating autoloader for /var/www/nextcloud/apps/federatedfilesharing
Generating optimized autoload files (authoritative)
Deprecation Notice: Class OCA\FederatedFileSharing\OCM\CloudFederationProviderFiles located in /var/www/nextcloud/apps/federatedfilesharing/lib/ocm/CloudFederationProviderFiles.php does not comply with psr-4 autoloading standard. It will not autoload anymore in Composer v2.0. in phar:///var/www/nextcloud/composer.phar/src/Composer/Autoload/ClassMapGenerator.php:201
Generated optimized autoload files (authoritative) containing 13 classes

Regenerating autoloader for /var/www/nextcloud/apps/federation
Generating optimized autoload files (authoritative)
Generated optimized autoload files (authoritative) containing 14 classes

Regenerating autoloader for /var/www/nextcloud/apps/files
Generating optimized autoload files (authoritative)
Generated optimized autoload files (authoritative) containing 45 classes

Regenerating autoloader for /var/www/nextcloud/apps/files_sharing
Generating optimized autoload files (authoritative)
Generated optimized autoload files (authoritative) containing 63 classes

Regenerating autoloader for /var/www/nextcloud/apps/files_trashbin
Generating optimized autoload files (authoritative)
Generated optimized autoload files (authoritative) containing 34 classes

Regenerating autoloader for /var/www/nextcloud/apps/files_versions
Generating optimized autoload files (authoritative)
Generated optimized autoload files (authoritative) containing 27 classes

Regenerating autoloader for /var/www/nextcloud/apps/lookup_server_connector
Generating optimized autoload files (authoritative)
Generated optimized autoload files (authoritative) containing 3 classes

Regenerating autoloader for /var/www/nextcloud/apps/oauth2
Generating optimized autoload files (authoritative)
Generated optimized autoload files (authoritative) containing 13 classes

Regenerating autoloader for /var/www/nextcloud/apps/provisioning_api
Generating optimized autoload files (authoritative)
Generated optimized autoload files (authoritative) containing 9 classes

Regenerating autoloader for /var/www/nextcloud/apps/settings
Generating optimized autoload files (authoritative)
Generated optimized autoload files (authoritative) containing 47 classes

Regenerating autoloader for /var/www/nextcloud/apps/sharebymail
Generating optimized autoload files (authoritative)
Generated optimized autoload files (authoritative) containing 7 classes

Regenerating autoloader for /var/www/nextcloud/apps/systemtags
Generating optimized autoload files (authoritative)
Generated optimized autoload files (authoritative) containing 5 classes

Regenerating autoloader for /var/www/nextcloud/apps/testing
Generating optimized autoload files (authoritative)
Generated optimized autoload files (authoritative) containing 6 classes

Regenerating autoloader for /var/www/nextcloud/apps/twofactor_backupcodes
Generating optimized autoload files (authoritative)
Generated optimized autoload files (authoritative) containing 23 classes

Regenerating autoloader for /var/www/nextcloud/apps/updatenotification
Generating optimized autoload files (authoritative)
Generated optimized autoload files (authoritative) containing 9 classes

Regenerating autoloader for /var/www/nextcloud/apps/user_ldap
Generating optimized autoload files (authoritative)
Generated optimized autoload files (authoritative) containing 63 classes

Regenerating autoloader for /var/www/nextcloud/apps/workflowengine
Generating optimized autoload files (authoritative)
Generated optimized autoload files (authoritative) containing 30 classes

The autoloaders are not up to date
Please run: bash build/autoloaderchecker.sh
And commit the result

This was referenced Dec 14, 2020
Signed-off-by: call-me-matt <nextcloud@matthiasheinisch.de>
@rullzer
Copy link
Member

rullzer commented Dec 22, 2020

Updated autoloaders.
Rebased. And squashed.

Copy link
Member

@rullzer rullzer left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fine by me

@rullzer rullzer merged commit ee57ef4 into master Dec 22, 2020
@rullzer rullzer deleted the enh/social-index branch December 22, 2020 13:42
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

7 participants