658536 - Also store stars on the TBPL server

Status: RESOLVED FIXED

(Tree Management Graveyard :: TBPL, defect)

Description

[Markus Stange [:mstange]]

Attachment: v1

Once we have build data in a database on the TBPL server (bug 656902) it will make more sense to store stars in the same database instead of requesting them from elastic search (as implemented in bug 601743). Then we'll be able to request them based on build ID, and not based on dates. We'll also avoid the extra HTTP request because we can just send them along with the build data in getRevisionBuilds.php.

This patch makes us keep using the elastic search server as the star source for the TinderboxJSONUser backend. The database on the TBPL server will only be used as a source when using the BuildbotDBUser backend.
However, both the TBPL server database and the elastic search server will be notified of new stars from either backend. The star will be posted to both submitBuildStar.php and starcomment.php. That has the advantage of storing stars in the TBPL server database even before we make the permanent switch away from the TinderboxJSONUser backend.

Comment 1

[Markus Stange [:mstange]]

I forgot to mention another advantage with this patch: Data._loadESNoteData was called whenever new Tinderbox/Buildbot results arrived, and for the buildbot backend that meant it was called once per push. And every time the stars for the whole day were requested. I was going to add some caching to solve this problem, but now we don't even need to do that.

Comment 2

[Arpad Borsos [:Swatinem]]

Comment on attachment 533976 [details] [diff] [review]
v1

Review of attachment 533976 [details] [diff] [review]:
-----------------------------------------------------------------

Apart from the comments below it looks fine.

::: php/getRevisionBuilds.php
@@ +30,5 @@
> +      'timestamp' => round($star['date']->sec)
> +    );
> +  }
> +  $runs[] = $run;
> +}

I don’t like this “PHP while-join” at all. We have a json based DB so use it! Just save the stars array inside tbpl->runs.

::: php/submitBuildStar.php
@@ +47,5 @@
> +  'buildID' => $buildID,
> +  'who' => $who,
> +  'note' => $note,
> +  'date' => new MongoDate(),
> +  'ip' => $_SERVER['REMOTE_ADDR']));

Very good, IP could be valuable.

This should also be updated to push an element into the inner array. A quick readup tells me it should be something like this:
$mongo->tbpl->runs->update(array("_id" => $id), array('$push' => array("notes" => NEW COMMENT)));

Comment 3

[Markus Stange [:mstange]]

Attachment: v2

So much better!

The only thing that's slightly awkward is the date handling in PHP: If a mongo field has the value "Fri May 20 2011 23:43:21 GMT+0200 (CEST)", PHP Mongo will create a MongoDate object for that, which json_encode will turn into this: {"sec":1305927801,"usec":777000}

But I think that's still tolerable.

Comment 4

[Arpad Borsos [:Swatinem]]

Comment on attachment 534131 [details] [diff] [review]
v2

Review of attachment 534131 [details] [diff] [review]:
-----------------------------------------------------------------

::: js/BuildbotDBUser.js
@@ +61,5 @@
> +    if (!run.notes)
> +      return [];
> +    return run.notes.map(function (note) {
> +      // Turn a PHP MongoDate object into a timestamp.
> +      note.timestamp = note.date.sec;

Why not save the unix timestamp inside mongo instead of a date object? Considering the JS here uses a timestamp instead of a date as well.

Comment 5

[Markus Stange [:mstange]]

Attachment: v3

Yeah, that's better.

Comment 6

[Arpad Borsos [:Swatinem]]

Comment on attachment 534151 [details] [diff] [review]
v3

Review of attachment 534151 [details] [diff] [review]:
-----------------------------------------------------------------

;-)

::: js/AddCommentUI.js
@@ +317,5 @@
>        rev: machineResult.revs[Config.treeInfo[machineResult.tree].primaryRepo],
>        who: email,
>        comment: comment,
>        timestamp: Math.ceil((new Date()).getTime()/1000),
> +    }, function () { /* dummy callback */ });

I think it would clean up some code in the future if we forward the comment to WOO in the submitBuildStar.php script server side rather than do 2 requests on the client. But that is for the future.