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

Pipeline list and execution list views refresh very inefficient and slow #544

Open
eea03 opened this issue Oct 27, 2015 · 4 comments
Open

Comments

@eea03
Copy link

eea03 commented Oct 27, 2015

During testing on our development environment, we have discovered that refreshing of pipelines and executions lists views is very inefficient. There are several reasons for this:

  1. changed pipelines / executions test in database is no table entries specific - it checks in database if some pipeline / execution has been changed since last refresh but regardless if should or should not be seen in table
  2. When change / delete is detected, ItemSetChangeEvent is generated so all the entries in the table are refreshed.
  3. refresh of containter generates tons of selects - it gets the pipelines and executions individually by getPipeline() and getExecution() service methods. Even worse, the counts of selects cannot be reasonably calculated. E.g. for 60 pipelines, refresh generated 266 selects. This was constant for each refresh while count of pipelines remained the same. When pipeline count increased, so did the count of selects. For executions, the count of selects is significantly lower even when there are more executions, but still quite high (100 selects for 120 executions). And the count of selects clearly depends also on page size of table. Mentioned values were with 20 page size, when i decreased to 10, also count of selects decreased to half

On environments with large amount of data, especially when database is on other server, as we already have seen on our test environment.

From what I read, I think there are 2 possible ways of fixing this:

  • use ValueChangeEvents instead of ItemSetChangeEvent - this way only really changed items would have been polled from db and refreshed in view. Disadvantage of this is that refresher would have to identify all changes in db and create change events from them
  • use service class to poll table entries in one select - removing the large number of rubish selects

Good start is to read:

This is quite a complex problem and requires further deep examination

@eea03
Copy link
Author

eea03 commented Nov 9, 2015

So after further thorrow testing, it was discovered that this large amount of selects was generated during creating tables - more exactly checking permissions for action buttons

This means that in pipeline view, there are 6 checks and each tries to load the pipeline, that is 6 selects for 1 table row. When table has 20 rows, this means 6 * 20 = 120 selects every time table is refreshed (which is very often as this is triggered by any change in the database)

Same thing for executions - 5 checks --> 5 * 20 = 100 selects every time table is refreshed. (again very often)

For scheduler view, situation sligtly differs but still there are redundant selects.

@eea03
Copy link
Author

eea03 commented Nov 9, 2015

Partly solved in #547 - introduced caches to minimalize count of selects

@eea03
Copy link
Author

eea03 commented Nov 9, 2015

I would like to point out again that these refreshers that are supposed to refresh the tables are very ineficcent as they don't check if something relevant has changed, they just check if anything has changed in the database - therefore countless meaningless refreshes are done

@tomas-knap
Copy link

Pull request #548 partially solves that, BUT:

Caching is already available in the container loading the data from DB so this pull request is not needed. This pull request #548 adds yet another and different caching mechanism, which just complicates maintenance.

Also in 2.3.1, we decreased the number of entities being loaded from db, so for example, all permissions from permissions table are not loaded for each pipeline as the pipeline is listed. Nevertheless the issue described in #584 is still there.

So the pull request was not merged to 2.3.X, but the findings in the issue description here should be re-examined (taking into account #584 and other changes in 2.3.1)

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

No branches or pull requests

2 participants