[locale-inspector] create ES compare-locale storage as part of the buildbot automation

RESOLVED FIXED

Status

Webtools
Elmo
P2
normal
RESOLVED FIXED
4 years ago
3 years ago

People

(Reporter: Pike, Assigned: rhelmer)

Tracking

Dependency tree / graph

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(2 attachments, 1 obsolete attachment)

(Reporter)

Description

4 years ago
Once we have working ES storage and display, we should create the indexed ES documents right in the buildbot automation.

At that point, we can also start using buildbot's log limit to auto-purge the log directories. We should sync with IT on a good number, but I'd set this limit as high as we can afford, as we're creating loads of logs in some scenarios. Just a toolkit build right now with 174 builds creates 1500 log files, so it'd be good to keep a few thousands, if we can.
(Reporter)

Updated

4 years ago
Blocks: 735563
(Reporter)

Updated

4 years ago
Priority: -- → P2
(Reporter)

Updated

3 years ago
Duplicate of this bug: 1039595
(Reporter)

Comment 2

3 years ago
https://github.com/Pike/locale-inspector/blob/master/l10ninsp/steps.py#L134 is the place where we put the data in the logs that we also want in elasticsearch.
(Reporter)

Updated

3 years ago
Assignee: nobody → rhelmer
Summary: [masterball] create ES compare-locale storage as part of the buildbot automation, set a log limit → [locale-inspector] create ES compare-locale storage as part of the buildbot automation, set a log limit
(Assignee)

Updated

3 years ago
Status: NEW → ASSIGNED
(Assignee)

Comment 3

3 years ago
Created attachment 8483011 [details] [diff] [review]
bug958067_locale-inspector.diff

This is the core bit that pushes to ES from the locale-inspector buildbot steps. master-ball needs a patch to support this too.
Attachment #8483011 - Flags: review?(l10n)
(Assignee)

Comment 4

3 years ago
Created attachment 8483015 [details] [diff] [review]
bug958067_master-ball.diff

Adds elasticsearch to requirements and add config placeholders for new ES feature.
Attachment #8483015 - Flags: review?(l10n)
(Assignee)

Comment 5

3 years ago
Regarding setting the log limit - is that done in buildbot.tac via maxRotatedFiles? As that file is auto-generated I am not sure how to change that... is this just something we would change in the running copy on prod, after this change is pushed to prod?
Flags: needinfo?(l10n)

Comment 6

3 years ago
Commit pushed to master at https://github.com/mozilla/socorro

https://github.com/mozilla/socorro/commit/51cd22241bd7b8cc9d01146954f48eb23ffa1aee
bug 958067 - move defaults to its own fragment
(Reporter)

Comment 7

3 years ago
Comment on attachment 8483015 [details] [diff] [review]
bug958067_master-ball.diff

Review of attachment 8483015 [details] [diff] [review]:
-----------------------------------------------------------------

We need a newer elasticsearch for this.

::: requirements.txt
@@ +4,4 @@
>  http://twistedmatrix.com/Releases/Twisted/8.2/Twisted-8.2.0.tar.bz2
>  simplejson
>  MySQL-python
> +elasticsearch<1.0.0

We'll need a > 1.0.0, as we're using ES 1.2.3. I'd go for 1.2.0, per http://elasticsearch-py.readthedocs.org/en/master/Changelog.html.
Attachment #8483015 - Flags: review?(l10n) → review-
(Reporter)

Comment 8

3 years ago
Comment on attachment 8483011 [details] [diff] [review]
bug958067_locale-inspector.diff

Review of attachment 8483011 [details] [diff] [review]:
-----------------------------------------------------------------

I'll cancel the review on this, I don't expect changes in this code from the elasticsearch-py update, but I'd rather have that tested ;-)

I'm pondering back and forth a bit if we should try to catch errors or do something with the result, but that's probably overdoing it.

Also, elasticsearch already retries 3 times, so my thoughts about that are mute.
Attachment #8483011 - Flags: review?(l10n)
(Reporter)

Comment 9

3 years ago
(In reply to Robert Helmer [:rhelmer] from comment #5)
> Regarding setting the log limit - is that done in buildbot.tac via
> maxRotatedFiles? As that file is auto-generated I am not sure how to change
> that... is this just something we would change in the running copy on prod,
> after this change is pushed to prod?

That should be in shared-master/shared.cfg,


# don't conserve disk space
c['buildHorizon'] = c['logHorizon'] = False
Flags: needinfo?(l10n)
(Assignee)

Comment 10

3 years ago
Created attachment 8486556 [details] [diff] [review]
add ES 1.2 to requirements, and ES_COMPARE_* vars to local.py-dist

Upgraded ES to 1.2 on my VM and tested with this, seems to work as expected.
Attachment #8483015 - Attachment is obsolete: true
Attachment #8486556 - Flags: review?(l10n)
(Reporter)

Updated

3 years ago
Attachment #8486556 - Flags: review?(l10n) → review+
(Assignee)

Comment 11

3 years ago
Landed:

To https://github.com/Pike/locale-inspector.git
   3df1f8b..d069fb6  master -> master
To https://github.com/Pike/master-ball.git
   b00e17b..797bcee  master -> master
(Assignee)

Comment 12

3 years ago
(In reply to Axel Hecht [:Pike] from comment #9)
> (In reply to Robert Helmer [:rhelmer] from comment #5)
> > Regarding setting the log limit - is that done in buildbot.tac via
> > maxRotatedFiles? As that file is auto-generated I am not sure how to change
> > that... is this just something we would change in the running copy on prod,
> > after this change is pushed to prod?
> 
> That should be in shared-master/shared.cfg,
> 
> 
> # don't conserve disk space
> c['buildHorizon'] = c['logHorizon'] = False

Let's make sure pushing to ES works consistently before we start pruning logs.
(Assignee)

Comment 13

3 years ago
(In reply to Robert Helmer [:rhelmer] from comment #12)
> (In reply to Axel Hecht [:Pike] from comment #9)
> > (In reply to Robert Helmer [:rhelmer] from comment #5)
> > > Regarding setting the log limit - is that done in buildbot.tac via
> > > maxRotatedFiles? As that file is auto-generated I am not sure how to change
> > > that... is this just something we would change in the running copy on prod,
> > > after this change is pushed to prod?
> > 
> > That should be in shared-master/shared.cfg,
> > 
> > 
> > # don't conserve disk space
> > c['buildHorizon'] = c['logHorizon'] = False
> 
> Let's make sure pushing to ES works consistently before we start pruning
> logs.

Well, I meant more this patch working, than ES itself :)
(Assignee)

Updated

3 years ago
Summary: [locale-inspector] create ES compare-locale storage as part of the buildbot automation, set a log limit → [locale-inspector] create ES compare-locale storage as part of the buildbot automation
(Assignee)

Updated

3 years ago
Blocks: 1072387
(Reporter)

Comment 14

3 years ago
This is fixed and in production, marking fixed.
Status: ASSIGNED → RESOLVED
Last Resolved: 3 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.