Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Bug 1310974 - Perform autoclassification after crossreferencing error lines #1955

Merged
merged 1 commit into from
Oct 26, 2016

Conversation

jgraham
Copy link
Member

@jgraham jgraham commented Oct 26, 2016

In the short term this change is a small negative as it will slow down
autoclassification tasks. However it's a necessary step to future
changes that will put the autoclassification results on the text log
error table and allow autoclassification of non-structured lines.

As part of this, we remove the use of callback tasks and just schedule
the crossreference task to run after the parser tasks. This is simpler
and reflects the fact that we only schedule one parser task at a time
now.


This change is Reviewable

… lines

In the short term this change is a small negative as it will slow down
autoclassification tasks. However it's a necessary step to future
changes that will put the autoclassification results on the text log
error table and allow autoclassification of non-structured lines.

As part of this, we remove the use of callback tasks and just schedule
the crossreference task to run after the parser tasks. This is simpler
and reflects the fact that we only schedule one parser task at a time
now.
@jgraham jgraham force-pushed the autoclassify_after_crossreference branch from e45de22 to 04a8255 Compare October 26, 2016 11:34
@jgraham jgraham temporarily deployed to treeherder-prototype October 26, 2016 11:34 Inactive
Copy link
Contributor

@edmorley edmorley left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm presuming after this has landed, we'll then remove the taskset code/models, but are holding off for now so as to not break existing jobs?



@retryable_task(name='crossreference-error-lines', max_retries=10)
def crossreference_error_lines(job_id):
def crossreference_error_lines(job_id, priority):
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This needs to be priority=None for backwards compatibility with any already-queued tasks at the time this is deployed.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's not quite that simple. Bug 1310972 still hasn't landed on prod so we have backwards incompatible changes to the queues in any case. Given that it's not clear that adding a falback here is helpful.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If these are going to land together as part of the "stopping ingestion" plan, then that sounds fine.

@jgraham jgraham merged commit 3777efd into master Oct 26, 2016
@edmorley edmorley deleted the autoclassify_after_crossreference branch October 26, 2016 22:25
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants