Page MenuHomePhabricator
Create Task
Maniphest T191024

Exception thrown while running DataSender::sendData in cluster codfw: Data should be a Document, a Script or an array containing Documents and/or Scripts
Closed, ResolvedPublic
Actions

Description

Seen in logstash :

#0 /srv/mediawiki/php-1.31.0-wmf.27/extensions/CirrusSearch/includes/DataSender.php(192): Elastica\Bulk->addData(array, string)
#1 [internal function]: CirrusSearch\DataSender->sendData(string, array)
#2 /srv/mediawiki/php-1.31.0-wmf.27/extensions/CirrusSearch/includes/Job/ElasticaWrite.php(87): call_user_func_array(array, array)
#3 /srv/mediawiki/php-1.31.0-wmf.27/extensions/CirrusSearch/includes/Job/Job.php(98): CirrusSearch\Job\ElasticaWrite->doJob()
#4 /srv/mediawiki/php-1.31.0-wmf.27/extensions/EventBus/includes/JobExecutor.php(59): CirrusSearch\Job\Job->run()
#5 /srv/mediawiki/rpc/RunSingleJob.php(79): JobExecutor->execute(array)
#6 {main}

The cluster param is set to codfw so I bet it's a retried job due to a frozen cluster. At this time I was restarting elastic in codfw.

We need to investigate why we receive such errors.
The data sent to data->sendData() is taken from $this->params['arguments'].
The retried retried job is created by cloning itself : $job = clone $this;

Details

SubjectRepoBranchLines +/-
Revert "Convert ElasticaWrite job to use json compatible params"mediawiki/extensions/CirrusSearchmaster+28 -169
Convert ElasticaWrite job to use json compatible paramsmediawiki/extensions/CirrusSearchmaster+169 -28
Restrict DataSender::sendData to Documentsmediawiki/extensions/CirrusSearchmaster+69 -55
Customize query in gerrit

Related Objects
Search...

Event Timeline

dcausse created this task.Mar 29 2018, 1:49 PM
Restricted Application edited projects, added Discovery-ARCHIVED, Analytics, Discovery-Search; removed Discovery-Search (Current work). · View Herald TranscriptMar 29 2018, 1:49 PM
Restricted Application added a subscriber: Aklapper. · View Herald Transcript
dcausse added projects: MediaWiki-Core-JobQueue, Services.Mar 29 2018, 2:53 PM

Also seeing:

Failed executing job: cirrusSearchElasticaWrite VisualEditor/Diffs arguments=["content",[[]]] cluster=codfw createdAt=1522329753 doNotRetry= errorCount=2 jobReleaseTimestamp=1522334866 method=sendData requestId=WrzoWQpAAE4AAFyMoWoAAABL retryCount=9

The value inside arguments is weird.

dcausse updated the task description. (Show Details)Mar 29 2018, 2:54 PM
dcausse added a subscriber: • Pchelolo.
• Pchelolo added a comment.Mar 29 2018, 2:58 PM

Is this something that was happening before but was not logged, or is that a new error due to the transition to EventBus?

dcausse added a comment.Mar 29 2018, 3:05 PM

I think it's new I see it only for the wikis we switched over:
other wikis
vs
mediawikiwiki and test wikis

• Pchelolo added a comment.EditedMar 29 2018, 3:15 PM

Ok, I think I know what happened here.

The elasticaWrite job does it's own retrying, and in the old queue respected allowRetries = false set in the job, so even though the job reported as failed, it wasn't logged/retried in the old queue.

In the new queue we log all the job failures, because we're mostly in the testing mode. So we need to change either of the things:

  1. We could change the ElasticaWrite job to report success if the retry job was successfully enqueued and only report error if some unexpected exception happened
  2. We could not log the error in the JobExecutor if the allowRetries is false, and treat an error as success, but that seems a bit convoluted.

I personally vote for option 1. In that case, the success of the Job execution would mean that it either successfully written to Elasetic or it successfully posted a retry message. That will leave false to some unexpected disastrous errors in which case ChangeProp will attempt to retry an original job. This will also not affect how the old queue works.

dcausse added a comment.Mar 29 2018, 3:47 PM

I can return true to tell the job executor that we succeeded but I still don't understand why the arguments in the params of the job that has been retried is not correct.
Can there be serialization issues?
I think the job holds a list of instances of the class \Elastica\Script\Script in its params is that supported with the new jobqueue?

• Pchelolo added a comment.EditedMar 29 2018, 4:08 PM

I think the job holds a list of instances of the class \Elastica\Script\Script in its params is that supported with the new jobqueue?

Ulala.. Per T161647 the new queue tries to serialize the job params into JSON, not use the PHP serialize/deserialize. For most of the job it's a drop-in replacement, cause the job params are just strings/numbers.

Does other Cirrus job use some other non-trivial types in params or the ElasticaWrite is the only one?

dcausse added a comment.Mar 29 2018, 4:34 PM

I have to check but I hope not.
I guess we have to fix that and handle the serialization on our own.

• fdans moved this task from Incoming to Radar on the Analytics board.Mar 29 2018, 4:50 PM
• EBjune triaged this task as Medium priority.Mar 29 2018, 5:06 PM
• Pchelolo added a comment.Mar 29 2018, 5:06 PM

I guess we have to fix that and handle the serialization on our own.

I think so. The CirrusSearch jobs extend the deprecated Job class from Mediawiki while the new JobSpecification class actually doesn't allow non-JSON-serializable parameters.

So it seems the best way forward is to modify the Cirrus search jobs.

