Problem/Motivation

When there is a thread with a lot of users in it, this can lead to mysql/mariadb limits on the number of joins.

Proposed resolution

Remaining tasks

User interface changes

API changes

Data model changes

Release notes snippet

CommentFileSizeAuthor
#2 3154558.patch4.23 KBheddn
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

heddn created an issue. See original summary.

heddn’s picture

heddn
Primary language English
Location Nicaragua
CreditAttribution: heddn at MTech, LLC commented
Status: Active » Needs review
FileSize
3154558.patch4.23 KB

Here, we leverage SQL, PHP and the URL context to get the same result, but with a lot fewer joins.

catch’s picture

catch
he/him
Primary language English
CreditAttribution: catch at Tag1 Consulting for American Federation of Teachers commented

The approach in #2 looks sensible to avoid hitting the join limit. If there turns out to be a pure-SQL way to achieve the same thing, this at least gives us a scalable basis to work from.

phjou’s picture

phjou
he/him
Location Vancouver 🇨🇦 🇪🇺
CreditAttribution: phjou commented

@heddn @catch Is there any of you who wants to be become co-maintainer? Just to have the right to merge patches, you both have a strong experience with Drupal and @catch I've seen you over multiple issues on this module.
We currently are two co-maintainers, I am basically merging the easy issues since I stopped using the module and I am maintaining other projects. And I've not seen any activity from @anmolgoyal74 for a while.

heddn’s picture

heddn
Primary language English
Location Nicaragua
CreditAttribution: heddn at MTech, LLC commented

@phjou thank you for your kind offer. I don't have any aspirations of stewarding this module excessively, but if reviewing a few patches here and there and committing things that make sense is OK with you, then please add me as a committer.

phjou’s picture

phjou
he/him
Location Vancouver 🇨🇦 🇪🇺
CreditAttribution: phjou commented

@heddn Just to let you know that I gave you access to commit and make releases. So at least you can merge the issues you solved and you encounter on your project. It will be better than nothing.

heddn’s picture

heddn
Primary language English
Location Nicaragua
CreditAttribution: heddn at MTech, LLC commented 
+++ b/src/Form/PrivateMessageForm.php
@@ -178,8 +178,9 @@ class PrivateMessageForm extends ContentEntityForm {
     if ($privateMessageThread) {
-      $form_state->set('thread_members', $privateMessageThread->getMembers());

The issue with multiple threads with the same users in them is lightly touched upon in #2960360: Add a method to remove a user from a thread. That is partially worked around here in that in the majority of cases (at least in the site I'm using this module), we already have the thread id. This helps protect against thread loading from the wrong thread. But only if you visit /private-messages/{private_message_thread}. The fix here doesn't seem to make any of the problem _worse_. In fact, it might make the situation slightly better. With that in mind, I think this is safe to commit. Any confirmation on that feeling?

catch’s picture

catch
he/him
Primary language English
CreditAttribution: catch at Tag1 Consulting for American Federation of Teachers commented

Agreed, we should be relying on thread ID where it's available. Relying on thread members could probably be limited to creating a new private message and typing in the list of members manually to match to existing threads.

  • heddn committed 9e0c4cb on 8.x-2.x 
    Issue #3154558 by heddn, catch, phjou: 1116 Too many tables; MariaDB can...
heddn’s picture

heddn
Primary language English
Location Nicaragua
CreditAttribution: heddn at MTech, LLC, Tag1 Consulting for American Federation of Teachers commented
Status: Needs review » Fixed

I think the feedback/discussion here is enough to grant a commit here is acceptable. Thanks for everyone's thoughtful feedback.

Status: Fixed » Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.