194863 2019-02-20 12:03:05 -0800 [ews-app] Add model for handling multiple Buildbot instances 2019-02-28 12:20:45 -0800 1 1 1 Unclassified WebKit Tools / Tests Other Unspecified Unspecified RESOLVED FIXED https://bugs.webkit.org/show_bug.cgi?id=195120 InRadar P2 Normal --- 1 aakash_jain aakash_jain aakash_jain ap commit-queue dean_johnson dewei_zhu jbedard kocsen_chung lforschler slewis webkit-bug-importer oldest_to_newest 1508178 0 aakash_jain 2019-02-20 12:03:05 -0800 [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). 1508195 1 362527 aakash_jain 2019-02-20 12:56:21 -0800 Created attachment 362527 Proposed patch 1509505 2 362868 aakash_jain 2019-02-24 16:22:37 -0800 Created attachment 362868 Proposed patch 1509650 3 362868 jbedard 2019-02-25 09:40:15 -0800 Comment on attachment 362868 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? 1509654 4 aakash_jain 2019-02-25 09:50:18 -0800 (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? 1509667 5 jbedard 2019-02-25 10:20:19 -0800 (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? 1509682 6 aakash_jain 2019-02-25 10:41:35 -0800 > 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. 1509691 7 362868 dean_johnson 2019-02-25 10:52:58 -0800 Comment on attachment 362868 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? 1509723 8 aakash_jain 2019-02-25 11:39:37 -0800 > 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. 1509895 9 362868 commit-queue 2019-02-25 18:55:09 -0800 Comment on attachment 362868 Proposed patch Clearing flags on attachment: 362868 Committed r242066: <https://trac.webkit.org/changeset/242066> 1509896 10 commit-queue 2019-02-25 18:55:10 -0800 All reviewed patches have been landed. Closing bug. 1509906 11 webkit-bug-importer 2019-02-25 19:19:17 -0800 <rdar://problem/48388064> 362527 2019-02-20 12:56:21 -0800 2019-02-24 16:22:37 -0800 Proposed patch instance-model.patch text/plain 2722 aakash_jain SW5kZXg6IFRvb2xzL0NoYW5nZUxvZwo9PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09 PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09Ci0tLSBUb29scy9DaGFuZ2VMb2cJKHJl dmlzaW9uIDI0MTgyNykKKysrIFRvb2xzL0NoYW5nZUxvZwkod29ya2luZyBjb3B5KQpAQCAtMSwz ICsxLDEzIEBACisyMDE5LTAyLTIwICBBYWthc2ggSmFpbiAgPGFha2FzaF9qYWluQGFwcGxlLmNv bT4KKworICAgICAgICBbZXdzLWFwcF0gQWRkIG1vZGVsIGZvciBoYW5kbGluZyBtdWx0aXBsZSBC dWlsZGJvdCBpbnN0YW5jZXMKKyAgICAgICAgaHR0cHM6Ly9idWdzLndlYmtpdC5vcmcvc2hvd19i dWcuY2dpP2lkPTE5NDg2MworCisgICAgICAgIFJldmlld2VkIGJ5IE5PQk9EWSAoT09QUyEpLgor CisgICAgICAgICogQnVpbGRTbGF2ZVN1cHBvcnQvZXdzLWFwcC9ld3MvbW9kZWxzL2luc3RhbmNl LnB5OiBBZGRlZC4KKyAgICAgICAgKEluc3RhbmNlKToKKwogMjAxOS0wMi0yMCAgQ2hyaXMgRHVt ZXogIDxjZHVtZXpAYXBwbGUuY29tPgogCiAgICAgICAgIFJlZ3Jlc3Npb24oUFNPTikgQ3Jhc2gg dW5kZXIgV2ViS2l0OjpXZWJQYWdlUHJveHk6OmRlY2lkZVBvbGljeUZvck5hdmlnYXRpb25BY3Rp b25TeW5jCkluZGV4OiBUb29scy9CdWlsZFNsYXZlU3VwcG9ydC9ld3MtYXBwL2V3cy9tb2RlbHMv aW5zdGFuY2UucHkKPT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09 PT09PT09PT09PT09PT09PT09PT09PQotLS0gVG9vbHMvQnVpbGRTbGF2ZVN1cHBvcnQvZXdzLWFw cC9ld3MvbW9kZWxzL2luc3RhbmNlLnB5CShub25leGlzdGVudCkKKysrIFRvb2xzL0J1aWxkU2xh dmVTdXBwb3J0L2V3cy1hcHAvZXdzL21vZGVscy9pbnN0YW5jZS5weQkod29ya2luZyBjb3B5KQpA QCAtMCwwICsxLDM2IEBACisjIENvcHlyaWdodCAoQykgMjAxOSBBcHBsZSBJbmMuIEFsbCByaWdo dHMgcmVzZXJ2ZWQuCisjCisjIFJlZGlzdHJpYnV0aW9uIGFuZCB1c2UgaW4gc291cmNlIGFuZCBi aW5hcnkgZm9ybXMsIHdpdGggb3Igd2l0aG91dAorIyBtb2RpZmljYXRpb24sIGFyZSBwZXJtaXR0 ZWQgcHJvdmlkZWQgdGhhdCB0aGUgZm9sbG93aW5nIGNvbmRpdGlvbnMKKyMgYXJlIG1ldDoKKyMg MS4gIFJlZGlzdHJpYnV0aW9ucyBvZiBzb3VyY2UgY29kZSBtdXN0IHJldGFpbiB0aGUgYWJvdmUg Y29weXJpZ2h0CisjICAgICBub3RpY2UsIHRoaXMgbGlzdCBvZiBjb25kaXRpb25zIGFuZCB0aGUg Zm9sbG93aW5nIGRpc2NsYWltZXIuCisjIDIuICBSZWRpc3RyaWJ1dGlvbnMgaW4gYmluYXJ5IGZv cm0gbXVzdCByZXByb2R1Y2UgdGhlIGFib3ZlIGNvcHlyaWdodAorIyAgICAgbm90aWNlLCB0aGlz IGxpc3Qgb2YgY29uZGl0aW9ucyBhbmQgdGhlIGZvbGxvd2luZyBkaXNjbGFpbWVyIGluIHRoZQor IyAgICAgZG9jdW1lbnRhdGlvbiBhbmQvb3Igb3RoZXIgbWF0ZXJpYWxzIHByb3ZpZGVkIHdpdGgg dGhlIGRpc3RyaWJ1dGlvbi4KKyMKKyMgVEhJUyBTT0ZUV0FSRSBJUyBQUk9WSURFRCBCWSBBUFBM RSBJTkMuIEFORCBJVFMgQ09OVFJJQlVUT1JTIGBgQVMgSVMnJyBBTkQKKyMgQU5ZIEVYUFJFU1Mg T1IgSU1QTElFRCBXQVJSQU5USUVTLCBJTkNMVURJTkcsIEJVVCBOT1QgTElNSVRFRCBUTywgVEhF IElNUExJRUQKKyMgV0FSUkFOVElFUyBPRiBNRVJDSEFOVEFCSUxJVFkgQU5EIEZJVE5FU1MgRk9S IEEgUEFSVElDVUxBUiBQVVJQT1NFIEFSRQorIyBESVNDTEFJTUVELiBJTiBOTyBFVkVOVCBTSEFM TCBBUFBMRSBJTkMuIE9SIElUUyBDT05UUklCVVRPUlMgQkUgTElBQkxFIEZPUgorIyBBTlkgRElS RUNULCBJTkRJUkVDVCwgSU5DSURFTlRBTCwgU1BFQ0lBTCwgRVhFTVBMQVJZLCBPUiBDT05TRVFV RU5USUFMCisjIERBTUFHRVMgKElOQ0xVRElORywgQlVUIE5PVCBMSU1JVEVEIFRPLCBQUk9DVVJF TUVOVCBPRiBTVUJTVElUVVRFIEdPT0RTIE9SCisjIFNFUlZJQ0VTOyBMT1NTIE9GIFVTRSwgREFU QSwgT1IgUFJPRklUUzsgT1IgQlVTSU5FU1MgSU5URVJSVVBUSU9OKSBIT1dFVkVSCisjIENBVVNF RCBBTkQgT04gQU5ZIFRIRU9SWSBPRiBMSUFCSUxJVFksIFdIRVRIRVIgSU4gQ09OVFJBQ1QsIFNU UklDVCBMSUFCSUxJVFksCisjIE9SIFRPUlQgKElOQ0xVRElORyBORUdMSUdFTkNFIE9SIE9USEVS V0lTRSkgQVJJU0lORyBJTiBBTlkgV0FZIE9VVCBPRiBUSEUgVVNFCisjIE9GIFRISVMgU09GVFdB UkUsIEVWRU4gSUYgQURWSVNFRCBPRiBUSEUgUE9TU0lCSUxJVFkgT0YgU1VDSCBEQU1BR0UuCisK K2Zyb20gX19mdXR1cmVfXyBpbXBvcnQgdW5pY29kZV9saXRlcmFscworCitmcm9tIGRqYW5nby5k YiBpbXBvcnQgbW9kZWxzCisKKworY2xhc3MgSW5zdGFuY2UobW9kZWxzLk1vZGVsKToKKyAgICBp bnN0YW5jZV9pZCA9IG1vZGVscy5BdXRvRmllbGQocHJpbWFyeV9rZXk9VHJ1ZSkKKyAgICBob3N0 bmFtZSA9IG1vZGVscy5UZXh0RmllbGQoKQorICAgIGFjdGl2ZSA9IG1vZGVscy5Cb29sZWFuRmll bGQoZGVmYXVsdD1UcnVlKQorICAgIGNyZWF0ZWQgPSBtb2RlbHMuRGF0ZVRpbWVGaWVsZChhdXRv X25vd19hZGQ9VHJ1ZSkKKyAgICBtb2RpZmllZCA9IG1vZGVscy5EYXRlVGltZUZpZWxkKGF1dG9f bm93PVRydWUpCisKKyAgICBkZWYgX19zdHJfXyhzZWxmKToKKyAgICAgICAgcmV0dXJuICd7fV97 fScuZm9ybWF0KHNlbGYuaW5zdGFuY2VfaWQsIHNlbGYuaG9zdG5hbWUpCg== 362868 2019-02-24 16:22:37 -0800 2019-02-27 14:46:09 -0800 Proposed patch instance-model-v2.patch text/plain 2723 aakash_jain SW5kZXg6IFRvb2xzL0NoYW5nZUxvZwo9PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09 PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09Ci0tLSBUb29scy9DaGFuZ2VMb2cJKHJl dmlzaW9uIDI0MjAxNCkKKysrIFRvb2xzL0NoYW5nZUxvZwkod29ya2luZyBjb3B5KQpAQCAtMSwz ICsxLDEyIEBACisyMDE5LTAyLTI0ICBBYWthc2ggSmFpbiAgPGFha2FzaF9qYWluQGFwcGxlLmNv bT4KKworICAgICAgICBbZXdzLWFwcF0gQWRkIG1vZGVsIGZvciBoYW5kbGluZyBtdWx0aXBsZSBC dWlsZGJvdCBpbnN0YW5jZXMKKyAgICAgICAgaHR0cHM6Ly9idWdzLndlYmtpdC5vcmcvc2hvd19i dWcuY2dpP2lkPTE5NDg2MworCisgICAgICAgIFJldmlld2VkIGJ5IE5PQk9EWSAoT09QUyEpLgor CisgICAgICAgICogQnVpbGRTbGF2ZVN1cHBvcnQvZXdzLWFwcC9ld3MvbW9kZWxzL2J1aWxkYm90 aW5zdGFuY2UucHk6IEFkZGVkLgorCiAyMDE5LTAyLTI0ICBNaWNoYWVsIENhdGFuemFybyAgPG1j YXRhbnphcm9AaWdhbGlhLmNvbT4KIAogICAgICAgICBbV1BFXVtHVEtdIFJlbW92ZSB1c2VyIGFn ZW50IHF1aXJrIGZvciB3YXNoaW5ndG9ucG9zdC5jb20KSW5kZXg6IFRvb2xzL0J1aWxkU2xhdmVT dXBwb3J0L2V3cy1hcHAvZXdzL21vZGVscy9idWlsZGJvdGluc3RhbmNlLnB5Cj09PT09PT09PT09 PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT0K LS0tIFRvb2xzL0J1aWxkU2xhdmVTdXBwb3J0L2V3cy1hcHAvZXdzL21vZGVscy9idWlsZGJvdGlu c3RhbmNlLnB5CShub25leGlzdGVudCkKKysrIFRvb2xzL0J1aWxkU2xhdmVTdXBwb3J0L2V3cy1h cHAvZXdzL21vZGVscy9idWlsZGJvdGluc3RhbmNlLnB5CSh3b3JraW5nIGNvcHkpCkBAIC0wLDAg KzEsMzYgQEAKKyMgQ29weXJpZ2h0IChDKSAyMDE5IEFwcGxlIEluYy4gQWxsIHJpZ2h0cyByZXNl cnZlZC4KKyMKKyMgUmVkaXN0cmlidXRpb24gYW5kIHVzZSBpbiBzb3VyY2UgYW5kIGJpbmFyeSBm b3Jtcywgd2l0aCBvciB3aXRob3V0CisjIG1vZGlmaWNhdGlvbiwgYXJlIHBlcm1pdHRlZCBwcm92 aWRlZCB0aGF0IHRoZSBmb2xsb3dpbmcgY29uZGl0aW9ucworIyBhcmUgbWV0OgorIyAxLiAgUmVk aXN0cmlidXRpb25zIG9mIHNvdXJjZSBjb2RlIG11c3QgcmV0YWluIHRoZSBhYm92ZSBjb3B5cmln aHQKKyMgICAgIG5vdGljZSwgdGhpcyBsaXN0IG9mIGNvbmRpdGlvbnMgYW5kIHRoZSBmb2xsb3dp bmcgZGlzY2xhaW1lci4KKyMgMi4gIFJlZGlzdHJpYnV0aW9ucyBpbiBiaW5hcnkgZm9ybSBtdXN0 IHJlcHJvZHVjZSB0aGUgYWJvdmUgY29weXJpZ2h0CisjICAgICBub3RpY2UsIHRoaXMgbGlzdCBv ZiBjb25kaXRpb25zIGFuZCB0aGUgZm9sbG93aW5nIGRpc2NsYWltZXIgaW4gdGhlCisjICAgICBk b2N1bWVudGF0aW9uIGFuZC9vciBvdGhlciBtYXRlcmlhbHMgcHJvdmlkZWQgd2l0aCB0aGUgZGlz dHJpYnV0aW9uLgorIworIyBUSElTIFNPRlRXQVJFIElTIFBST1ZJREVEIEJZIEFQUExFIElOQy4g QU5EIElUUyBDT05UUklCVVRPUlMgYGBBUyBJUycnIEFORAorIyBBTlkgRVhQUkVTUyBPUiBJTVBM SUVEIFdBUlJBTlRJRVMsIElOQ0xVRElORywgQlVUIE5PVCBMSU1JVEVEIFRPLCBUSEUgSU1QTElF RAorIyBXQVJSQU5USUVTIE9GIE1FUkNIQU5UQUJJTElUWSBBTkQgRklUTkVTUyBGT1IgQSBQQVJU SUNVTEFSIFBVUlBPU0UgQVJFCisjIERJU0NMQUlNRUQuIElOIE5PIEVWRU5UIFNIQUxMIEFQUExF IElOQy4gT1IgSVRTIENPTlRSSUJVVE9SUyBCRSBMSUFCTEUgRk9SCisjIEFOWSBESVJFQ1QsIElO RElSRUNULCBJTkNJREVOVEFMLCBTUEVDSUFMLCBFWEVNUExBUlksIE9SIENPTlNFUVVFTlRJQUwK KyMgREFNQUdFUyAoSU5DTFVESU5HLCBCVVQgTk9UIExJTUlURUQgVE8sIFBST0NVUkVNRU5UIE9G IFNVQlNUSVRVVEUgR09PRFMgT1IKKyMgU0VSVklDRVM7IExPU1MgT0YgVVNFLCBEQVRBLCBPUiBQ Uk9GSVRTOyBPUiBCVVNJTkVTUyBJTlRFUlJVUFRJT04pIEhPV0VWRVIKKyMgQ0FVU0VEIEFORCBP TiBBTlkgVEhFT1JZIE9GIExJQUJJTElUWSwgV0hFVEhFUiBJTiBDT05UUkFDVCwgU1RSSUNUIExJ QUJJTElUWSwKKyMgT1IgVE9SVCAoSU5DTFVESU5HIE5FR0xJR0VOQ0UgT1IgT1RIRVJXSVNFKSBB UklTSU5HIElOIEFOWSBXQVkgT1VUIE9GIFRIRSBVU0UKKyMgT0YgVEhJUyBTT0ZUV0FSRSwgRVZF TiBJRiBBRFZJU0VEIE9GIFRIRSBQT1NTSUJJTElUWSBPRiBTVUNIIERBTUFHRS4KKworZnJvbSBf X2Z1dHVyZV9fIGltcG9ydCB1bmljb2RlX2xpdGVyYWxzCisKK2Zyb20gZGphbmdvLmRiIGltcG9y dCBtb2RlbHMKKworCitjbGFzcyBCdWlsZGJvdEluc3RhbmNlKG1vZGVscy5Nb2RlbCk6CisgICAg aW5zdGFuY2VfaWQgPSBtb2RlbHMuQXV0b0ZpZWxkKHByaW1hcnlfa2V5PVRydWUpCisgICAgaG9z dG5hbWUgPSBtb2RlbHMuVGV4dEZpZWxkKCkKKyAgICBhY3RpdmUgPSBtb2RlbHMuQm9vbGVhbkZp ZWxkKGRlZmF1bHQ9VHJ1ZSkKKyAgICBjcmVhdGVkID0gbW9kZWxzLkRhdGVUaW1lRmllbGQoYXV0 b19ub3dfYWRkPVRydWUpCisgICAgbW9kaWZpZWQgPSBtb2RlbHMuRGF0ZVRpbWVGaWVsZChhdXRv X25vdz1UcnVlKQorCisgICAgZGVmIF9fc3RyX18oc2VsZik6CisgICAgICAgIHJldHVybiAne31f e30nLmZvcm1hdChzZWxmLmluc3RhbmNlX2lkLCBzZWxmLmhvc3RuYW1lKQo=