This is a performance patch that works with the latest version of notifications 5.x. It has two parts:

- Better query fields for _notifications('query'), will replace the table join by a simple IN(gid1, gid2...) condition.

- New _notifications('subscription fields') op implemented, this does nothing for now, but will provide some useful metadata to the Notifications engine so we can add new performance improvements in the future.

As stated, this needs latest notifications-dev to work. If the testing is successful I'll be releasing one more notifications stable version to work with this.

CommentFileSizeAuthor
#1 og_notifications_performance.patch1.36 KBjose reyero

Comments

jose reyero’s picture

jose reyero commented
Status: Active » Needs review
StatusFileSize
new og_notifications_performance.patch1.36 KB

Forgot the patch, sorry :-(

moshe weitzman’s picture

moshe weitzman commented
Status: Needs review » Patch (to be ported)

Actually, needs port to 6

moshe weitzman’s picture

moshe weitzman commented
Project: Organic Groups » Organic Group settings
Version: 5.x-7.3 » 5.x-1.0
Component: og_notifications » Code

Committed. Will deploy now to groups.drupal.org

moshe weitzman’s picture

moshe weitzman commented

Deployed. Submitting a new comment went down to 6ms from 82 seconds. Thats several orders of magnitude. Thanks Jose.

Zen’s picture

Zen commented

subscribe

ajayg’s picture

ajayg commented

Since it is already deployed at g.d.o why not consider this as reviewed by community and commit to 5.x version first. It can be ported to 6.x later.

Zen’s picture

Zen commented

José: Please read http://drupal.org/node/331292#comment-1230089 . Although Notifications 5.x.1.2 was released a couple of weeks after this patch, I'm not sure if the corresponding patches went through.

Please also comment on whether this is required for D6 at all apropos your recent query builder patch. This patch has been committed to OG 5-3 already.

-K

jose reyero’s picture

jose reyero commented
Status: Patch (to be ported) » Closed (fixed)

Yes, this was in the Notifications 5.x.1.2 and it's already in 6.x

Let me know if we need anything else.

About 6.x version, please: #439096: Notification for the post targeted to more than one OG group does not get into queue