Skip to content

Various v2.9.0 changes - #4337

Merged
Kami merged 15 commits into
masterfrom
v290_changes
Sep 13, 2018
Merged

Kami merged 15 commits into
masterfrom
v290_changes

Conversation

@Kami

@Kami Kami commented Sep 11, 2018 •

Copy link
Copy Markdown
Member

Trying to get StackStorm/st2web#605 finished so we can include at least one directly user-visible performance improvement.

Changes

  1. Some improvements in the GET /v1/executions API endpoint - sometimes parameters field can be None
  2. GET/v1/actions/overview API endpoint - performance improvements
    1. Remove log statement which logged under INFO every action object which caused unnecessary and massive performance overhead
    2. Reduce number of queries from N * 2 to 2

@Kami Kami added this to the 2.9.0 milestone Sep 11, 2018

secret_parameters = get_secret_parameters(parameters=parameters)
result['parameters'] = mask_secret_parameters(parameters=result['parameters'],
result['parameters'] = mask_secret_parameters(parameters=result.get('parameters', {}),

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Noticed this issue while testing WebUI changes.

GET /actions/views/parameters/1
"""
action_db = LookupUtils._get_action_by_id(action_id)
LOG.info('Found action: %s, runner: %s', action_db, action_db.runner_type['name'])

@Kami Kami Sep 11, 2018 •

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

This reduces duration of this request for 10 seconds where there are many actions in the database.

There is no need to log this, especially under INFO.

result = []
for item in resp.json:
action_api = ActionAPI(**item)
# TODO: This is extreme inefficient and results in N * 2 queries where N is number of

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

This is simply horrible, with aws and some other packs API endpoint takes 30+ seconds to return (well, now 10 seconds less with logging fix, but still too long).

@Kami

Kami commented Sep 11, 2018 •

Copy link
Copy Markdown
Member Author

Changes in bcb63f5 and 7d070d8 substantially improve performance of GET /v1/actions/views/overview API endpoint.

It changes the code to perform 1 database query instead of n * 2 where n is number of actions in the database. This + the logging change now makes this API endpoint return in ~6-7 seconds instead of ~35 seconds on installations with a lot of actions.

Having said that, 6 is still too long, imo (especially, since it was faster in the previous release).

The main problem is that in the new WebUI "Rules" tab / app now depends on this API endpoint.

@enykeev Can we some how get rid of request to this API endpoint all together in the new rules tab? Or can we replace it with a call to some other (faster) API endpoint?

EDIT: If we need to retrieve action parameters for that app / view, there is not much we can do with the existing endpoints (even /v1/actions API endpoint takes 3 seconds to return if we retrieve parameters when there are many actions in the system).

@Kami

Kami commented Sep 12, 2018

Copy link
Copy Markdown
Member Author

Adding to my comment above - another option to provide a better WebUI rules tab user experience would be delay loading action parameters and display rules immediately on the page once rules are loaded and full in action parameters loaded once action data is loaded from that API endpoint.

This would provide a much better user-experience - current approach with page being "white" aka not-rendered / empty until actions overview API endpoint loads is very not friendly for installations with a lot of actions.

/cc @enykeev

@Kami
Kami requested a review from bigmstone September 13, 2018 08:49
@Kami Kami changed the title [WIP] Various v2.9.0 changes Various v2.9.0 changes Sep 13, 2018
/v1/executions/{id}, etc.

Submitting metrics for those unique endpoints would result in a lot of
unique metrics which is an anti-pattern and results on unnecessary high
metrics server load.
don't result in unique metric names and don't cause any load issues.

@bigmstone bigmstone left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

LGTM

@Kami
Kami merged commit 9658e8d into master Sep 13, 2018
@m4dcoder
m4dcoder deleted the v290_changes branch May 29, 2019 21:24
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants