Optimize Gnocchi aggregates API

[rafaelweingartner]

Gnocchi aggregates API can present slowness with time, due to the growing number of resources and revisions in large scale OpenStack Cloud environments. This happens due to the situation where Gnocchi does not apply filters in the MySQL queries in the resource tables. I mean, start/stop filters are not applied, and others as well, which makes Gnocchi to manipulate all dataset in the MySQL database and then the filtering is executed in Python.

To cope with that situation, we are proposing an optimization to improve the query executed by Gnocchi in MySQL, and apply the filtering right away; thus, the dataset manipulated by MySQL will be drastically reduced. After applying the patch, we noticed a reduction of 5-6 folds in the response time for the Gnocchi aggregates API.

[rafaelweingartner]

@tobias-urdin can you look into this one? It is a nice to have in Gnocchi.

[rafaelweingartner]

we need to merge this one first: https://github.com/gnocchixyz/gnocchi/pull/1308

[tobias-urdin]

@mergifyio rebase

[mergify[bot]]

rebase

❌ Unable to rebase: user tobias-urdin is unknown.

Please make sure `tobias-urdin` has logged in Mergify dashboard.

[tobias-urdin]

@Mergifyio rebase

[mergify[bot]]

rebase

✅ Branch has been successfully rebased

[rafaelweingartner]

Hello @chungg and @tobias-urdin I guess this patch is ready for your reviews again.

[tobias-urdin]

Hello @chungg and @tobias-urdin I guess this patch is ready for your reviews again.

I will check sometime this week but it's probably best to get @chungg review as well.

Please rebase on the now merged changes for SQLAlchemy 2.0 support, remove some duplicate commits that seems to be on here, squash commits and add some clear commit messages.

Thanks!

[rafaelweingartner]

@tobias-urdin and @chungg rebase and squash done!

[rafaelweingartner]

Hello guys, Do we need some other amendments in this patch?

[rafaelweingartner]

Thank you @chungg for your reviews! I have extract the code you suggested to a new PR at https://github.com/gnocchixyz/gnocchi/pull/1361.

Besides that, are we missing something else here?

[rafaelweingartner]

@tobias-urdin thank you for merging the patch that fixed the pipeline, and the other PR that was derived from this one. Now, we just need to work on this PR, if some extra change is required.

[rafaelweingartner]

@chungg and @tobias-urdin I have rebased the patch. Can you guys check it?

[rafaelweingartner]

Hello @tobias-urdin, @chungg, @jd do you guys think that we need to do some more adjusts in the code, so it can get merged?

[jd]

There is no way I could review this TBH, it's been so long. I'm ok to approve and merge it though as I don't want to block any improvement. Anyone else wants to weight it before I do it?

[rafaelweingartner]

There is no way I could review this TBH, it's been so long. I'm ok to approve and merge it though as I don't want to block any improvement. Anyone else wants to weight it before I do it?

Thanks!

BTW, we have been constantly improving Gnocchi for years so far, and we have more coming. Would it be possible for one of us to be come a committer with merge permissions in the project here?

[jd]

There is no way I could review this TBH, it's been so long. I'm ok to approve and merge it though as I don't want to block any improvement. Anyone else wants to weight it before I do it?

Thanks!

BTW, we have been constantly improving Gnocchi for years so far, and we have more coming. Would it be possible for one of us to be come a committer with merge permissions in the project here?

I give you a soft-yes and will let @tobias-urdin and others interested to weigh in.

[rafaelweingartner]

I don't have the time to go through this in detail or test it, but just browsing through it and the fact that the testing is green (hoping that most things are tested that this touches) makes me OK to just merge.

I do however have some remarks.

* The commit messages is very undescriptive and it looks like this PR is addressing more things in this PR than just the optimization itself

* The revision_start -> search_start etc, why was that done can you explain? Did we merge broken code with the group history PR before? If so, why is that not in a separate PR and bulked into this PR that is unrelated to the optimization itself.

* This adds a lot of LOG.debug calls, in pretty much the entire code path, I understand that might be good for debugging but is it really neccesary that we merge all that?

Overall it's kind of hard to understand what parts of this change is just cleanup in formatting, what fixes bugs, I would hope that would be cleaner in the future.

I'm all for giving our more access, my primary goal is still maintenance and keeping the project alive, especially to make sure it doesn't block anything in the OpenStack ecosystem so the project doesn't get dropped from there (keeping up with things such as the SQLAlchemy 2.0 migration).

I do really appreciate that you're taking the effort to address larger flaws in Gnocchi, like this one, I didn't even know about it until this PR, but I would hope that a high standard in commits is still kept even if the project is in maintenance mode.

Let me start by saying that Gnocchi is very convoluted and quite complicated code-wise. The idea is simple, but the way it was coded, makes it very difficult to understand what is being done, and how to fix the issues we find or the extensions that need to be developed. We are trying to address that by adding logs, changing/creating new functions with more descriptive names, and so on. However, it is a task that is going to take much more time until we reach a state where things get better.

