Page MenuHomePhabricator
Create Task
Maniphest T10172

Commits messages with repository-monogram-like strings are confusing the monogram parser
Closed, ResolvedPublic
Actions

Description

Description

While trying to import a repository with large commits, the import was stuck at 99.99%.

./bin/repository/importing show that it hangs on some commits.

In the daemons log, there is the following error:

Daemon 53194 STDE [Mon, 18 Jan 2016 13:03:19 +0100] [2016-01-18 13:03:19] EXCEPTION: (PhutilProxyException) Error while executing Task ID 6926387. {>} (AphrontParameterQueryException) Expected a numeric scalar or null for %Ld conversion. Query: r.id IN (%Ld) at [<phutil>/src/xsprintf/qsprintf.php:294]
Daemon 53194 STDE [Mon, 18 Jan 2016 13:03:19 +0100] arcanist(head=master, ref.master=98d71571e444), phabricator(head=cleger/master, ref.master=d7e6dfc154b1, ref.cleger/master=a0175cb4c586), phutil(head=master, ref.master=adb8a9c074ba)
Daemon 53194 STDE [Mon, 18 Jan 2016 13:03:19 +0100]   #0 <#2> qsprintf_check_scalar_type(string, string, string) called at [<phutil>/src/xsprintf/qsprintf.php:267]
Daemon 53194 STDE [Mon, 18 Jan 2016 13:03:19 +0100]   #1 <#2> qsprintf_check_type(array, string, string) called at [<phutil>/src/xsprintf/qsprintf.php:134]
Daemon 53194 STDE [Mon, 18 Jan 2016 13:03:19 +0100]   #2 <#2> xsprintf_query(AphrontMySQLiDatabaseConnection, string, integer, array, integer) called at [<phutil>/src/xsprintf/xsprintf.php:70]
Daemon 53194 STDE [Mon, 18 Jan 2016 13:03:19 +0100]   #3 <#2> xsprintf(string, AphrontMySQLiDatabaseConnection, array) called at [<phutil>/src/xsprintf/qsprintf.php:64]
Daemon 53194 STDE [Mon, 18 Jan 2016 13:03:19 +0100]   #4 <#2> qsprintf(AphrontMySQLiDatabaseConnection, string, array) called at [<phabricator>/src/applications/repository/query/PhabricatorRepositoryQuery.php:517]
Daemon 53194 STDE [Mon, 18 Jan 2016 13:03:19 +0100]   #5 <#2> PhabricatorRepositoryQuery::buildWhereClauseParts(AphrontMySQLiDatabaseConnection) called at [<phabricator>/src/infrastructure/query/policy/PhabricatorCursorPagedPolicyAwareQuery.php:266]
Daemon 53194 STDE [Mon, 18 Jan 2016 13:03:19 +0100]   #6 <#2> PhabricatorCursorPagedPolicyAwareQuery::buildWhereClause(AphrontMySQLiDatabaseConnection) called at [<phabricator>/src/infrastructure/query/policy/PhabricatorCursorPagedPolicyAwareQuery.php:96]
Daemon 53194 STDE [Mon, 18 Jan 2016 13:03:19 +0100]   #7 <#2> PhabricatorCursorPagedPolicyAwareQuery::loadStandardPageRows(PhabricatorRepository) called at [<phabricator>/src/applications/repository/query/PhabricatorRepositoryQuery.php:170]
Daemon 53194 STDE [Mon, 18 Jan 2016 13:03:19 +0100]   #8 <#2> PhabricatorPolicyAwareQuery::execute() called at [<phabricator>/src/applications/repository/phid/PhabricatorRepositoryRepositoryPHIDType.php:70]
Daemon 53194 STDE [Mon, 18 Jan 2016 13:03:19 +0100]   #9 <#2> PhabricatorRepositoryRepositoryPHIDType::loadNamedObjects(PhabricatorObjectQuery, array) called at [<phabricator>/src/applications/phid/query/PhabricatorObjectQuery.php:100]
Daemon 53194 STDE [Mon, 18 Jan 2016 13:03:19 +0100]   #10 <#2> PhabricatorObjectQuery::loadObjectsByName(array, array) called at [<phabricator>/src/applications/phid/query/PhabricatorObjectQuery.php:57]
Daemon 53194 STDE [Mon, 18 Jan 2016 13:03:19 +0100]   #11 <#2> PhabricatorObjectQuery::loadPage() called at [<phabricator>/src/infrastructure/query/policy/PhabricatorPolicyAwareQuery.php:227]
Daemon 53194 STDE [Mon, 18 Jan 2016 13:03:19 +0100]   #12 <#2> PhabricatorPolicyAwareQuery::execute() called at [<phabricator>/src/applications/phid/query/PhabricatorObjectListQuery.php:113]
Daemon 53194 STDE [Mon, 18 Jan 2016 13:03:19 +0100]   #13 <#2> PhabricatorObjectListQuery::loadObjects(array) called at [<phabricator>/src/applications/phid/query/PhabricatorObjectListQuery.php:50]
Daemon 53194 STDE [Mon, 18 Jan 2016 13:03:19 +0100]   #14 <#2> PhabricatorObjectListQuery::execute() called at [<phabricator>/src/applications/differential/customfield/DifferentialCustomField.php:44]
Daemon 53194 STDE [Mon, 18 Jan 2016 13:03:19 +0100]   #15 <#2> DifferentialCustomField::parseObjectList(string, array) called at [<phabricator>/src/applications/differential/customfield/DifferentialSubscribersField.php:81]
Daemon 53194 STDE [Mon, 18 Jan 2016 13:03:19 +0100]   #16 <#2> DifferentialSubscribersField::parseValueFromCommitMessage(string) called at [<phabricator>/src/applications/differential/conduit/DifferentialParseCommitMessageConduitAPIMethod.php:58]
Daemon 53194 STDE [Mon, 18 Jan 2016 13:03:19 +0100]   #17 <#2> DifferentialParseCommitMessageConduitAPIMethod::execute(ConduitAPIRequest) called at [<phabricator>/src/applications/conduit/method/ConduitAPIMethod.php:118]
Daemon 53194 STDE [Mon, 18 Jan 2016 13:03:19 +0100]   #18 <#2> ConduitAPIMethod::executeMethod(ConduitAPIRequest) called at [<phabricator>/src/applications/conduit/call/ConduitCall.php:135]
Daemon 53194 STDE [Mon, 18 Jan 2016 13:03:19 +0100]   #19 <#2> ConduitCall::executeMethod() called at [<phabricator>/src/applications/conduit/call/ConduitCall.php:85]
Daemon 53194 STDE [Mon, 18 Jan 2016 13:03:19 +0100]   #20 <#2> ConduitCall::execute() called at [<phabricator>/src/applications/diffusion/query/lowlevel/DiffusionLowLevelCommitFieldsQuery.php:41]
Daemon 53194 STDE [Mon, 18 Jan 2016 13:03:19 +0100]   #21 <#2> DiffusionLowLevelCommitFieldsQuery::executeQuery() called at [<phabricator>/src/applications/diffusion/query/lowlevel/DiffusionLowLevelQuery.php:23]
Daemon 53194 STDE [Mon, 18 Jan 2016 13:03:19 +0100]   #22 <#2> DiffusionLowLevelQuery::execute() called at [<phabricator>/src/applications/repository/worker/commitmessageparser/PhabricatorRepositoryCommitMessageParserWorker.php:100]
Daemon 53194 STDE [Mon, 18 Jan 2016 13:03:19 +0100]   #23 <#2> PhabricatorRepositoryCommitMessageParserWorker::updateCommitData(DiffusionCommitRef) called at [<phabricator>/src/applications/repository/worker/commitmessageparser/PhabricatorRepositoryGitCommitMessageParserWorker.php:11]
Daemon 53194 STDE [Mon, 18 Jan 2016 13:03:19 +0100]   #24 <#2> PhabricatorRepositoryGitCommitMessageParserWorker::parseCommitWithRef(PhabricatorRepository, PhabricatorRepositoryCommit, DiffusionCommitRef) called at [<phabricator>/src/applications/repository/worker/commitmessageparser/PhabricatorRepositoryCommitMessageParserWorker.php:40]
Daemon 53194 STDE [Mon, 18 Jan 2016 13:03:19 +0100]   #25 <#2> PhabricatorRepositoryCommitMessageParserWorker::parseCommit(PhabricatorRepository, PhabricatorRepositoryCommit) called at [<phabricator>/src/applications/repository/worker/PhabricatorRepositoryCommitParserWorker.php:40]
Daemon 53194 STDE [Mon, 18 Jan 2016 13:03:19 +0100]   #26 <#2> PhabricatorRepositoryCommitParserWorker::doWork() called at [<phabricator>/src/infrastructure/daemon/workers/PhabricatorWorker.php:122]
Daemon 53194 STDE [Mon, 18 Jan 2016 13:03:19 +0100]   #27 <#2> PhabricatorWorker::executeTask() called at [<phabricator>/src/infrastructure/daemon/workers/storage/PhabricatorWorkerActiveTask.php:171]
Daemon 53194 STDE [Mon, 18 Jan 2016 13:03:19 +0100]   #28 <#2> PhabricatorWorkerActiveTask::executeTask() called at [<phabricator>/src/infrastructure/daemon/workers/PhabricatorTaskmasterDaemon.php:22]
Daemon 53194 STDE [Mon, 18 Jan 2016 13:03:19 +0100]   #29 PhabricatorTaskmasterDaemon::run() called at [<phutil>/src/daemon/PhutilDaemon.php:183]
Daemon 53194 STDE [Mon, 18 Jan 2016 13:03:19 +0100]   #30 PhutilDaemon::execute() called at [<phutil>/scripts/daemon/exec/exec_daemon.php:125]

Debug output of daemon does not seems to add anything relevant.

Reproducing

Import a repository in diffusion with a really large commit such as rtems one:

git://git.rtems.org/rtems.git

The following commits will hang:

rRTEMSba46ffa6169c Message, Change, Owners, Herald
rRTEMSa4f6b023f698 Message, Change, Owners, Herald
rRTEMSfbe75c6e547d Message, Change, Owners, Herald

The expected error message is the one above.
By the way, the commits are insanely huge and it could be nice to be able to mark some commits as unimportable or something like that.

Revisions and Commits

rP Phabricator
D15049rPba2215986639 (stable) Fix two issues with repository monogram regular expressions
D15049rP155cb1d2c5a8 Fix two issues with repository monogram regular expressions

Related Objects

Event Timeline

clemvangelis created this task.Jan 18 2016, 12:20 PM
clemvangelis updated the task description. (Show Details)
epriestley added a revision: D15049: Fix two issues with repository monogram regular expressions.Jan 18 2016, 5:36 PM
epriestley added a subscriber: epriestley.Jan 18 2016, 5:37 PM

This should be fixed by D15049. Thanks for the detailed reproduction instructions, including a repository I could clone locally.

By the way, the commits are insanely huge and it could be nice to be able to mark some commits as unimportable or something like that.

These commits (or the first one, at least) are quite small by our standards. The first one only takes about 500ms to parse standalone:

time ./bin/repository reparse --message --change rRTEMSba46ffa6169c
Done.                                                                         
                                                                              
real	0m0.559s
user	0m0.311s
sys	0m0.079s
epriestley renamed this task from Large commits message/change set are unable to import and daemons are crashing to Commits messages with repository-monogram-like strings are confusing the monogram parser.Jan 18 2016, 5:39 PM
epriestley claimed this task.
epriestley triaged this task as Normal priority.

Specifically, the issue was that these commits contained strings like R2/R13, which the parser was getting confused about because of an improperly grouped regular expression.

epriestley closed this task as Resolved by committing rP155cb1d2c5a8: Fix two issues with repository monogram regular expressions.Jan 18 2016, 5:46 PM
epriestley added a commit: rP155cb1d2c5a8: Fix two issues with repository monogram regular expressions.
clemvangelis added a comment.Jan 19 2016, 8:03 AM

Great, it works like a charm !

Thanks !

epriestley added a commit: rPba2215986639: (stable) Fix two issues with repository monogram regular expressions.Jan 20 2016, 12:26 PM
Phabricator · Built by Phacility