Skip to:
Content

BuddyPress.org

Opened 12 years ago

Closed 8 years ago

#4759 closed defect (bug) (fixed)

Group forum permalink after reply broken when using persistent object cache

Reported by: wpdennis's profile wpdennis Owned by: r-a-y's profile r-a-y
Milestone: 2.7 Priority: normal
Severity: blocker Version: 1.0
Component: Forums Keywords: has-patch commit
Cc:

Description

When using a persistent object cache (in my case memcached) the group forum doesn't refresh correctly. I've found multiple issues after posting a reply to an existing topic.

Wrong Permalinks

In the group forum topic list the topic permalink is broken (for the topic which received a reply). The permalinks to the other topics stay correct.

In bp_get_the_topic_permalink() the topic reaches the third condition (bp_is_single_item()) instead the first (bp_get_the_topic_object_slug()). The reason is, that in $forum_template->topic the $object-* members aren't set, like $forum_template->topic->object_slug and $forum_template->topic->object_id.

Since the $object-* members of $topic should contain the group and are missing, there is no connection between topic and group anymore. So the function returns a default forum link instead of a group forum link, which results in a 404.

But I can't figure out, when the object-* members should be added to $topic. Is it in bb_append_meta() in functions.bb-meta.php? It only adds $forum_template->topic->voices_count in my case, but since it's a function of bbpress, it isn't the right spot, I think.

Can someone help me to find the function which should add the $object-* members?


Disabling Cache

I tried to exclude the group "bb-topic" from object cache, Doing so, I get wrong permalinks for each topic. That's very strange. It means, if a $forum_template->topic isn't loaded from cache, it misses the object-* members. But before I enabled memcached at all, everything worked fine.

Multiple times it happened, that the links started working again. Can't say for sure, but it seems that if the cache expires (or memcached deleted it), the $topic is correct again.


Additional issues

  • After posting a reply, the reply is missing on the topic page (for everyone, including the author himself).
  • In some cases the group forum directory (topic list) doesn't show the reply and doesn't refresh the reply count + timestamp of the topic. Found the maybe related ticket from DJPaul, too.

Maybe we should separate these issues. But before opening more tickets I would like to track the permalink issue down. Maybe it's only one bug - or maybe it's just my installation.

Additionally, BP 1.7 isn't far ahead and maybe we just need a quick workaround instead of a bugfix, if it gets more complex. What do you think?

Attachments (1)

4759.01.patch (626 bytes) - added by r-a-y 8 years ago.

Download all attachments as: .zip

Change History (9)

#1 @DJPaul
12 years ago

Which version of bbPress are you using? The version bundled with BP 1.6.1, or the standalone bbPress plugin?

#2 @wpdennis
12 years ago

Paul, it's the bundled version. I'm running a WP 3.5 multisite with subdomains. BP is sitewide activated.

I think I just found something.

The reason why new replies aren't visible, is that bb_cache_posts() returns old ids by:

wp_cache_get( $key, 'bb_cache_posts_post_ids' )

The cached value is old. But in bb_insert_post() I've found:

wp_cache_flush( 'bb_forums' );
wp_cache_flush( 'bb_query' );
wp_cache_flush( 'bb_cache_posts_post_ids' );

So it should'nt be out dated, right? Digging deeper: wp_cache_flush() calls flush() in object-cache.php from memcached:

function flush() {
	// Don't flush if multi-blog.
	if ( function_exists('is_site_admin') || defined('CUSTOM_USER_TABLE') && defined('CUSTOM_USER_META_TABLE') )
			return true;

	$ret = true;
	foreach ( array_keys($this->mc) as $group )
		$ret &= $this->mc[$group]->flush();
	return $ret;
}

is_site_admin() is deprecated but exists. Is it possible, that every wp_cache_flush fails in a multisite installation and that's the only reason for all issues?

#3 @wpdennis
12 years ago

If we are sure, and someone can confirm that wp_cache_flush() is the problem, we could add the following during the initialization of BuddyPress:

if (bp_is_active('forums') && function_exists('is_site_admin') && function_exists('wp_cache_add_non_persistent_groups'))
    wp_cache_add_non_persistent_groups(array('bb_query', 'bb_cache_posts_post_ids', 'bb_forums', 'bb_topics', 'bb_posts', 'bb_forum', 'bb_topic', 'bb_post', 'bb_option', 'bb_option_not_set', 'bb_topic_tag_terms'));

Read: if the function 'is_site_admin' exists wp_cache_flush() doesn't work, so we could disable forum caching by adding the bbPress groups to non persistent groups.

This would hotfix the issue. Since it's obsolete in BP 1.7 maybe this hotfix is enough.

This needs some testing. I'm not deep enough into bbPress but adding wp_cache_add_non_persistent_groups in one of my plugins solved the issues, as far as I can tell right now.

#4 @DJPaul
12 years ago

Aside: we need one issue per ticket. If we think it's in the bbPress code, the tickets should be posted on the bbPress trac.

#5 @DJPaul
12 years ago

  • Keywords dev-feedback 2nd-opinion removed
  • Milestone changed from Awaiting Review to Future Release

#6 @r-a-y
8 years ago

  • Keywords has-patch added
  • Milestone changed from Future Release to 2.7
  • Version changed from 1.6.1 to 1.0

I came across this issue today (don't ask why).

The problem is with the legacy forums' usage of their own object caching functions, particularly wp_cache_flush( 'my-cache-group' ). By default, WP's wp_cache_flush() doesn't accept a parameter, so it is impossible to clear the cache for these cache groups. Whereas old bbPress / BackPress does support cache group clearing:
https://backpress.trac.wordpress.org/browser/trunk/includes/functions.wp-object-cache.php?marks=77,80#L67

BackPress (incorrectly) assumes that those using a persistent object cache will be using an object cache drop-in that supports cache group clearing.

Attached patch implements wpdennis' suggestion from comment:3 to use wp_cache_add_non_persistent_groups(), but only for the cache groups that are necessary.

Last edited 8 years ago by r-a-y (previous) (diff)

@r-a-y
8 years ago

#7 @boonebgorges
8 years ago

  • Keywords commit added

Whoa.

I came across this issue today (don't ask why).

Ha.

Patch looks good.

#8 @r-a-y
8 years ago

  • Owner set to r-a-y
  • Resolution set to fixed
  • Status changed from new to closed

In 11165:

Legacy Forums: Fix persistent caching issue with bbPress v1.2.

For the legacy forums component, we bundle bbPress v1.2, which includes
BackPress. BackPress uses its own version of wp_cache_flush() that
allows for clearing cache groups:
https://backpress.trac.wordpress.org/browser/trunk/includes/functions.wp-object-cache.php?marks=77,80#L67

bbPress v1 uses this syntax for wp_cache_flush() throughout its codebase:
https://bbpress.trac.wordpress.org/browser/tags/1.2/bb-includes/functions.bb-posts.php?marks=452-454#L449
https://bbpress.trac.wordpress.org/browser/tags/1.2/bb-includes/functions.bb-topics.php?marks=218-219,226-228#L213

However, WordPress' version of wp_cache_flush() does not support clearing
cache groups. This causes various cache invalidation issues as bbPress v1
(incorrectly) assumes that those using a persistent object cache plugin
will support cache group clearing.

To workaround this, we intentionally tell WordPress not to use the
following cache groups as persistent - bb_forums, bb_query, bb_post,
and bb_cache_posts_post_ids.

Props wpdennis, r-a-y.

Fixes #4522, #4759.

Note: See TracTickets for help on using tickets.