Bug 194863 – [ews-app] Add model for handling multiple Buildbot instances

• Component: Tools / Tests
• Status: RESOLVED FIXED

[Aakash Jain]

[ews-app] Add instance table to keep tracking of multiple buildbot instance, or multiple version of same instance (in case Buildbot instance has to be formatted for some reason).

[Aakash Jain]

Created attachment 362527 [details]
Proposed patch

[Aakash Jain]

Created attachment 362868 [details]
Proposed patch

[Jonathan Bedard]

Comment on attachment 362868 [details]
Proposed patch

View in context: https://bugs.webkit.org/attachment.cgi?id=362868&action=review

> Tools/BuildSlaveSupport/ews-app/ews/models/buildbotinstance.py:29
> +    instance_id = models.AutoField(primary_key=True)

Curious why we need a separate instance_id, can't the hostname be a primary key? Do we ever host multiple buildbot instances on the same hostname (that seems wrong)? Or is it possible that we so significantly change the characteristics of a buildbot instance at a specific hostname that we want the EWS database to conceptually consider the updated buildbot a new instance?

[Aakash Jain]

(In reply to Jonathan Bedard from comment #3)
> Curious why we need a separate instance_id, can't the hostname be a primary key? Do we ever host multiple buildbot instances on the same hostname (that seems wrong)?

Yes, it's possible to have two entries in this table with same hostname. This table is specifically being added to handle two cases:
1) if the buildbot machine is formatted, and start sending data again (with same hostname).
2) if we have two buildbot machines(different hostname, e.g.: OpenSource and internal) running in parallel and sending data to Django app.

In case #1, this table would have a new entry for the same hostname, previous entry would be marked inactive (and urls would for inactive hostname would not be created).


> Or is it possible that we so significantly change the characteristics of a buildbot instance at a specific hostname that we want
> the EWS database to conceptually consider the updated buildbot a new
> instance?

[Jonathan Bedard]

(In reply to Aakash Jain from comment #4)
> (In reply to Jonathan Bedard from comment #3)
> > Curious why we need a separate instance_id, can't the hostname be a primary key? Do we ever host multiple buildbot instances on the same hostname (that seems wrong)?
> 
> Yes, it's possible to have two entries in this table with same hostname.
> This table is specifically being added to handle two cases:
> 1) if the buildbot machine is formatted, and start sending data again (with
> same hostname).
> 2) if we have two buildbot machines(different hostname, e.g.: OpenSource and
> internal) running in parallel and sending data to Django app.
> 
> In case #1, this table would have a new entry for the same hostname,
> previous entry would be marked inactive (and urls would for inactive
> hostname would not be created).

Is case #1 meant to handle, for instance, changing buildbot from .8 to 1.0, where the old URLs are basically defunct? I ask because I would not expect to have a new entry if the buildbot instance was, for example, just restarted to pick up a new configuration.

As a related note, I assume code updating active/created/modified will be added in the future, I am wondering how we will distinguish case #1 from something like network failures.
 
> > Or is it possible that we so significantly change the characteristics of a buildbot instance at a specific hostname that we want
> > the EWS database to conceptually consider the updated buildbot a new
> > instance?

[Aakash Jain]

> Is case #1 meant to handle, for instance, changing buildbot from .8 to 1.0, where the old URLs are basically defunct? 
It can handle those cases as well, however the primary motivation was to handle the case when we have an unexpected data loss event on buildbot machine (e.g.: buildbot machine crashed and couldn't be recovered). In that case buildbot on same hostname would re-start build-id/builder-id/step-id from 1. Therefore the previous urls (e.g: https://ews-build.webkit-uat.org/#/builders/1/builds/1) would be valid, but points to new build instead of old build, so the urls generated using data from old build info might link to new data which would be misleading. Therefore we would want to distinguish whether the build data is before/after the buildbot machine was formatted (and not generate urls at all for old build data). Every build column would also have buildbot_instance_id, and using that we could check if the build data is old or new.


> I ask because I would not expect to have a new entry if the buildbot instance was, for example, just restarted to pick up a new configuration.
Exactly, restart or any buildbot configuration change or any network failures would NOT have any impact on this table. We would be adding a new entry in this table for the same hostname, only after a very major event like buildbot machine crash/data-loss/format.

> As a related note, I assume code updating active/created/modified will be added in the future
Yes, many more patches upcoming.

> I am wondering how we will distinguish case #1 from something like network failures.
Network failure wouldn't have any impact on this table. As to how will we decide to add another entry in this table for existing hostname, that can be either manual or in some automated manner (by having some logic to detect such event). Let's discuss that logic, or whether to not have that logic(and do it manually) later on when I upload that patch.

[Dean Johnson]

Comment on attachment 362868 [details]
Proposed patch

View in context: https://bugs.webkit.org/attachment.cgi?id=362868&action=review

Motivation/concept LGTM; couple questions on implementation / design.

> Tools/BuildSlaveSupport/ews-app/ews/models/buildbotinstance.py:33
> +    modified = models.DateTimeField(auto_now=True)

What value does having a 'modified' column bring, and when do you plan on having it update? Does auto_now imply that it updates whenever something is added/updated that has a reference to a BuildbotInstance?

Also, how do you plan on doing the deactivation of the old master?

[Aakash Jain]

> What value does having a 'modified' column bring
When we change an entry from 'active' to 'inactive' (either manually or in some automated manner), the created timestamp would remain same, but 'modified' timestamp would be updated. So it would indicate the timestamp when any entry in this table was last modified. Might help in debugging in some cases. 

> and when do you plan on having it update? Does auto_now imply that it updates whenever something is added/updated that has a reference to a BuildbotInstance?
Django handles it automatically. It is updated whenever the entry in this table is created/updated. 
https://docs.djangoproject.com/en/2.1/ref/models/fields/
"DateField.auto_now": Automatically set the field to now every time the object is saved. Useful for “last-modified” timestamps. 

> Also, how do you plan on doing the deactivation of the old master?
Haven't finalized yet whether it would be using automated logic or manual. Might need some discussion.

[WebKit Commit Bot]

Comment on attachment 362868 [details]
Proposed patch

Clearing flags on attachment: 362868

Committed r242066: <https://trac.webkit.org/changeset/242066>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.

[Radar WebKit Bug Importer]

<rdar://problem/48388064>