Regarding the addition of logs, it goes towards that situation. It is very difficult to troubleshoot Gnocchi. Therefore, the addition of those logs will help people to work with Gnocchi in the future. Furthermore, they (the logs) do not cause any harm. You can use it in INFO log level (debug=false), and then you will not see those DEBUG logs.

Those flaws that nobody knows exist that we are fixing/patching come due to the fact that most OpenStack setups are not taking full advantage of Gnocchi (as far as we have seen). We are only facing them because we have a situation where we are putting pressure on Gnocchi to the limit without cleaning/removing things (the setup) every upgrade with a newer installation and so on. Moreover, we have a generic billing pipeline that receives data not just from OpenStack, but other systems that compose the cloud.

And, regarding the description. It is clear for people using Gnocchi that have some understanding of its inner workings. Every metric pushed to Gnocchi is created for a resource. Every time we create a resource, we have the create_at field, that is marked, and every time we update it, we have a revision created for it. When we do a search, via the Gnocchi search API, before this patch, the search was going through all resources in the system. However, that is costly. We can filter the resources affected by the search using the created_at/revision_start ended_at/revision_end fields when they are provided by the user calling the API. This is the optimization that we are doing here. Gnocchi was filtering the results in Python, and loading all measure, for all resources for all timestamps. What we propose is to filter the result-set right away in the DB.

Regarding the quality standard, can you describe/show us what needs to be improved? Then, we can work to address it and make the standard to be in the same level as you expect it to be.

[rafaelweingartner]

There is no way I could review this TBH, it's been so long. I'm ok to approve and merge it though as I don't want to block any improvement. Anyone else wants to weight it before I do it?

Thanks! BTW, we have been constantly improving Gnocchi for years so far, and we have more coming. Would it be possible for one of us to be come a committer with merge permissions in the project here?

I give you a soft-yes and will let @tobias-urdin and others interested to weigh in.

I do not know what that means =). However, even if we do not deserve a committer role. We just need to understand what are the requirements before a merge can be done.

We have more feature and optimizations to push to Gnocchi, and sometimes the time it takes to get them in is quite long.

[jd]

I think the root problem is that the project is barely maintained at this stage. All the people who worked on Gnocchi years ago and wrote most of that code are gone in the wild. This means nobody is smart enough or has the time to review all that code, especially because it's a huge change that seems (I did not dig through it) to address multiple problems at the same time.

I'm not concerned by the project's fate, maintenance, etc, at this stage, so I'm not going to block anything. As I said, I'm giving a soft yes meaning "I think it's better to have this merged because you seem to care about the project more than anyone else, but also, I'm not smart enough to say yay that works let's merge it."

[rafaelweingartner]

I think the root problem is that the project is barely maintained at this stage. All the people who worked on Gnocchi years ago and wrote most of that code are gone in the wild. This means nobody is smart enough or has the time to review all that code, especially because it's a huge change that seems (I did not dig through it) to address multiple problems at the same time.

I'm not concerned by the project's fate, maintenance, etc, at this stage, so I'm not going to block anything. As I said, I'm giving a soft yes meaning "I think it's better to have this merged because you seem to care about the project more than anyone else, but also, I'm not smart enough to say yay that works let's merge it."

Thanks for the message! I understand what you mean. We are trying to reduce the scope for every new PR here. Gnocchi is a very interesting system. I mean, it has an interesting design and idea/concept, that we see a lot of potential. However, code-wise it has problems that we are working to address and to improve.

[rafaelweingartner]

Hello guys, Do you want us to execute some other changes in this PR before it is merged?

[rafaelweingartner]

I was revisiting this PR, and the only file that was changed, and is not directly related to the patch proposed here is the following:

• doc/source/rest.j2.

I can extract that is needed, but it was an opportunity to improve a documentation, and we just did it. All of the rest is related to the changeset proposed. All of the logs were needed/helped us to understand the situation, and they are being added in DEBUG log level. Therefore, if one does not want them, it is a matter of disabling DEBUG.

[jd]

Yes if you have changes that can be extracted, do this, that'll help for sure!

[rafaelweingartner]

Yes if you have changes that can be extracted, do this, that'll help for sure!

Done. The documentation improvement was extracted to: https://github.com/gnocchixyz/gnocchi/pull/1373

[rafaelweingartner]

Hello guys, can we merge this one? Or, do you think that we need some further improvements here?

The part of the documentation was already extracted to https://github.com/gnocchixyz/gnocchi/pull/1373.

[rafaelweingartner]

As said, I'm ok with that since I don't think it'd be fair to block it just on the basis we don't have time to review it.

Thanks for your support! If there is anything else missing here, please let me know. We have more patches to open for Gnocchi, but we are doing one by one, to avoid managing/dealing with conflicts in multiple fronts.