Jump to content


Feld0

Member Since 18 Apr 2011
Offline Last Active Private
*****

Topics I've Started

Use eager loading to generate the Currently Online list

13 March 2013 - 08:48 PM

In its current design, IP.Board runs the following query once for every single member in the Currently Online list (replacing the "3479" at the end with the ID of each active member, of course):

SELECT m.*, m.member_id as my_member_id,p.*,pp.*,g.*,ccb.cache_content FROM members m LEFT JOIN pfields_content p ON ( p.member_id=m.member_id ) LEFT JOIN profile_portal pp ON ( pp.pp_member_id=m.member_id ) LEFT JOIN groups g ON ( g.g_id=m.member_group_id ) LEFT JOIN content_cache_sigs ccb ON ( ccb.cache_content_id=m.member_id ) WHERE m.member_id=3479

 

It would be sensible to refactor the feature to use eager loading to grab all the members in a single query, no matter how many of them there are:

SELECT m.*, m.member_id as my_member_id,p.*,pp.*,g.*,ccb.cache_content FROM members m LEFT JOIN pfields_content p ON ( p.member_id=m.member_id ) LEFT JOIN profile_portal pp ON ( pp.pp_member_id=m.member_id ) LEFT JOIN groups g ON ( g.g_id=m.member_group_id ) LEFT JOIN content_cache_sigs ccb ON ( ccb.cache_content_id=m.member_id ) WHERE m.member_id IN (3479, 5654, 4543, 6576, 8695, ...)

 

The online list alone is responsible for adding well over a hundred queries to our index. Disabling it made it come up a good half-second faster. All these queries add a great deal more traffic and overhead to the database connection than there needs to be.


4.0: Adopt a naming convention for database columns

08 March 2013 - 07:13 PM

A pet peeve I have with the 3.x series of IPS apps is the lack of a standardized naming convention for tables and columns in the database schema. This is a mostly cosmetic change, yes, but making them adhere to some naming conventions would make them a great deal more human-friendly to work with - which usually means happier developers. ;)

 

 

 

For example, depending on which table you're looking in, you might find a user's account ID referred to as "member", "member_id", "mid", "author_id", "ps_member", "i_member", "log_member", "comment_by", "report_by", or a slew of other names. Topic ID's are referred to as everything from "tid" to "topic_id" to "exdat2". This lack of convention stretches through every app's schema.

 

It's mostly foreign and primary keys that stand to benefit from this. One convention I'm a fan of is calling a table's primary key "id", and foreign keys referring to it "{singular}_id". So instead of letting each table have its own idea of how to reference a topic ID, they could all be, simply, "topic_id".

 

In some tables, column names have enigmatic abbreviations that don't make their purpose particularly obvious - like "can_mm" on the "moderators" table. That column could be renamed to "can_multimod" and suddenly becomes much easier to understand.

 

Sometimes, even the column names within a single table are really inconsistent. Look at the schema of the "moderators" table for a prime example - most of its columns hold what amounts to a simple boolean value denoting whether a moderator can or cannot perform an action. The way it is, one column is prefixed with "can_", two with "mod_can_", one with "allow_", and most don't have a prefix at all. Wouldn't it make some sense to standardize all these columns to be simply prefixed with "can_"?

 

And sometimes, prefixes are used in places where they serve no clear purpose. Every single column in the "nexus_ads" table, for example, is prefixed with "ad_", which isn't particularly meaningful when the entire table only contains ad data. The "ad_id" and "ad_locations" columns in the "nexus_ads" table would still be just as meaningful if they were called "id" and "locations".

 

 

 

Again, I realize that a site's users never have to see any part of the database schema, but developers sure do. Applications will have to be largely refactored to function on IP.Suite 4.0 anyway, so this is a rare opportunity to make breaking changes to the schema.


Auto-Upgrader: Don't overwrite the favicon!

01 March 2013 - 06:19 AM

