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

MH-13152, Reduce Workflow Messages #476

Merged
merged 1 commit into from Oct 10, 2018

Conversation

Projects
None yet
4 participants
@lkiesow
Copy link
Member

lkiesow commented Oct 8, 2018

The high amount of messages sent while workflows are processed can
easily cause performance issues and reduce parallel throughput.

Workflow messages contain metadata about the event and they are used to
update information in Opencast's administrative user interface and in
the external API. These information may change within workflow
operations and need to be updated after each operation as well as on
general workflow state changes (e.g. starting a workflow).

Updates are unnecessary, however, when new operations are launched since
they have been updated right before that operation already and noting
has changed since. Not sending those messages will cut the total amount
of workflow messages almost in half.

@lkiesow lkiesow requested a review from smarquard Oct 8, 2018

@smarquard
Copy link
Contributor

smarquard left a comment

I tested this on our 4.x dev system with some additional logging, and it reduces the number of workflow messages per operation from 2 to 1.

This seems a good candidate for cherry-picking into 3.x, 4.x and 5.x (or adopters handling high volumes may want to do this in local builds).

@smarquard

This comment has been minimized.

Copy link
Contributor

smarquard commented Oct 9, 2018

Is there any issue with the last operation in a workflow here? That is, if the last operation updates mediapackage info, is there a subsequent end-of-workflow operation that will update the index? I'm not seeing any issues with this, just wondering in theory.

@lkiesow

This comment has been minimized.

Copy link
Member

lkiesow commented Oct 9, 2018

The op == null condition should catch the end of workflows. The op.getState() != RUNNING instead of something like state == INSTANTIATED is a deliberate choice to not block special cases like FAILING. Mostly, we probably wouldn't need to send messages then either but there may be special cases and these messages are in any case very rare. So, better safe than sorry ;-P

About the base branch, I have no problem redirecting this to r/6.x and maybe r/5.x. I wouldn't go before that. Any preferences?

@smarquard

This comment has been minimized.

Copy link
Contributor

smarquard commented Oct 9, 2018

r/5.x seems a good choice.

MH-13152, Reduce Workflow Messages
The high amount of messages sent while workflows are processed can
easily cause performance issues and reduce parallel throughput.

Workflow messages contain metadata about the event and they are used to
update information in Opencast's administrative user interface and in
the external API. These information may change within workflow
operations and need to be updated after each operation as well as on
general workflow state changes (e.g. starting a workflow).

Updates are unnecessary, however, when new operations are launched since
they have been updated right before that operation already and noting
has changed since. Not sending those messages will cut the total amount
of workflow messages almost in half.
@JamesUoM

This comment has been minimized.

Copy link
Contributor

JamesUoM commented Oct 9, 2018

I would be interested in backporting this to r/3.x

@karendolan

This comment has been minimized.

Copy link
Contributor

karendolan commented Oct 9, 2018

+1

@lkiesow lkiesow force-pushed the lkiesow:t/mh-13152-reduce-workflow-messages branch from 094a210 to c00246c Oct 9, 2018

@lkiesow lkiesow changed the base branch from develop to r/5.x Oct 9, 2018

@lkiesow

This comment has been minimized.

Copy link
Member

lkiesow commented Oct 9, 2018

Feel free to do that.
I'll leave this at 5.x where it is now.

@smarquard smarquard merged commit 1e85685 into opencast:r/5.x Oct 10, 2018

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment