Closed Bug 958067 Opened 10 years ago Closed 9 years ago

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

Categories

(Webtools Graveyard :: Elmo, defect, P2)

defect

Tracking

(Not tracked)

RESOLVED FIXED

People

(Reporter: Pike, Assigned: rhelmer)

References

Details

Attachments

(2 files, 1 obsolete file)

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.
Blocks: 735563
Priority: -- → P2
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.
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
Status: NEW → ASSIGNED
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)
Attached patch bug958067_master-ball.diff (obsolete) — Splinter Review
Adds elasticsearch to requirements and add config placeholders for new ES feature.
Attachment #8483015 - Flags: review?(l10n)
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 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-
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)
(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)
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)
Attachment #8486556 - Flags: review?(l10n) → review+
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
(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.
(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 :)
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
Blocks: 1072387
This is fixed and in production, marking fixed.
Status: ASSIGNED → RESOLVED
Closed: 9 years ago
Resolution: --- → FIXED
Product: Webtools → Webtools Graveyard
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: