mailcow / mailcow-dockerized

mailcow: dockerized - 🐮 + 🐋 = 💕
https://mailcow.email
GNU General Public License v3.0
8.26k stars 1.12k forks source link

Fix blocking last logins fetching #5880

Closed PierrePlt closed 3 weeks ago

PierrePlt commented 1 month ago

(Probably)Fixes https://github.com/mailcow/mailcow-dockerized/issues/5645

The problem was that fetching the last logins source IP's country of origin can sometime take a LONG time (up to a few minutes depending on the speed of the requests to dfdata.bella.network).

When the country <-> IP relationship isn't cached, this can cause the /user page to take from a few seconds to a few minutes to load for users with a long connection history.

This was caused by a variable declared in headers.inc.php that was never used. Removing this variable solves the issue with loading the /user page but when changing tab the user would still be confronted by a long loading time since the background request to api/v1/get/last-login would block the loading of other pages.

The second fix is to only load the last logins when the recent logins menu is loaded.

Not sure if this is the issue the original author had, but this fixes the issue discussed on https://github.com/mailcow/mailcow-dockerized/issues/5645 on the last few days

Known issue: When IP_SHORTCOUNTRY is not cached, opening the /user page, sliding down until the recent logins section is visible, and then opening another tab needing to make an API request will confront the user with a longer than usual waiting time since the request made by the second tab will be blocked by the request to api/v1/get/last-login

mkuron commented 1 month ago

Thanks. It does seem like eliminating the bella.network requests significantly speeds up the login.

T0biii commented 1 month ago

https://dfdata.bella.network/:

[09.10.2022] Added new API endpoint only for flag lookup (/country/thomas.bella.network)

maybe we should use also use the country Endpoint, or do we need the other infos? The Json Output of country is a bit smaller and different than for lookup

country (1,2 kB):

image

lookup (1,5 kB):

image
PierrePlt commented 1 month ago

I tried the endpoint, it doesn't seem to work with IPv6 sadly...\ Regarding querying speed, it also doesn't seem to be a lot faster than the standard endpoint, it's a shame because yes as far as I know we only need shortcountry.

T0biii commented 1 month ago

maybe @Thomas2500 can say something about IPv6 and maybe the speed?

Thomas2500 commented 1 month ago

Hello @T0biii and @PierrePlt! Thanks for the hint. The parser in the backend had an error where an IPv6 address was interpreted as an IPv4 address and thus too strongly shortened. So every IPv6 within the address space ::0 to ::fffe:ffff:ffff was found. The error has now been fixed and IPv6 addresses should now be displayed correctly.

Regarding the speed of the API: The old API path https://dfdata.bella.network/lookup/ uses the old method to retrieve data, which includes many additional lookups such as DNS resolution, rDNS, ASN lookup and other background checks. The new method with https://dfdata.bella.network/country/ only resolves the country and skips all other checks. In tests, the query speed was reduced from approx. 700ms to 100ms. The change should be particularly noticeable for queries like this one. I have just created a pull request #5886.

DerLinkman commented 1 month ago

Will fixed within next release. Thanks to @Thomas2500

PierrePlt commented 3 weeks ago

It most likely won't be fixed in the next release.\ @Thomas2500's fix allows for an increase in performance when fetching IP location.\ This PR removes the unnecessary fetching of all the locations when loading the /user page.