Page MenuHomePhabricator
Create Task
Maniphest T54329

Provide reviewer counts per Gerrit changeset in batch form
Closed, ResolvedPublic
Actions

Description

The Gerrit API is somewhat limited in that retrieving the number of reviewers (a reviewers count) for each changeset requires individual API requests. At somewhere around 76,000 changesets currently (and growing), this isn't a practical system for batch-downloading reviewer counts for all Gerrit changesets.

In discussions with Christian and Chad, a cron job via puppet that executes a simple SQL query and outputs the results of such a query to a text file (or .tsv file, most likely) is probably the best approach to take here.

Version: wmf-deployment
Severity: normal
See Also:
https://bugzilla.wikimedia.org/show_bug.cgi?id=40331

Details

Reference
bz52329

Related Objects

Event Timeline

• bzimport raised the priority of this task from to Medium.Nov 22 2014, 2:07 AM
• bzimport added projects: Gerrit, acl*sre-team.
• bzimport set Reference to bz52329.
• bzimport added a subscriber: Unknown Object (MLST).
• MZMcBride created this task.Jul 31 2013, 5:33 PM
gerritbot added a comment.Jul 31 2013, 5:43 PM

Change 76945 had a related patch set uploaded by Demon:
Provide reviewer counts per patch

https://gerrit.wikimedia.org/r/76945

gerritbot added a comment.Aug 2 2013, 8:35 PM

Change 76945 merged by Ryan Lane:
Provide reviewer counts per patch

https://gerrit.wikimedia.org/r/76945

• MZMcBride added a comment.Aug 3 2013, 2:56 AM

It's 02:56 UTC right now:

$ curl https://gerrit.wikimedia.org/reviewer-counts.json
<!DOCTYPE HTML PUBLIC "-IETFDTD HTML 2.0//EN">
<html><head>
<title>404 Not Found</title>
</head><body>
<h1>Not Found</h1>
<p>The requested URL /reviewer-counts.json was not found on this server.</p>
</body></html>

