Remove support for 12 character ('short') SHA revisions

ASSIGNED
Assigned to

Status

P2
normal
ASSIGNED
2 years ago
2 months ago

People

(Reporter: wlach, Assigned: emorley)

Tracking

(Depends on: 2 bugs, Blocks: 2 bugs)

Details

Attachments

(3 attachments, 2 obsolete attachments)

I'd like us to consider unsupporting submitting <40 character revisions. Is there any reason we shouldn't? We have a bunch of code associated with keeping the short and long revision fields in sync that I'd like not to have to rewrite when moving stuff into the main db (bug 1306674).
Do we have any submitters who are giving us 12 character revisions but not 40? (or 12 then 40 later?)
Flags: needinfo?(emorley)
Flags: needinfo?(cdawson)
There are still some job types (eg l10n jobs) that use the 12 character SHA.

Though:
(a) I think we should just give them a hard deadline and then leave it up to them
(b) Even if we have to support 12 character jobs, perhaps we can remove support from eg push ingestion / the resultset API etc

New Relic attribute recording is likely a good way forwards here.
Flags: needinfo?(emorley)
(In reply to Ed Morley [:emorley] from comment #2)
> There are still some job types (eg l10n jobs) that use the 12 character SHA.
> ...

Ok, that's annoying but I guess we'll just live with it in the short term.

At least the business logic to handle this will be easier to write with the Django ORM.
Flags: needinfo?(cdawson)
I think we should at least identify which jobs still are using 12 characters and file bugs for them.

The l10n jobs (and I imagine any/most other culprits) are in builds-4hr - I'll have a shot at analysing now.
Even builds-pending/builds-running contain 12 character SHAs :-(

Builds-pending:

{
  "pending": {
    "try": {
      "1a855f92cc45": [
        {
          "submitted_at": 1475064907,
          "id": 125865687,
          "buildername": "Windows 10 64-bit try debug test mochitest-gl-1"
        },


Builds-running (at least it has the full SHA in there):

{
  "running": {
    "autoland": {
      "cdf154426881": [
        {
          "submitted_at": 1475263520,
          "buildername": "Android armv7 API 15+ autoland build",
          "start_time": 1475263723,
          "number": 320,
          "claimed_by_name": "buildbot-master74.bb.releng.usw2.mozilla.com:/builds/buildbot/build1/master",
          "request_ids": [
            126258716
          ],
          "last_heartbeat": 1475264067,
          "id": 127438343,
          "revision": "cdf154426881032934224b0ad60bc366afebff41"
        }
      ],
Created attachment 8796693 [details]
Script to find 12 character jobs in builds-4hr archives
Created attachment 8796694 [details]
Builds-4hr jobs with 12 char SHA on 2016-09-29

Main candidates are:
* PGO builds plus their corresponding tests
* Various jobs run on comm-* (though these are < tier-3)
* Desktop+Android l10n nightlies
Summary: Consider unsupporting submitting 12 character ('short') revisions → Remove support for 12 character ('short') SHA revisions
Depends on: 1306718
Depends on: 1306720
Depends on: 1306722
Depends on: 1306723
Some places are still using 12 character SHAs in UI URLs too.

eg on:
https://hg.mozilla.org/mozilla-central/rev/af6e01b8574b

"[default view]" -> https://treeherder.mozilla.org/#/jobs?repo=mozilla-central&revision=87cd291d2db6
"[failures only]" -> https://treeherder.mozilla.org/#/jobs?repo=mozilla-central&revision=87cd291d2db6&filter-resultStatus=testfailed&filter-resultStatus=busted&filter-resultStatus=exception

(the perfherder links on that page use 40 character SHAs however)

Unfortunately we can't use server logs to see how many people are doing this, since the fragment part of the URL is not sent in the HTTP request. Think we'll have to resort to GitHub code search etc.
Depends on: 1306834
Depends on: 1306835
Depends on: 1306837
Depends on: 1306839
Will, I still think we can simplify things on our side somewhat, ie we can remove support for 12 character SHAs from:
* Resultsets ingestion via our API
* Resultsets ingestion via json-pushes polling
* Resultsets ingestion via Pulse (if not already done; I can't find the schema for it)
* [DONE] Jobs ingestion via Pulse (see pulse-job.yml)
* The `revision` table (since I'm pretty sure for buildbot job ingestion we only need short_revision on the result_set table, right?)

Similarly, whilst the revision hash removal (bug 1257602) is blocked on some older repos (though we should double-check which - given B2G EOL and ESRs only run taskcluster as tier <1 so can probably just have those jobs hidden), we can probably remove support from many other places.
I mean we could even remove `short_revision` from the `result_set` table if we were prepared to do LIKE queries against `long_revision` for these legacy buildbot job types and/or UI URLs that are still using 12 character SHAs.
Created attachment 8809474 [details]
Script to find 12 character jobs in builds-4hr archives
Attachment #8796693 - Attachment is obsolete: true
Created attachment 8809476 [details]
Builds-4hr jobs with 12 char SHA on 2016-11-10

Checking again shows many cases fixed.

Left are some PGO jobs that are instances of bug 1313274, plus aurora instances of bug 1306722 (needs backporting).
Attachment #8796694 - Attachment is obsolete: true
Depends on: 1340265
Now that bug 1306718 is deployed, builds-{pending,running} use the full 40 character SHAs.

This just leaves the self-serve triggered builds (eg sheriffs triggering PGO runs but only pasting in 12 character SHAs), which is bug 1313274. An alternative and possibly quicker fix for that is to make it easier to copy the full SHA from Treeherder (bug 1340265).
Assignee: wlachance → emorley
I'm not going to wait on bug 1313274 or bug 1340265 any longer. Let's just do this.
Priority: -- → P2
Depends on: 1427311
Status: NEW → ASSIGNED
Blocks: 1498644
You need to log in before you can comment on or make changes to this bug.