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

Cache informaton about Talk desktop usage #1769

Open
ChristophWurst opened this issue Jan 4, 2024 · 3 comments
Open

Cache informaton about Talk desktop usage #1769

ChristophWurst opened this issue Jan 4, 2024 · 3 comments

Comments

@ChristophWurst
Copy link
Member

ChristophWurst commented Jan 4, 2024

Steps to reproduce

  1. Enable the app
  2. Trigger operations that cause a notification

Expected behaviour

Lean datatabase usage.

Actual behaviour

Excessive database usage for information that changes rarely. On my personal instance with fairly little activity the name lookup in oc_authtoken is the second most executed query on the authtoken table. Details are found in nextcloud/server#42574.

public function hasTalkDesktop(string $userId, int $maxAge = 0): bool {
$query = $this->connection->getQueryBuilder();
$query->select('name')
->from('authtoken')
->where($query->expr()->eq('uid', $query->createNamedParameter($userId)));
if ($maxAge !== 0) {
$query->andWhere($query->expr()->gte(
'last_activity',
$query->createNamedParameter($maxAge, IQueryBuilder::PARAM_INT)
));
}
$result = $query->executeQuery();
while ($row = $result->fetch()) {
if (preg_match('/ \(Talk Desktop Client - [A-Za-z ]+\)$/', $row['name'])) {
$result->closeCursor();
return true;
}
}
$result->closeCursor();
return false;
}
}

We can cache this information, even if it's just for a few minutes.

Server configuration

Operating system:

Web server:

Database:

PHP version:

Nextcloud version: 26.0.9

Where did you install Nextcloud from:

Signing status:

Login as admin user into your Nextcloud and access
http://example.com/index.php/settings/integrity/failed 
paste the results here.

List of activated apps:

If you have access to your command line run e.g.:
sudo -u www-data php occ app:list
from within your Nextcloud installation folder

Nextcloud configuration:

If you have access to your command line run e.g.:
sudo -u www-data php occ config:list system
from within your Nextcloud installation folder

or 

Insert your config.php content here
Make sure to remove all sensitive content such as passwords. (e.g. database password, passwordsalt, secret, smtp password, …)

Are you using an external user-backend, if yes which one: LDAP/ActiveDirectory/Webdav/...

Client configuration

Browser:

Operating system:

Logs

Nextcloud log (data/nextcloud.log)

Insert your Nextcloud log here

Browser log

Insert your browser log here, this could for example include:

a) The javascript console log
b) The network log 
c) ...
@nickvergessen
Copy link
Member

Excessive database usage for information that changes rarely.

Well the problem is there is a "timing constraint" on the query. It checks for clients that contacted the server in the last 120 seconds, to avoid duplicate notifications. I'm not sure sure how long we could cache this information, given that the info is used every ~30 seconds when a client pulls notifications.

However we could skip the query when the Talk app is not enabled at all.

But other than that I'm not sure this makes a lot of sense if we don't make it 2 queries:

  1. has a talk client - cachable for ~5-10 minutes?
  2. if 1 is true run a second query to check the timing constraint

@ChristophWurst
Copy link
Member Author

checks for clients that contacted the server in the last 120 seconds

Beware the default activity refresh interval of 300 seconds. If you test a shorter time window you may get false negatives for active clients.

@nickvergessen
Copy link
Member

  • Skip the query for the desktop client itself 🙈

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants