feat: calculate post numbers via subquery

On heavily volatile communities running on distributed systems, if two replies are posted at approximately the same time, each node will try to use the same post number, which results in a failed integrity constraint.

Potential options for fixing this were discussed in https://github.com/flarum/framework/issues/3350. We settled on calculating the post number via subquery during post instance creation, which should solve this issue since DB transactions are atomic.

To facilicate this, we needed to implement support for DB attributes that are calculated via subquery at insert, but otherwise stored and accessed normally. This PR adds a `InsertViaSubqueryTrait` which adds exactly this feature.
This commit is contained in:
Alexander Skvortsov 2022-03-23 15:29:55 -04:00
parent 6df4101bae
commit b17f26e6a8
3 changed files with 80 additions and 1 deletions

View File

@ -0,0 +1,73 @@
<?php
/*
* This file is part of Flarum.
*
* For detailed copyright and license information, please view the
* LICENSE file that was distributed with this source code.
*/
namespace Flarum\Database;
use Illuminate\Database\Eloquent\Builder;
use Illuminate\Database\MySqlConnection;
trait InsertsViaSubqueryTrait
{
/**
* A list that maps attribute names to callables that produce subqueries
* used to calculate the inserted value.
*
* Each callable should take an instance of the model being saved,
* and return an Eloquent query builder that queries for the subquery
* generated value. The result of the query should be one row of one column.
*
* Subquery attributes should be added in the static `boot` method of models
* using this trait.
*
* @var array<string, callable(AbstractModel): Builder>
*/
protected static $subqueryAttributes = [];
/**
* Overriden so that some fields can be inserted via subquery.
* The goal here is to construct a subquery that returns primitives
* for the provided `attributes`, and uses additional subqueries for
* statically-specified subqueryAttributes.
*/
protected function insertAndSetId(Builder $query, $attributes)
{
$subqueryAttrNames = array_keys(static::$subqueryAttributes);
$literalAttributes = array_diff_key($attributes, array_flip($subqueryAttrNames));
/** @var Builder */
$insertRowSubquery = static::query()->limit(1);
foreach ($literalAttributes as $attrName => $value) {
$parameter = $query->getGrammar()->parameter($value);
$insertRowSubquery->addBinding($value, 'select');
$insertRowSubquery->selectRaw("$parameter as $attrName");
}
foreach (static::$subqueryAttributes as $attrName => $callback) {
$insertRowSubquery->selectSub($callback($this), $attrName);
}
$attrNames = array_merge(array_keys($literalAttributes), $subqueryAttrNames);
$query->insertUsing($attrNames, $insertRowSubquery);
// This should be accurate, as it's the same mechanism used by Laravel's `insertGetId`.
// See https://github.com/laravel/framework/blob/master/src/Illuminate/Database/Query/Processors/Processor.php#L30-L37.
/** @var MySqlConnection */
$con = $query->getQuery()->getConnection();
$idRaw = $con->getPdo()->lastInsertId($keyName = $this->getKeyName());
$id = is_numeric($idRaw) ? (int) $idRaw : $idRaw;
$this->setAttribute($keyName, $id);
// This is necessary to get the computed value of saved attributes.
$this->exists = true;
$this->refresh();
}
}

View File

@ -74,7 +74,7 @@ class PostReplyHandler
// If this is the first post in the discussion, it's technically not a // If this is the first post in the discussion, it's technically not a
// "reply", so we won't check for that permission. // "reply", so we won't check for that permission.
if ($discussion->post_number_index > 0) { if ($discussion->first_post_id !== null) {
$actor->assertCan('reply', $discussion); $actor->assertCan('reply', $discussion);
} }

View File

@ -10,6 +10,7 @@
namespace Flarum\Post; namespace Flarum\Post;
use Flarum\Database\AbstractModel; use Flarum\Database\AbstractModel;
use Flarum\Database\InsertsViaSubqueryTrait;
use Flarum\Database\ScopeVisibilityTrait; use Flarum\Database\ScopeVisibilityTrait;
use Flarum\Discussion\Discussion; use Flarum\Discussion\Discussion;
use Flarum\Foundation\EventGeneratorTrait; use Flarum\Foundation\EventGeneratorTrait;
@ -41,6 +42,7 @@ class Post extends AbstractModel
{ {
use EventGeneratorTrait; use EventGeneratorTrait;
use ScopeVisibilityTrait; use ScopeVisibilityTrait;
use InsertsViaSubqueryTrait;
protected $table = 'posts'; protected $table = 'posts';
@ -102,6 +104,10 @@ class Post extends AbstractModel
}); });
static::addGlobalScope(new RegisteredTypesScope); static::addGlobalScope(new RegisteredTypesScope);
static::$subqueryAttributes['number'] = function (Post $post) {
return static::query()->where('discussion_id', $post->discussion_id)->selectRaw('max(number)+1');
};
} }
/** /**