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

[SlowQuery] Deletion of notification #2586

Open
MrKrisKrisu opened this issue May 22, 2024 · 4 comments
Open

[SlowQuery] Deletion of notification #2586

MrKrisKrisu opened this issue May 22, 2024 · 4 comments
Labels
bug Something isn't working database help wanted Extra attention is needed SlowQuery

Comments

@MrKrisKrisu
Copy link
Member

MrKrisKrisu commented May 22, 2024

Describe the bug

The selective deletion of notifications from the database is very slow, as we work with JSON search parameters here. Another solution must be found here.

Steps to reproduce

# User@Host: xxx[xxx] @ localhost []
# Thread_id: 54900  Schema: traewelling  QC_hit: No
# Query_time: 0.475209  Lock_time: 0.000010  Rows_sent: 0  Rows_examined: 437294
# Rows_affected: 0  Bytes_sent: 0
#
# explain: id	select_type	table	type	possible_keys	key	key_len	ref	rows	r_rows	filtered	r_filtered	Extra
# explain: 1	SIMPLE	notifications	ALL	NULL	NULL	NULL	NULL	452297	437294.00	100.00	0.00	Using where
#
SET timestamp=xxx;
delete from `notifications` where `type` = 'App\\Notifications\\UserJoinedConnection' and json_unquote(json_extract(`data`, '$."status"."id"')) = xxx;
# User@Host: xxx[xxx] @ localhost []
# Thread_id: 54932  Schema: traewelling  QC_hit: No
# Query_time: 1.955730  Lock_time: 0.000015  Rows_sent: 0  Rows_examined: 437294
# Rows_affected: 0  Bytes_sent: 0
#
# explain: id	select_type	table	type	possible_keys	key	key_len	ref	rows	r_rows	filtered	r_filtered	Extra
# explain: 1	SIMPLE	notifications	ALL	NULL	NULL	NULL	NULL	452297	437294.00	100.00	0.00	Using where
#
SET timestamp=xxx;
delete from `notifications` where `type` = 'App\\Notifications\\StatusLiked' and json_unquote(json_extract(`data`, '$."status"."id"')) = xxx;

DatabaseNotification::where('type', UserFollowed::class)
->where('notifiable_id', $follow->follow_id)
->where('data->follower->id', $follow->user_id)
->delete();

@MrKrisKrisu MrKrisKrisu added bug Something isn't working database SlowQuery labels May 22, 2024
@MrKrisKrisu MrKrisKrisu changed the title Deletion of notification slow [SlowQuery] Deletion of notification May 25, 2024
@HerrLevin
Copy link
Member

What if we just keep the notifications?

@MrKrisKrisu
Copy link
Member Author

What if we just keep the notifications?

This would not be GDPR compliant, as we would also delete notifications from users who delete their account.

@HerrLevin
Copy link
Member

Ah. Right. I forgot about that.

We have a few options to improve this slow query. (Just spitting ideas:)

  • Change delete-statement to a select statement and iterate over the entries for individual delete statements
  • Add WITH (NOLOCK) to delete statements (will not work with sqlite iirc)
  • Reduce cost of statement by removing json-queries. We could just slap a LIKE "%statusID/userID% in there, iterate over the results, filter false-positives and delete the rest. This will increase process-time but reduce database-/lock-time.

what do you think?

@MrKrisKrisu MrKrisKrisu added the help wanted Extra attention is needed label May 26, 2024
@jeyemwey
Copy link
Contributor

Reduce cost of statement by removing json-queries. We could just slap a LIKE "%statusID/userID% in there

We could even merge both of those where clauses with an AND, but I'm not sure if LIKE wouldn't just also do a full scan which would lock and create similar issues.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working database help wanted Extra attention is needed SlowQuery
Projects
None yet
Development

No branches or pull requests

3 participants