wait_for_tasks: use property collector to monitor multiple tasks at once

[leust]

The current wait_for_task implementation polls each task individually by creating separate SOAP requests repeatedly. This is inefficient when a service manages many concurrent tasks, as each poll generates CPU load and network traffic on both the client and vCenter server.

This change implements a centralized property collector that uses VMware's WaitForUpdatesEx API to monitor multiple tasks efficiently over a single persistent socket connection.

• Runs a background thread that manages the property collector and processes task updates asynchronously
• Uses WaitForUpdatesEx to wait for task state changes instead of polling, reducing SOAP requests.
• Maintains 1 TaskFilter per task (create/delete).
• Automatically recreates the property collector on errors

The feature is opt-in via the use_property_collector_for_tasks constructor parameter (default: False).

The wait_for_task() method signature remains unchanged, making the optimization transparent to callers.

Change-Id: Iebffb74a53a7fa8d679daeb486b844fdea5b5113

[fwiesel]

Couple of things:

I'd avoid eventlet primitives and go straight for the threading ones. Eventlet will hopefully gone soon.

I've added some small corrections, but I have also a larger problem with how much context (functions and variables) are shared between the threads.

In my mind, it should be sufficient to have one synchronized queue to communicate with the thread. I'd setup both of them in __init__ and stop the thread in __del__.
While Nova does call logout() before terminating (and only then), Cinder doesn't.

As you need to create the property filters in the monitoring thread with _recreate_property_collector, I'd move the whole creation (and deletion) of the filters into that thread and as many variables as possible to local variables of the function

So, something along the lines of:

class VMwareAPISession(object):
    def _wait_for_task_with_property_collector(self, task):
      ctx = context.get_current()
      future = futures.Future()
      task_value = vim_util.get_moref_value(task)
      self._pending_tasks.put((task_value, future, ctx))
      return future.result()

   def __del__(self):
      self._pending_tasks.shutdown(immediate=True)

    def _property_collector_loop(self):
        property_collector = ...
        property_collector_version = ""
        # Maps task moref value -> (future, context, filter) for running tasks
        running_tasks = {}
        ...
                if pending_task is not None:
                    task_value, future, ctx = pending_task
                    task_filter = self._create_task_filter(task_value)
                    self._running_tasks[task_value] = (future, ctx,
                                                       task_filter)
                    LOG.debug("Received task %s, moved to running.",
                              task_value)
        ...

[fwiesel]

Here, I'd go with

self._property_collector_thread = threading.Thread(target=self._property_collector_loop)
self._property_collector_thread.start()

[fwiesel]

If I see it correctly _start_property_collector_thread is only called through _create_session, which itself is using a lock, so we can't have concurrent calls of _start_property_collector_thread.

I see that the corresponding logout() which calls _stop_property_collector_thread is not taking that same lock, which doesn't sit right with me.

I'd rather have the same lock being taken in _create_session and logout() to ensure that both the session as well the thread will be closed atomically.

It didn't make a difference, as long python wasn't really multi-threaded.

[fwiesel]

Why no try/except here, but above?

[leust]

@fwiesel may I have your 2nd review on this? Thanks.

[fwiesel]

You've addressed the rather cosmetic changes, but not my main issue, the part that comes after:

but I have also a larger problem with how much context (functions and variables) are shared between the threads.