bneradt commented on code in PR #12291:
URL: https://github.com/apache/trafficserver/pull/12291#discussion_r2153212704


##########
tests/gold_tests/autest-site/ports.py:
##########
@@ -30,6 +31,72 @@
 g_ports = None  # ports we can use
 
 
+class AsyncPortQueue(OrderedSetQueue):
+
+    def __init__(self):
+        super().__init__()
+        self._listening_ports = _get_listening_ports()
+
+    async def select_available(self, amount, dmin, dmax):
+        rmin = dmin - 2000
+        rmax = 65536 - dmax
+
+        port_tasks = []
+        await asyncio.gather(*port_tasks)
+        if rmax > amount:
+            # Fill in ports, starting above the upper OS-usable port range.
+            port = dmax + 1
+            while port < 65536 and self.qsize() < amount:
+                port_tasks.append(self._check_port(port))
+                port += 1
+        if rmin > amount and self.qsize() < amount:
+            port = 2001
+            # Fill in more ports, starting at 2001, well above well known 
ports,
+            # and going up until the minimum port range used by the OS.
+            while port < dmin and self.qsize() < amount:
+                port_tasks.append(self._check_port(port))
+                port += 1
+
+        await asyncio.gather(*port_tasks)

Review Comment:
   What do you think of this? It iteratively adds tasks until we get the 
desired amount of free ports:
   
   ```patch
   ╰─➤  git diff HEAD
   diff --git a/tests/gold_tests/autest-site/ports.py 
b/tests/gold_tests/autest-site/ports.py
   index c5259634c..ed2943249 100644
   --- a/tests/gold_tests/autest-site/ports.py
   +++ b/tests/gold_tests/autest-site/ports.py
   @@ -17,7 +17,7 @@
    #  limitations under the License.
    
    import asyncio
   -from typing import Set
   +from typing import Generator, Set
    import socket
    import subprocess
    import os
   @@ -37,27 +37,45 @@ class AsyncPortQueue(OrderedSetQueue):
            super().__init__()
            self._listening_ports = _get_listening_ports()
    
   -    async def select_available(self, amount, dmin, dmax):
   -        rmin = dmin - 2000
   -        rmax = 65536 - dmax
   +    async def select_available(self, amount: int, dmin: int, dmax: int) -> 
None:
   +        '''Populate the port queue with ports that are not in use by the OS.
    
   -        port_tasks = []
   -        await asyncio.gather(*port_tasks)
   -        if rmax > amount:
   +        This method fills in the queue with at least `amount` ports that 
are not
   +        within `dmin` and `dmax`.
   +
   +        :param amount: The number of ports to populate the queue with.
   +        :param dmin: The minimum port number we expect that the OS uses.
   +        :param dmax: The maximum port number we expect that the OS uses.
   +        '''
   +        def task_generator(amount: int, dmin: int, dmax: int) -> Generator:
   +            task_counter = 0
                # Fill in ports, starting above the upper OS-usable port range.
                port = dmax + 1
   -            while port < 65536 and self.qsize() < amount:
   -                port_tasks.append(self._check_port(port))
   +            while port < 65536 and task_counter < amount:
   +                yield self._check_port(port)
                    port += 1
   -        if rmin > amount and self.qsize() < amount:
   -            port = 2001
   +                task_counter += 1
                # Fill in more ports, starting at 2001, well above well known 
ports,
                # and going up until the minimum port range used by the OS.
   +            port = 2001
                while port < dmin and self.qsize() < amount:
   -                port_tasks.append(self._check_port(port))
   +                yield self._check_port(port)
                    port += 1
   -
   -        await asyncio.gather(*port_tasks)
   +                task_counter += 1
   +
   +        tasks = task_generator(amount, dmin, dmax)
   +        while self.qsize() < amount:
   +            port_tasks = []
   +            ports_still_needed = amount - self.qsize()
   +            while len(port_tasks) < ports_still_needed:
   +                try:
   +                    port_tasks.append(next(tasks))
   +                except StopIteration:
   +                    # No more tasks to generate.
   +                    host.WriteWarning('Ran out of ports to check, stopping 
port queue setup.')
   +                    return
   +            host.WriteDebug('_setup_port_queue', f"Gathering 
{len(port_tasks)} port tasks out of {ports_still_needed} desired.")
   +            await asyncio.gather(*port_tasks)
    
        async def _check_port(self, port):
            if await self._is_port_open(port):
   ```
   
   With this change:
   
   ```
   ╰─➤  time ./autest.sh --sandbox /tmp/sb --clean=none -f emergency
   Running Test emergency:. Passed
   
   Generating Report: --------------
   Total of 1 test
     Unknown: 0
     Exception: 0
     Failed: 0
     Warning: 0
     Skipped: 0
     Passed: 1
   ./autest.sh --sandbox /tmp/sb --clean=none -f emergency  0.58s user 0.30s 
system 84% cpu 1.037 total
   ```
   
   0.58 seconds is still more than 0.41 seconds (the amount of time before any 
of the patches on this branch), but the value is so close and small no one will 
notice it. Can you check that it sill makes your system's check efficient?



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: github-unsubscr...@trafficserver.apache.org

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org

Reply via email to