Skip to content

Conversation

@arifer612
Copy link
Contributor

In this PR, I propose the use of the docker label net.unraid.docker.webui as
the primary source when parsing the container's web UI address. In the case that
the label is missing (i.e. for containers that were created before upgrading to
OS 6.10.X), we will fallback to the retrieving the details from the XML template.

There are 2 reasons for proposing this PR:

  1. Compatibility with docker-compose. The current set up only explicitly
    refers to the XML template for the web UI address. This rules out all
    Compose files that are spun up that do not reference a container that were
    once spun up before using a template. Thus, no web UI option will be provided
    for such containers.
  2. Speed when loading the Docker page. The web UI address is parsed by
    retrieving the address format from the XML template in the private
    function getControlURL. This is called within the public function
    getAllInfo. However, before calling getControlURL, we would already have
    read through the container's labels and assigned net.unraid.docker.webui to
    a temporary variable. For all recently created containers with defined web UI
    addresses, this variable will be non-empty and, if spun up using a template,
    is the same as the template value obtained from the XML template. It would be
    much faster to just use the label's value instead of calling another function
    that furnishes the same value after an I/O operation.

On my server with about 60 running containers, I have measured a drop from 9.5s
to 9.0s, over an average of 10 tests, with and without applying the changes of
this PR. I only have 1 server and it is running an i7-8770. The difference would
not be as pronounced with faster CPUs but it should still be present. I believe
that this result shows some support in my claims in reason #2 above.

This commit makes the 'net.unraid.docker.webui' docker label the primary source
when parsing the web UI address. If the docker label is missing, the template
value will be used instead.
@bergware
Copy link
Contributor

Nice improvement, thanks!

@arifer612
Copy link
Contributor Author

Thank you! I have just realised that I missed one line that also calls for the
template file value instead of referencing the Docker label. With the last
fixup, I have managed to reduce the time by another 0.5s to 8.5s on average. I
have not observed any issues, but please do point out if this PR causes any
unexpected errors!

@bergware
Copy link
Contributor

I made some tests and it looks alright, thanks.

@limetech limetech merged commit 1320bc3 into unraid:master Jun 27, 2022
@limetech
Copy link
Contributor

Yes, thank you!

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.

3 participants