• EBjune moved this task from needs triage to Up Next on the Discovery-Search board.Mar 29 2018, 5:09 PM
EBernhardson claimed this task.Apr 3 2018, 12:03 AM
EBernhardson moved this task from Up Next to Current work on the Discovery-Search board.
EBernhardson edited projects, added Discovery-Search (Current work); removed Discovery-Search.
EBernhardson moved this task from Incoming to not in use - please delete on the Discovery-Search (Current work) board.
• Pchelolo moved this task from Backlog to watching on the Services board.Apr 3 2018, 2:07 PM
• Pchelolo edited projects, added Services (watching); removed Services.
• mobrovac edited projects, added Services (blocked); removed Services (watching).Apr 4 2018, 4:59 PM
• mobrovac added a parent task: T189137: Migrate CirrusSearch jobs to Kafka queue.Apr 4 2018, 5:02 PM
gerritbot subscribed.Apr 4 2018, 7:23 PM

Change 424039 had a related patch set uploaded (by EBernhardson; owner: EBernhardson):
[mediawiki/extensions/CirrusSearch@master] Restrict DataSender::sendData to Documents

https://gerrit.wikimedia.org/r/424039

gerritbot added a project: Patch-For-Review.Apr 4 2018, 7:23 PM

Change 424040 had a related patch set uploaded (by EBernhardson; owner: EBernhardson):
[mediawiki/extensions/CirrusSearch@master] Convert ElasticaWrite job to use json compatible params

https://gerrit.wikimedia.org/r/424040

dcausse moved this task from not in use - please delete to Needs review on the Discovery-Search (Current work) board.Apr 10 2018, 5:33 PM
gerritbot added a comment.Apr 17 2018, 9:04 AM

Change 424039 merged by jenkins-bot:
[mediawiki/extensions/CirrusSearch@master] Restrict DataSender::sendData to Documents

https://gerrit.wikimedia.org/r/424039

gerritbot added a comment.Apr 17 2018, 9:13 AM

Change 424040 merged by jenkins-bot:
[mediawiki/extensions/CirrusSearch@master] Convert ElasticaWrite job to use json compatible params

https://gerrit.wikimedia.org/r/424040

ReleaseTaggerBot added a project: MW-1.31-release-notes (WMF-deploy-2018-04-17 (1.31.0-wmf.30)).Apr 17 2018, 10:00 AM
EBernhardson moved this task from Needs review to Needs Reporting on the Discovery-Search (Current work) board.Apr 17 2018, 3:53 PM
EBernhardson mentioned this in T192609: Search backend error during sending {numBulk} documents to the {index} index(s) after {tookMs}: {error_message}.Apr 19 2018, 10:50 PM
gerritbot added a comment.Apr 19 2018, 10:54 PM

Change 427827 had a related patch set uploaded (by EBernhardson; owner: EBernhardson):
[mediawiki/extensions/CirrusSearch@master] Revert "Convert ElasticaWrite job to use json compatible params"

https://gerrit.wikimedia.org/r/427827

gerritbot added a comment.Apr 20 2018, 11:55 AM

Change 427893 had a related patch set uploaded (by DCausse; owner: DCausse):
[mediawiki/extensions/CirrusSearch@master] Do not propagate Elastica doc modifications out of DataSender

https://gerrit.wikimedia.org/r/427893

dcausse added a comment.EditedApr 20 2018, 12:02 PM
In T191024#4145680, @gerritbot wrote:

Change 427893 had a related patch set uploaded (by DCausse; owner: DCausse):
[mediawiki/extensions/CirrusSearch@master] Do not propagate Elastica doc modifications out of DataSender

https://gerrit.wikimedia.org/r/427893

Please ignore, unrelated patch. I tagged the wrong bug when sending this patch.

gerritbot added a comment.Apr 20 2018, 2:04 PM

Change 427827 abandoned by EBernhardson:
Revert "Convert ElasticaWrite job to use json compatible params"

Reason:
turned out to not be this patch causing problems.

https://gerrit.wikimedia.org/r/427827

• Pchelolo added a comment.Apr 25 2018, 1:44 PM

I believe the fix for it has been deployed and we can try to proceed with switching cirrus search for some more wikis?

dcausse added a comment.Apr 25 2018, 1:47 PM

@Pchelolo yes I believe so as well.
We might want to test more wikis or all of them perhaps?
We have a cluster restart in progress that triggers write freezes (using the jobqueue to store jobs). It can be interesting to see how it reacts in such conditions?

• Pchelolo closed this task as Resolved.Apr 25 2018, 1:50 PM
• Pchelolo edited projects, added Services (done); removed Patch-For-Review, Services (blocked), Analytics.

We might want to test more wikis or all of them perhaps?

Right now we're running Kafka queue on test wikis and MediaWiki wiki. I don't think we should switch all of them right away, maybe choose a couple projects with a bit more traffic for the next iteration?

We have a cluster restart in progress that triggers write freezes (using the jobqueue to store jobs). It can be interesting to see how it reacts in such conditions?

I can see that from the graphs actually. It's not entirely clear how exactly it reacts, because it's only enabled for super-low-traffic wikis.

I think we can resolve this task and move the conversation to the original T189137

Restricted Application added a project: Analytics. · View Herald TranscriptApr 25 2018, 1:50 PM
Aklapper edited projects, added Analytics-Radar; removed Analytics.Jun 10 2020, 6:44 AM
Content licensed under Creative Commons Attribution-ShareAlike (CC BY-SA) 4.0 unless otherwise noted; code licensed under GNU General Public License (GPL) 2.0 or later and other open source licenses. By using this site, you agree to the Terms of Use, Privacy Policy, and Code of Conduct. · Wikimedia Foundation · Privacy Policy · Code of Conduct · Terms of Use · Disclaimer · CC-BY-SA · GPL