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

ASSIGNED
Assigned to

Status

P2
normal
ASSIGNED
2 years ago
9 months ago

People

(Reporter: wlach, Assigned: emorley)

Tracking

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

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)
(Assignee)

Comment 2

2 years ago
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.
(Assignee)

Updated

2 years ago
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)
(Assignee)

Comment 4

2 years ago
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.
(Assignee)

Comment 5

2 years ago
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"
        }
      ],
(Assignee)

Comment 6

2 years ago
Created attachment 8796693 [details]
Script to find 12 character jobs in builds-4hr archives
(Assignee)

Comment 7

2 years ago
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
(Assignee)

Updated

2 years ago
Summary: Consider unsupporting submitting 12 character ('short') revisions → Remove support for 12 character ('short') SHA revisions
(Assignee)

Updated

2 years ago
Depends on: 1306718
(Assignee)

Updated

2 years ago
Depends on: 1306720
(Assignee)

Updated

2 years ago
Depends on: 1306722
(Assignee)

Updated

2 years ago
Depends on: 1306723
(Assignee)

Comment 8

2 years ago
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.
(Assignee)

Updated

2 years ago
Depends on: 1306834
(Assignee)

Updated

2 years ago
Depends on: 1306835
(Assignee)

Updated

2 years ago
Depends on: 1306837
(Assignee)

Updated

2 years ago
Depends on: 1306839
(Assignee)

Comment 9

2 years ago
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.
(Assignee)

Comment 10

2 years ago
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.
(Assignee)

Updated

2 years ago
Blocks: 1306674
Depends on: 1313274
(Assignee)

Comment 11

2 years ago
Created attachment 8809474 [details]
Script to find 12 character jobs in builds-4hr archives
Attachment #8796693 - Attachment is obsolete: true
(Assignee)

Comment 12

2 years ago
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
(Assignee)

Updated

2 years ago
Depends on: 1340265
(Assignee)

Comment 13

2 years ago
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
(Assignee)

Updated

a year ago
Blocks: 1178227
(Assignee)

Comment 14

10 months ago
I'm not going to wait on bug 1313274 or bug 1340265 any longer. Let's just do this.
Priority: -- → P2
(Assignee)

Updated

9 months ago
Depends on: 1427311
(Assignee)

Updated

9 months ago
Status: NEW → ASSIGNED
You need to log in before you can comment on or make changes to this bug.