I decided to give the auto-upgrader a chance for the latest round of maintenance upgrades, and I have to say that I was pleasantly surprised by how smooth the process was! Really love what you've done here, IPS devs.  :thumbsup:

 

I had only one gripe with the process... it overwrote my favicons! :ohmy: I had to manually upload those back to my server after the auto-upgrade, which put a damper on the experience.

 

If you could tweak the upgrader to skip uploading the default favicon.ico, that would be a small, but very appreciated, improvement.


Implement a Sphinx driver for personal conversations

27 January 2013 - 07:23 AM

For large communities, the ability to move tables to InnoDB and replace fulltext searches on them with Sphinx is quite honestly a lifesaver. MyISAM's table locks are a very real issue when you have over 3000 posts per day, and Sphinx performs great as a drop-in replacement for fulltext search.

 

But Sphinx searching support isn't universally implemented across the IPS suite, which makes it impossible to cleanly migrate some tables to InnoDB without breaking the search feature on them. If these tables happen to be part of actively used functionality on a large site, said functionality can run into some nasty performance issues.

 

Case in point: my message_posts and message_topics tables are huge, at ~335 000 and ~34 000 rows, respectively. We've spent a fair bit of money and put a lot of work into setting up an optimal environment for InnoDB, which is quite evident in the general responsiveness of our public-facing pages, but as soon as you open the personal messenger, things slow down noticeably. It's still serviceable, but it does present an issue when one part of the site is an order of magnitude less responsive than the rest of it. It's especially slow when submitting a reply to a convo, as any write operations in MyISAM initiate a full table lock for every user on the site. Those locks stack up when you deal with over 100 pageviews per minute during peak hours.

 

I realize that personal convos aren't a public-facing feature of forums, and optimizing the public-facing stuff first should take priority. But it's still a feature that is typically in very widespread use, and I don't see any reason to bar it from InnoDB's benefits.

 

Replacing MyISAM + fulltext indexes with a properly tuned InnoDB + Sphinx setup proved hugely beneficial for our 1.1 million posts, so I'd like to kindly request that Sphinx support be extended to personal conversations so those can take advantage of InnoDB as well, because the two tables associated with personal convos can easily grow very large on big communities.

 

 

If at all practical, I'd like to see a full Sphinx driver implemented for every IPS application in the future, to completely eliminate MyISAM/fulltext indexes as a dependency for any component of IP.Suite.


Make dynamic "new posts" comply with the HTML spec

24 January 2013 - 01:32 AM

IPS staff, please refer to ticket #835988.

 

 

The feature that notifies users of new posts in a topic made while writing a response (and subsequently allows the user to view them) is not compliant with the HTML spec, which breaks the feature when running IP.Board through a minification mechanism such as CloudFlare's before serving it to the user.

 

This happens not because the minifier breaks something, but because the feature relies on post ID's stored in HTML comments that it parses client-side to function. The minifier is the component compliant with the specs here, as the W3C stipulates that the contents of HTML comments should have no functional purpose. Here is the relevant snippet from the post template:

<!--post:{$post['post']['pid']}-->
<if test="sDeleted:|:$post['post']['_isDeleted'] AND $post['post']['_softDeleteSee']">
	{parse template="softDeletedPostBit" group="topic" params="$post, $displayData['sdData'], $topic"}
</if>
<if test="sDeletedNot:|:! $post['post']['_isDeleted']">
	<div class='post_block hentry clear clearfix <if test="isSolvedCss:|:$post['post']['_isMarkedAnswered']">solved</if> <if test="postQueued:|:$post['post']['_isHidden']">moderated</if>' id='post_id_{$post['post']['pid']}'>

It's understandable that the post ID needs to be in the HTML source somewhere for the feature to function, but putting it in a comment is fundamentally wrong. The better approach, to stay within the specs and allow compliant minifiers to be used, is to put this information in a custom data attribute, which was designed precisely for this purpose and is rightfully left alone by minifiers.