revi-archive/phorge-phorge
1
0
Fork 0
mirror of https://we.phorge.it/source/phorge.git synced 2025-02-04 02:48:25 +01:00
Code Issues Releases Wiki Activity

Clean up some log spam caused by races in VersionedDraft

Browse source 
Summary:
Two minor issues that I caught in the log while fixing Phame permissions:

  - We had a JS bug which would cause us to immediately generate two comment previews at the exact same time -- one for loading the page, and one for "switching to desktop". Instead, only generate the "switch to desktop" preview if we really switched to desktop from a different device layout.
  - These two requests could end up reading/writing the VersionedDraft table at exactly the same time fairly often (e.g., after a comment submission, the page would load, send two preview requests at exactly the same time, and they'd race fairly reliably for me locally). If we do race, recover from the race.

Test Plan:
Submitted some Phame comments.

  - No more error log errors about VersionedDraft keys.
  - Saw only one preview request when loading the page instead of two.

Here's the specific stack trace I caught:

```
[Mon Sep 05 12:15:33.639930 2016] [:error] [pid 50608] [client 127.0.0.1:55278] [2016-09-05 14:15:33] EXCEPTION: (AphrontDuplicateKeyQueryException) #1062: Duplicate entry 'PHID-POST-fknnpzjnsdgc3rqobhst-PHID-USER-pr5rjpuilpfserepsd2k-13' for key 'key_object' at [<phutil>/src/aphront/storage/connection/mysql/AphrontBaseMySQLDatabaseConnection.php:314]
[Mon Sep 05 12:15:33.640801 2016] [:error] [pid 50608] [client 127.0.0.1:55278] arcanist(head=master, ref.master=9e82ef979e81), corgi(head=master, ref.master=5b9171222bc9), instances(head=stable, ref.master=485bc8128198, ref.stable=2983bc917601), ledger(head=master, ref.master=4da4a24b8779), libcore(), phabricator(head=phame2, ref.master=4b6da9735ba7, ref.phame2=4b6da9735ba7), phutil(head=stable, ref.master=97f05269fdb1, ref.stable=c14343ee620e), services(head=stable, ref.master=1fcb5cdb7582, ref.stable=2d8088a5b4b3)
[Mon Sep 05 12:15:33.640815 2016] [:error] [pid 50608] [client 127.0.0.1:55278]   #0 <#2> AphrontBaseMySQLDatabaseConnection::throwCommonException(integer, string) called at [<phutil>/src/aphront/storage/connection/mysql/AphrontBaseMySQLDatabaseConnection.php:348]
[Mon Sep 05 12:15:33.640830 2016] [:error] [pid 50608] [client 127.0.0.1:55278]   #1 <#2> AphrontBaseMySQLDatabaseConnection::throwQueryCodeException(integer, string) called at [<phutil>/src/aphront/storage/connection/mysql/AphrontBaseMySQLDatabaseConnection.php:289]
[Mon Sep 05 12:15:33.640833 2016] [:error] [pid 50608] [client 127.0.0.1:55278]   #2 <#2> AphrontBaseMySQLDatabaseConnection::throwQueryException(mysqli) called at [<phutil>/src/aphront/storage/connection/mysql/AphrontBaseMySQLDatabaseConnection.php:185]
[Mon Sep 05 12:15:33.640836 2016] [:error] [pid 50608] [client 127.0.0.1:55278]   #3 <#2> AphrontBaseMySQLDatabaseConnection::executeRawQuery(string) called at [<phutil>/src/xsprintf/queryfx.php:8]
[Mon Sep 05 12:15:33.640839 2016] [:error] [pid 50608] [client 127.0.0.1:55278]   #4 <#2> queryfx(AphrontMySQLiDatabaseConnection, string, string, string, array, string)
[Mon Sep 05 12:15:33.640841 2016] [:error] [pid 50608] [client 127.0.0.1:55278]   #5 <#2> call_user_func_array(string, array) called at [<phutil>/src/aphront/storage/connection/AphrontDatabaseConnection.php:42]
[Mon Sep 05 12:15:33.640844 2016] [:error] [pid 50608] [client 127.0.0.1:55278]   #6 <#2> AphrontDatabaseConnection::query(string, string, string, array, string) called at [<phabricator>/src/infrastructure/storage/lisk/LiskDAO.php:1261]
[Mon Sep 05 12:15:33.640846 2016] [:error] [pid 50608] [client 127.0.0.1:55278]   #7 <#2> LiskDAO::insertRecordIntoDatabase(string) called at [<phabricator>/src/infrastructure/storage/lisk/LiskDAO.php:1106]
[Mon Sep 05 12:15:33.640849 2016] [:error] [pid 50608] [client 127.0.0.1:55278]   #8 <#2> LiskDAO::insert() called at [<phabricator>/src/infrastructure/storage/lisk/LiskDAO.php:1075]
[Mon Sep 05 12:15:33.640851 2016] [:error] [pid 50608] [client 127.0.0.1:55278]   #9 <#2> LiskDAO::save() called at [<phabricator>/src/applications/draft/storage/PhabricatorVersionedDraft.php:65]
[Mon Sep 05 12:15:33.640854 2016] [:error] [pid 50608] [client 127.0.0.1:55278]   #10 <#2> PhabricatorVersionedDraft::loadOrCreateDraft(string, string, integer) called at [<phabricator>/src/applications/transactions/editengine/PhabricatorEditEngine.php:1669]
[Mon Sep 05 12:15:33.640857 2016] [:error] [pid 50608] [client 127.0.0.1:55278]   #11 <#2> PhabricatorEditEngine::buildCommentResponse(PhamePost) called at [<phabricator>/src/applications/transactions/editengine/PhabricatorEditEngine.php:894]
[Mon Sep 05 12:15:33.640859 2016] [:error] [pid 50608] [client 127.0.0.1:55278]   #12 <#2> PhabricatorEditEngine::buildResponse() called at [<phabricator>/src/applications/phame/controller/post/PhamePostEditController.php:60]
[Mon Sep 05 12:15:33.640862 2016] [:error] [pid 50608] [client 127.0.0.1:55278]   #13 <#2> PhamePostEditController::handleRequest(AphrontRequest) called at [<phabricator>/src/aphront/configuration/AphrontApplicationConfiguration.php:237]
[Mon Sep 05 12:15:33.640865 2016] [:error] [pid 50608] [client 127.0.0.1:55278]   #14 phlog(AphrontDuplicateKeyQueryException) called at [<phabricator>/src/aphront/handler/PhabricatorAjaxRequestExceptionHandler.php:27]
[Mon Sep 05 12:15:33.640868 2016] [:error] [pid 50608] [client 127.0.0.1:55278]   #15 PhabricatorAjaxRequestExceptionHandler::handleRequestException(AphrontRequest, AphrontDuplicateKeyQueryException) called at [<phabricator>/src/aphront/configuration/AphrontApplicationConfiguration.php:644]
[Mon Sep 05 12:15:33.640870 2016] [:error] [pid 50608] [client 127.0.0.1:55278]   #16 AphrontApplicationConfiguration::handleException(AphrontDuplicateKeyQueryException) called at [<phabricator>/src/aphront/configuration/AphrontApplicationConfiguration.php:242]
[Mon Sep 05 12:15:33.640873 2016] [:error] [pid 50608] [client 127.0.0.1:55278]   #17 AphrontApplicationConfiguration::processRequest(AphrontRequest, PhutilDeferredLog, AphrontPHPHTTPSink, MultimeterControl) called at [<phabricator>/src/aphront/configuration/AphrontApplicationConfiguration.php:149]
[Mon Sep 05 12:15:33.640879 2016] [:error] [pid 50608] [client 127.0.0.1:55278]   #18 AphrontApplicationConfiguration::runHTTPRequest(AphrontPHPHTTPSink) called at [<phabricator>/webroot/index.php:17]
```

Reviewers: chad

Reviewed By: chad

Differential Revision: https://secure.phabricator.com/D16494
This commit is contained in:
epriestley 2016-09-05 12:24:58 -07:00
parent 4b6da9735b
commit 74d5ae7bec
3 changed files with 45 additions and 18 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

22
resources/celerity/map.php
View file

@ -437,7 +437,7 @@ return array(
'rsrc/js/application/search/behavior-reorder-profile-menu-items.js' => 'e2e0a072',
'rsrc/js/application/search/behavior-reorder-queries.js' => 'e9581f08',
'rsrc/js/application/slowvote/behavior-slowvote-embed.js' => '887ad43f',
'rsrc/js/application/transactions/behavior-comment-actions.js' => '06460e71',
'rsrc/js/application/transactions/behavior-comment-actions.js' => '0300eae6',
'rsrc/js/application/transactions/behavior-reorder-configs.js' => 'd7a74243',
'rsrc/js/application/transactions/behavior-reorder-fields.js' => 'b59e1e96',
'rsrc/js/application/transactions/behavior-show-older-transactions.js' => '94c65b72',
@ -594,7 +594,7 @@ return array(
'javelin-behavior-bulk-job-reload' => 'edf8a145',
'javelin-behavior-calendar-month-view' => 'fe33e256',
'javelin-behavior-choose-control' => '327a00d1',
'javelin-behavior-comment-actions' => '06460e71',
'javelin-behavior-comment-actions' => '0300eae6',
'javelin-behavior-config-reorder-fields' => 'b6993408',
'javelin-behavior-conpherence-drag-and-drop-photo' => 'cf86d16a',
'javelin-behavior-conpherence-menu' => '1d45c74d',
@ -931,6 +931,15 @@ return array(
'javelin-dom',
'phabricator-keyboard-shortcut',
),
'0300eae6' => array(
'javelin-behavior',
'javelin-stratcom',
'javelin-workflow',
'javelin-dom',
'phuix-form-control-view',
'phuix-icon-view',
'javelin-behavior-phabricator-gesture',
),
'05270951' => array(
'javelin-util',
'javelin-magical-init',
@ -945,15 +954,6 @@ return array(
'aphront-typeahead-control-css',
'phui-tag-view-css',
),
'06460e71' => array(
'javelin-behavior',
'javelin-stratcom',
'javelin-workflow',
'javelin-dom',
'phuix-form-control-view',
'phuix-icon-view',
'javelin-behavior-phabricator-gesture',
),
'065227cc' => array(
'javelin-behavior',
'javelin-dom',

26
src/applications/draft/storage/PhabricatorVersionedDraft.php
View file

@ -55,11 +55,27 @@ final class PhabricatorVersionedDraft extends PhabricatorDraftDAO {
return $draft;
}
return id(new PhabricatorVersionedDraft())
->setObjectPHID($object_phid)
->setAuthorPHID($viewer_phid)
->setVersion((int)$version)
->save();
try {
return id(new self())
->setObjectPHID($object_phid)
->setAuthorPHID($viewer_phid)
->setVersion((int)$version)
->save();
} catch (AphrontDuplicateKeyQueryException $ex) {
$duplicate_exception = $ex;
}
// In rare cases we can race ourselves, and at one point there was a bug
// which caused the browser to submit two preview requests at exactly
// the same time. If the insert failed with a duplicate key exception,
// try to load the colliding row to recover from it.
$draft = self::loadDraft($object_phid, $viewer_phid);
if ($draft) {
return $draft;
}
throw $duplicate_exception;
}
public static function purgeDrafts(

15
webroot/rsrc/js/application/transactions/behavior-comment-actions.js
View file

@ -176,17 +176,28 @@ JX.behavior('comment-actions', function(config) {
JX.DOM.listen(form_node, 'shouldRefresh', null, always_trigger);
request.start();
var old_device = JX.Device.getDevice();
var ondevicechange = function() {
var new_device = JX.Device.getDevice();
var panel = JX.$(config.panelID);
if (JX.Device.getDevice() == 'desktop') {
if (new_device == 'desktop') {
request.setRateLimit(500);
always_trigger();
// Force an immediate refresh if we switched from another device type
// to desktop.
if (old_device != new_device) {
always_trigger();
}
} else {
// On mobile, don't show live previews and only save drafts every
// 10 seconds.
request.setRateLimit(10000);
JX.DOM.hide(panel);
}
old_device = new_device;
};
ondevicechange();