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

/api/v3/me endpoint is painfully slow #13953

Closed
Algunenano opened this issue May 14, 2018 · 14 comments
Closed

/api/v3/me endpoint is painfully slow #13953

Algunenano opened this issue May 14, 2018 · 14 comments
Labels

Comments

@Algunenano
Copy link
Contributor

Algunenano commented May 14, 2018

Context

When entering carto.com, loading the Dashboard information is really slow. With my team account, the api/v3/me endpoint takes ~1.5 seconds to load.

Additional info

Checking the load times in Kibana you can see load times of up to 5 seconds and you can see that team accounts take more or less the same time, so the load time might grow with the team size (unchecked supposition).

@ramiroaznar Can you check if this can be added to RT, please?

@juanignaciosl
Copy link
Contributor

cc @CartoDB/rt-managers

@javitonino
Copy link
Contributor

Did a quick benchmark and this is largely due to db_size_in_bytes. If we can avoid it, it would be a quick win. Maybe frontend knows if that field is being used, and we can skip it in the /me presenter.

@ethervoid
Copy link
Contributor

@rubenmoya for awareness

@rubenmoya
Copy link
Contributor

It's being used in the settings dropdown:
screen shot 2018-08-09 at 12 48 46

In the account page:
screen shot 2018-08-09 at 12 49 28

And in the organization page (I don't have a screenshot)

@juanignaciosl
Copy link
Contributor

This is not a big problem for normal accounts, but big ones are painful. Team takes ~1 minute now.

cc @CartoDB/rt-managers

@inigomedina
Copy link

We are in the process of changing that profile dropdown, as it is already implemented at carto.com

screenshot-carto com-2018 08 20-11-04-40

This new profile dropdown doesn't include the db size information. I don't know if we have any plan to implement this on the account side, but it would be a good first step to alleviate that endpoint now it's being used actively.

@javitonino
Copy link
Contributor

This problem seems to come and go depending on DB load. Some days it's better than others.

The dropdown inside dashboard does include this information though. Unless we are removing that, we still need this.

Things to investigate:

  • See why the function (CDB_UserDataSize in the extension) is so slow. It should be that bad, given that it's just checking postgres metadata.
  • Caching. We already have a cache of DB size used for reporting size to superadmin, but it's not used here. Maybe it makes sense? Since this is used for quota checking in the UI (blocking the import of new tables) we have to be careful and maybe add invalidation which can get messy.

I'd start with the first one before considering caching.

@rafatower
Copy link
Contributor

The main problem seems to be the functionCDB_UserDataSize() and the locks in the DB, as @javitonino pointed out. Just a few examples from the DB logs:

LOG:  duration: 81944.602 ms  statement: SELECT cartodb.CDB_UserDataSize('stephaniemongon')

Processing synchronizations since 1534756026 [Re LOG:  duration: 95376.979 ms  statement: SELECT cartodb.CDB_UserDataSize('aromeu')
Processing synchronizations since 1534756027 [Re LOG:  duration: 96716.983 ms  statement: SELECT cartodb.CDB_UserDataSize('aromeu')
Processing synchronizations since 1534756027 [Re LOG:  duration: 95993.316 ms  statement: SELECT cartodb.CDB_UserDataSize('aromeu')

Processing synchronizations since 1534756507 [Re LOG:  duration: 73674.724 ms  statement: SELECT cartodb.CDB_UserDataSize('fernando-carto')
Processing synchronizations since 1534756507 [Re LOG:  duration: 73795.507 ms  statement: SELECT cartodb.CDB_UserDataSize('fernando-carto')

Many of those seem to come from syncs.

Here's a ticket with a relevant discussion about DDL and locks that may affect CDB_UserDataSize(): #12829 (the good parts are to be extracted)

@inigomedina
Copy link

inigomedina commented Aug 20, 2018

The dropdown inside dashboard does include this information though. Unless we are removing that, we still need this.

My point is that. As said, we have already changed that dropdown outside dashboard. It makes sense to make it coherent and consistent by having just one profile dropdown.

The design team has involved into the new profile dropdown, probably @piensaenpixel can help us with this. In my opinion, the perfect solution, small change with a huge impact, would be to change that dropdown.

Of course, we can investigate why the function is so slow from time to time, or add cache, but both mean, as you say, efforts and in the case of the cache probably further problems.

@javitonino
Copy link
Contributor

It's not only that dropdown though. We currently also check that quota in order to enable/disable the import button and on the profile/organization pages, which are also UI (interactive). We also use it in imports/syncs but that is lees of an issue since it's a background job.

@rafatower
Copy link
Contributor

Furthermore: we also check the quota for the COPY endpoint. That's a good reason to try to fix the issue at exension/db level.

@inigomedina
Copy link

Yes, I understand that part: we give that calculation in different places.

My argument is that one of those places, the profile dropdown, is actually being changed. And the way it's being changed makes sense as it now displays just links to the main sections of the account instead of providing "real time" information of any aspect of the service. So, regardless of the problem we're facing now the profile dropdown should be updated on the account side as well removing that db size information. That would alleviate the problem a lot since the dashboard is a more common entry point than for instance the account page.

@javitonino
Copy link
Contributor

That would alleviate the problem a lot since the dashboard is a more common entry point than for instance the account page.

Just doing that won't alleviate the problem without more changes. We cannot remove it from that endpoint since itr's used in other parts of the frontend, and that endpoint must contain anything that is needed for frontend (since it's the only way front statics know how to get information from backend). So, even if it's used only in profile pages, we still need it in that endpoint. Also, in the dashboard, there is also (at least) the enable/disabled import button, so that dropdown is not the only place.

We would have to remove or change all usages of that from the frontend before we can remove it from that endpoint. My point is that it's probably not such a quick fix as it looks like.

@javitonino
Copy link
Contributor

Addressing this as part of a broader review of dashboard performance. #14615

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

No branches or pull requests

8 participants