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

Use single https session to retrieve all rock-on definitions #2707 #2708

Conversation

phillxnet
Copy link
Member

By sharing a single https session, instantiated on the first GET, we avoid major cumulative delays and reduce client/server resource overhead. Note that we still employ a dedicated https session to retrieve our 'index' file of 'root.json'.

Includes

  • Minor fstring adoption in modified area of code.
  • Info log of definitions retrieval and initial parsing time.
    Intended as a diagnostic aid, and to assist with field feedback on this process of indeterminate duration.
  • Individual file retrieval timeout reduced to 5 seconds.

Fixes #2707

…#2707

By sharing a single https session, instantiated on the first GET,
we avoid major cumulative delays and reduce client/server resource
overhead. Note that we still employ a dedicated https session to
retrieve our 'index' file of 'root.json'.

## Includes
- Minor fstring adoption in modified area of code.
- Info log of definitions retrieval and initial parsing time.
Intended as a diagnostic aid, and to assist with field
feedback on this process of indeterminate duration.
- Individual file retrieval timeout reduced to 5 seconds.
@phillxnet
Copy link
Member Author

Testing

In the associated issue there are a number of comment details the apparent improvement here, and a freshly built RPM (15.5 amd64 build host) with this patch in-place installs successfully and is able to acquire the initial Rock-on list, and then update twice there-after with the following new info log entries detailing the retrieval time:

[17/Oct/2023 17:37:13] INFO [storageadmin.views.rockon:505] Rock-on definitions retrieved in: 5.99 seconds.
[17/Oct/2023 17:37:33] INFO [storageadmin.views.rockon:505] Rock-on definitions retrieved in: 5.98 seconds.
[17/Oct/2023 17:37:49] INFO [storageadmin.views.rockon:505] Rock-on definitions retrieved in: 6.01 seconds.

With an overall API (/api/rockons/update) time similarly logged as follows.
N.B. we also have a newer gunicorn version using a more appropriate worker type in play here: see #2706

127.0.0.1 - - [17/Oct/2023:17:37:16 +0100] "POST /api/rockons/update HTTP/1.0" 200 0 "https://rleap15-5.lan/" "Mozilla/5.0 (X11; Linux x86_64; rv:109.0) Gecko/20100101 Firefox/115.0" 9441ms
127.0.0.1 - - [17/Oct/2023:17:37:36 +0100] "POST /api/rockons/update HTTP/1.0" 200 0 "https://rleap15-5.lan/" "Mozilla/5.0 (X11; Linux x86_64; rv:109.0) Gecko/20100101 Firefox/115.0" 9235ms
127.0.0.1 - - [17/Oct/2023:17:37:52 +0100] "POST /api/rockons/update HTTP/1.0" 200 0 "https://rleap15-5.lan/" "Mozilla/5.0 (X11; Linux x86_64; rv:109.0) Gecko/20100101 Firefox/115.0" 9244ms

@phillxnet
Copy link
Member Author

@Hooverdan96 Thanks for linking in that operationally orientated issue. Much appreciated. I created a new issue as this may not be the whole story, but I'm confident this and the referenced gunicorn update/worker change should at least help. Hopefully representing a fix for our occasionally referenced time-outs.

@FroggyFlox & @Hooverdan96 I'll go ahead and get this merged ready for our nearing next testing rpm release.

@phillxnet phillxnet merged commit c38d47e into rockstor:testing Oct 17, 2023
@phillxnet phillxnet deleted the 2707-Use-single-https-session-to-retrieve-all-rock-on-definitions branch October 18, 2023 16:41
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

1 participant