:-(

• MZMcBride added a comment.Aug 3 2013, 3:20 AM

Tangentially: I don't love the idea of writing into the root directory here. I realize that other jobs already do (e.g., https://gerrit.wikimedia.org/mediawiki-extensions.txt), but it may make sense to create a separate directory for this.

• MZMcBride added a comment.Aug 3 2013, 3:21 AM

(In reply to comment #2)

Change 76945 merged by Ryan Lane:
Provide reviewer counts per patch

https://gerrit.wikimedia.org/r/76945

Copying Daniel Z.'s comment here:

this doesn't appear to work and the reason:

fatal: gerrit2 does not have "accessDatabase" capability.

• demon added a comment.Aug 3 2013, 4:36 AM

(In reply to comment #4)

Tangentially: I don't love the idea of writing into the root directory here.
I
realize that other jobs already do (e.g.,
https://gerrit.wikimedia.org/mediawiki-extensions.txt), but it may make
sense
to create a separate directory for this.

Maybe something like gerrit.wm.o/reports/foo?

(In reply to comment #5)

(In reply to comment #2)

Change 76945 merged by Ryan Lane:
Provide reviewer counts per patch

https://gerrit.wikimedia.org/r/76945

Copying Daniel Z.'s comment here:

this doesn't appear to work and the reason:

fatal: gerrit2 does not have "accessDatabase" capability.

Whoops yeah that's true. Easily fixed and I'll do it when I'm in front of my computer again.

• MZMcBride added a comment.Aug 3 2013, 4:37 AM

(In reply to comment #6)

Maybe something like gerrit.wm.o/reports/foo?

Yup.

Whoops yeah that's true. Easily fixed and I'll do it when I'm in front of my
computer again.

Yay. :-)

QChris added a comment.Aug 3 2013, 9:03 AM

(In reply to comment #6)

(In reply to comment #5)

fatal: gerrit2 does not have "accessDatabase" capability.

[...] I'll do it when I'm in front of my
computer again.

I granted the capability in the mean time.

• MZMcBride added a comment.Aug 7 2013, 11:07 PM

$ curl https://gerrit.wikimedia.org/reviewer-counts.json
<!DOCTYPE HTML PUBLIC "-IETFDTD HTML 2.0//EN">
<html><head>
<title>404 Not Found</title>
</head><body>
<h1>Not Found</h1>
<p>The requested URL /reviewer-counts.json was not found on this server.</p>
</body></html>

What are next steps here?

• MZMcBride added a comment.Aug 12 2013, 6:34 PM

Chad or Christian: can you please take a look at this bug when you get a chance? I imagine it'll require either running (or finding someone to run) this command from the command line to see what error is now output.

• demon added a comment.Aug 12 2013, 7:04 PM

gerrit2 couldn't write to the output file. I fixed this, reviewer counts are now up: https://gerrit.wikimedia.org/reviewer-counts.js

Should now update every weekend.

• MZMcBride added a comment.Aug 13 2013, 3:42 AM

(In reply to comment #11)

gerrit2 couldn't write to the output file. I fixed this, reviewer counts are
now up: https://gerrit.wikimedia.org/reviewer-counts.js

As discussed, this file was moved to https://gerrit.wikimedia.org/reviewer-counts.json. :-) Thanks for working on this.

Should now update every weekend.

I believe you mean daily.

• MZMcBride added a comment.Aug 13 2013, 3:53 AM

Christian: Chad and I looked over https://gerrit.wikimedia.org/reviewer-counts.json and it seems to be missing about 6,100 changesets.

From http://p.defau.lt/?XeY_5g8lrYwjWblQzZVGrQ:

gerrit> select count(*) from changes;
count(*)

78406

gerrit> select count(*) from change_id;
count(*)

72305

Do you know why the change_id table is missing thousands of changesets? Can the query be adjusted to include all changesets?

More importantly, whatever is generating this .json file isn't outputting valid JSON. It doesn't load into Python's json.loads() and http://jsonlint.com/ says it's invalid.

The Python error:

Traceback (most recent call last):

File "<stdin>", line 1, in <module>
File "/System/Library/Frameworks/Python.framework/Versions/2.7/lib/python2.7/json/__init__.py", line 326, in loads
  return _default_decoder.decode(s)
File "/System/Library/Frameworks/Python.framework/Versions/2.7/lib/python2.7/json/decoder.py", line 369, in decode
  raise ValueError(errmsg("Extra data", s, end, len(s)))

ValueError: Extra data: line 2 column 1 - line 72311 column 1 (char 67 - 4914008)

The JSONLint error:

Parse error on line 7:
..._count": "2" }}{ "type": "row",
---------------------^

Expecting 'EOF', '}', ',', ']'

Nemo_bis added a comment.Aug 13 2013, 4:45 AM

No time to read the bug now, but a note in case: the count should not include the patch owner and jenkins-bot.

gerritbot added a comment.Aug 13 2013, 10:27 AM

Change 78944 had a related patch set uploaded by QChris:
Switch to changes table for gerrit's reviewer count file

https://gerrit.wikimedia.org/r/78944

gerritbot added a comment.Aug 18 2013, 4:13 PM

Change 79584 had a related patch set uploaded by QChris:
Add gsql format that returns result set as single Json object

https://gerrit.wikimedia.org/r/79584

gerritbot added a comment.Aug 19 2013, 6:15 PM

Change 79584 merged by Demon:
Add gsql format that returns result set as single Json object

https://gerrit.wikimedia.org/r/79584

Nemo_bis added a comment.Sep 13 2013, 6:27 AM

+ops because that's what the patch is waiting for.

gerritbot added a comment.Sep 17 2013, 5:43 PM

Change 78944 merged by Ottomata:
Switch to changes table for gerrit's reviewer count file

https://gerrit.wikimedia.org/r/78944

• MZMcBride added a comment.Sep 18 2013, 1:35 AM

(In reply to comment #19)

Change 78944 merged by Ottomata:
Switch to changes table for gerrit's reviewer count file

https://gerrit.wikimedia.org/r/78944

Thanks, Otto!

Christian: we still need the JSON --> JSON_SINGLE change, I think? After that gets merged, I think this bug will be resolved/fixed. :-)

gerritbot added a comment.Sep 18 2013, 10:53 AM

Change 84743 had a related patch set uploaded by QChris:
Switch to single Json object for gerrit's reviewer count query

https://gerrit.wikimedia.org/r/84743

QChris added a comment.Sep 18 2013, 10:59 AM

(In reply to comment #20)

Christian: we still need the JSON --> JSON_SINGLE change, I think?

See comment 21 :-)

After that
gets merged, I think this bug will be resolved/fixed. :-)

Before we can merge that, we need to upgrade to a gerrit that actually understands
JSON_SINGLE. The war file is prepared, and only needs to be put in place.
I pinged ^demon about that.

Aklapper added a comment.Sep 19 2013, 5:27 PM

[removing ops keyword again as patch in comment 21 is -1'ed currently.]

gerritbot added a comment.Nov 3 2013, 6:00 PM

Change 84743 merged by Ori.livneh:
Change output format of Gerrit review count gsql to JSON_SINGLE

https://gerrit.wikimedia.org/r/84743

ori added a comment.Nov 3 2013, 6:16 PM

Actually, there's a straightforward way to improve on Ifbb8176fc that I'd like to see. Instead of redirecting the output to a file in /var/www, make a few assertions about the gsql output first -- that consists of a valid, single JSON object (and no more) and that it has the keys you expect it to have. You might want to write a simple script to do that rather than try to fit it in a single command line in the manifest. It should be straightforward to do. Christian, do you think you could do that sometime soon?

Nemo_bis added a comment.Nov 17 2013, 12:47 PM

So, isn't https://gerrit.wikimedia.org/reviewer-counts.json supposed to contain something now? It's still empty.

• MZMcBride added a comment.Nov 17 2013, 11:08 PM

(In reply to comment #26)

So, isn't https://gerrit.wikimedia.org/reviewer-counts.json supposed to
contain something now? It's still empty.

Looks like it broke. Probably a decent time to consider changing the path, as Ori suggested. Maybe /j/?

• MZMcBride added a comment.Nov 27 2013, 5:19 AM

Chad or Christian: could one of you poke at this? Or do you know who might be able to?

QChris added a comment.Nov 27 2013, 2:06 PM

Sorry MZMcBride, I cannot help here.
I had a look back when you said that it broke.
But the cron-job definition looked good to me.
Running the query by hand worked as well for me.
gerrit2 user has Access Database capability as well.

As I lack shell access to the box, I cannot look there directly and
I'll have to defer to Chad.

(I heard random ramblings about not all repos being garbage collected
properly. So some random speculation might be that the cronjob's ssh
connection fails? Maybe username mismatch between gerrit and gerrit2?
I doubt that it's the case, but it would explain both things. Meh.
Someone with shell+sudo access to the machine has to check.)

• MZMcBride added a comment.Dec 10 2013, 5:01 PM

Daniel: when you have a minute, can you please take a look? I suspect some error is being reported to cron.

Dzahn added a comment.Dec 10 2013, 5:12 PM

took a look at the cron on ytterbium

crontab -u gerrit2 -l

  • 1 * * * ssh -p 29418 localhost gerrit gsql --format JSON_SINGLE -c "'SELECT changes.change_id AS change_id, COUNT(DISTINCT patch_set_approvals.account_id) AS reviewer_count FROM changes LEFT JOIN patch_set_approvals ON (changes.change_id = patch_set_approvals.change_id) GROUP BY changes.change_id'" > /var/www/reviewer-counts.json

looked normal, ran it with sudo -u gerrit2, it created the file just fine. i chowned it to gerrit2.

try it now

http://gerrit.wikimedia.org/reviewer-counts.json

Nemo_bis added a comment.Jul 22 2014, 3:38 PM

$ curl https://gerrit.wikimedia.org/reviewer-counts.json | json_pp -json_opt pretty | grep -c '"reviewer_count" : "0"'
973

But most are abandoned patches. :(

Ricordisamoa subscribed.Jul 16 2015, 10:03 PM
Restricted Application added a subscriber: Matanya. · View Herald TranscriptJul 16 2015, 10:03 PM
hashar mentioned this in T147776: cronspam from cobalt after the Gerrit migration.Oct 10 2016, 8:08 AM
Stashbot subscribed.Mar 9 2017, 8:02 PM

Mentioned in SAL (#wikimedia-operations) [2017-03-09T20:02:57Z] <mutante> cobalt: remove crontab entry of user gerrit2 that created reviewer counts, gzip /var/www/reviewer-counts.json and moved to /root/ for backup (re: gerrit:341592) T54329

Content licensed under Creative Commons Attribution-ShareAlike (CC BY-SA) 4.0 unless otherwise noted; code licensed under GNU General Public License (GPL) 2.0 or later and other open source licenses. By using this site, you agree to the Terms of Use, Privacy Policy, and Code of Conduct. · Wikimedia Foundation · Privacy Policy · Code of Conduct · Terms of Use · Disclaimer · CC-BY-SA · GPL