hubzero / hubzero-cms

Platform for Scientific Collaboration
https://hubzero.org
GNU General Public License v2.0
47 stars 57 forks source link

[NCN-789] Remove conversions from db calls #1707

Closed jsperhac closed 4 months ago

jsperhac commented 5 months ago

Summary

Tool session starts rely on checks of tool export controls and on the tool user's country and location. These checks are based on lookup of the user's IP address. There are inefficiencies in how these lookups are performed that may help expedite tool starts, if improved.

I propose improvements to these lookups based on common entries in the slow query log.

Motivation

Jira card: https://sdx-sdsc.atlassian.net/browse/NCN-789

Nanohub has recently had queries in the slow query log (from 40 to several hundred per day) that look like this:

SELECT LOWER(countrySHORT) FROM ipcountry WHERE ipFROM <= INET_ATON('193.x.y.z') AND ipTO >= INET_ATON('193.x.y.z');

(where 193.x.y.z represents a real IP address in dotted quad format) These logged queries were taking 5-6 sec to execute.

It is inefficient to use INET_ATON() calls in the database to convert a dotted-quad IP to a long (especially in the WHERE clause, as this invalidates use of the index). This conversion could be done in the PHP instead. Quick query benchmarking indicated we can save about a factor of 2 in time by making the conversion in the PHP and making this change to the query:

SELECT LOWER(countrySHORT) FROM ipcountry WHERE ipFROM <= <IP-as-LONG> AND ipTO >= <IP-as-LONG>;

I tracked the query down to libraries/Hubzero/Geocode/Geocode.php and found a couple of other places in the Geocode library code where we are relying on the database to do string conversions and/or IP conversions.

Places where the INET_ATON heavy library code was called include the com_tools controller that handles session management (core/components/com_tools/site/controllers/sessions.php) and the utility code that validates user permissions to run export controlled tools. Since nanohub has been experiencing slow tool starts, improving efficiencies there seems like a good move.

Changes made

PR Checklist