Removed progress method and its indicator from SB API. (#701)

Removes progress() and supportsProgTh() from SB API. They can be still used within the plugin.

As ucx and gds_mt are already using multi-threads to deliver performance, idea of shared progress thread became less useful. Shared progress thread can also cause contention between the backends, while we want the isolation in modularity as much as possible. So team decided to remove it from the SB API. Plugins internally can still use it, such as current ucx implementation, just not a mandatory method in SB API anymore.

Author: mkhazraee

docs/BackendGuide.md

@@ -51,12 +51,10 @@ The key/value parameters are a map of strings to byte arrays that are passed fro
 * supportsLocal(): Indicates if the backend supports transfers within a node
 * supportsRemote(): Indicates if the backend supports transfers across nodes
 * supportsNotif(): Indicates if the backend supports notifications
-* supportsProgressThread(): Indicates if the backend supports progress() method. That method should call the underlying procedure of progressing transfers for this backend.
 * getSupportedMems(): Indicates memory types supported by the backend
 
-Based on the first 4 methods (supports*), the required methods to be implemented change. For instance, UCX backend implements all as it supports all scenarios, while GDS backend only has supportsLocal, detailed more in Example implementations. Note that a network backend should have supportsRemote and supportsNotif to be set to true, and preferably supportsLocal also to true, so another backend doesn’t need to be involved for local transfers. For a storage backend, it should have supportsLocal and supportsNotif is optional. supportsProgressThread is optional for both cases. Additionally, a backend that supportsRemote must also support supportNotifs.
+Based on the first 3 methods (supports*), the required methods to be implemented change. For instance, UCX backend implements all as it supports all scenarios, while GDS backend only has supportsLocal, detailed more in Example implementations. Note that a network backend must have supportsRemote and supportsNotif to be set to true, and preferably supportsLocal also to true, so another backend doesn't need to be involved for local transfers. For a storage backend, it should have supportsLocal and supportsNotif is optional.
 
-Note that supportProgressThread is an indicator whether a backend has implemented the progress() method, but does not imply how the progress thread is implemented. During creation of a backend, the provided init params indicate how the progress thread is intended to be used. For instance, if the enablement of progress thread is set to false, while a backend cannot work without a separate progress thread, the backend creation would fail. This flag is useful for the NIXL agent if we want to provide some agent level guarantees, such as minimum time between calls to progress for backends, or if a central progress method is implemented (for future proofing, not currently implemented).
 ### Connection Management:
 
 * connect(): Initiates connection to a remote agent.
@@ -106,12 +104,6 @@ Finally, note that a call to releaseXferReq should not block and be asynchronous
 
 Note that getNotif does not know which agent it should look for to receive the notification. So there should be a method to extract the agent name from the notification received, corresponding to a transfer. genNotif generates a notification which is not bound to any transfers, and does not provide any ordering guarantees. If a backend does not set supportsNotifications, these two methods are not needed.
 
-### Progress Thread:
-
-* progress(): Makes progress on transfers and notifications.
-
-If a backend requires a progress call, such as UCX, to proceed with the transfers, for both check of transfer status or received notification, they can implement a progress thread, and a frequency of waking up that thread will be passed during backend creation. In addition, each time a user calls to check a transfer status, or check received notifications, this method is called, enabling progress if a progress thread is not implemented.
-
 ## Descriptor List Abstraction
 
 A key underlying abstraction for NIXL library is a descriptor list, that is made of a memory space (host/GPU/block/File/Obj-Store) and a list of descriptors. There are 2 types of descriptors used for the SB API.
@@ -142,9 +134,9 @@ The plugin manager maintains API versioning of these above APIs. This can allow
 
 ## Comparing two plugins as an example
 
-NIXL UCX plugin provides networking across different nodes, while GDS plugin provides storage access. Moreover, UCX plugin sets all of the “supports” flags, while GDS only has the supportsLocal flag set. The reason being UCX requires a progress thread and provides notifications, and can do transfers within an Agent, for instance from GPU to CPU, and across Agents. Therefore, it should implement all of the methods mentioned previously.
+NIXL UCX plugin provides networking across different nodes, while GDS plugin provides storage access. UCX plugin sets all of the “supports” flags, while GDS only has the supportsLocal flag set. The reason being UCX is a network plugin that should support inter-agent communication and notifications, and it also supports intra-agent transfers, for instance from GPU to CPU.
 
-However, for NIXL storage backends, there is no need to run a NIXL agent on a remote storage node. Instead, a distributed storage client on the local agent talks to the remote distributed storage, and therefore from NIXL agent point of view for all storage, whether local or remote, it has to talk to this local storage client. In other words, all the transfers are loopback to the agent itself. For the current use case, there is no need for notifications within the same agent, or a progress thread either.
+However, for NIXL storage backends, there is no need to run a NIXL agent on a remote storage node. Instead, a distributed storage client on the local agent talks to the remote distributed storage, and therefore from NIXL agent point of view for all storage, whether local or remote, it has to talk to this local storage client. In other words, all the transfers are loopback to the agent itself. For the current use case, there is no need for notifications within the same agent.
 
 Moreover, the GDS plugin does not require a local connection to itself, so it returns SUCCESS for connect and disconnect, and for loadLocal simply returns back the input pointer as its output. The only 6 remaining methods that it has to implement are:
 
@@ -213,20 +205,20 @@ Note that inside a transfer, a backend might provide methods for network resilie
 
 ### Get transfer status:
 
-The agent will call the backend specific transfer handle that is stored within the agent transfer handle, and check the status of the transfer. This is achieved through a call to **checkXfer** in the SB API. Internal to the backend, they can call the **progress** method in SB API, if that’s necessary to get the latest status of the transfers. If the agent is run in progress thread mode, the agent will call that periodically, and therefore reduce the load on this internal call.
+The agent will call the backend specific transfer handle that is stored within the agent transfer handle, and check the status of the transfer. This is achieved through a call to **checkXfer** in the SB API. Internal to the backend, they can call their internal progress method, if that’s necessary to get the latest status of the transfers.
 
 ### Invalidate transfer request:
 
 The agent will call the **releaseReqH** from the SB API on the backend specific transfer handle to release it, and potentially abort the transfer if in progress and the backend has the capability. Then the agent will release the other resources within the agent level transfer handle to fully release it.
 
 ### Get notifications:
 
-The agent will iterate over all the backends that support notification, and call their **getNotifs** from the SB API, which will return a list of notifications received from each remote node between the previous call to this method and this time. Then the agent will merge the results from all such backends, and append them to the map that the user has provided. Similar to get transfer status, Internal to the backend, they can call the **progress** method in SB API, if that’s necessary to get the latest notifications received from the transfers initiated by the other agents towards them. If the agent is run in progress thread mode, the agent will call that periodically, and therefore reduce the load on this internal call.
+The agent will iterate over all the backends that support notification, and call their **getNotifs** from the SB API, which will return a list of notifications received from each remote node between the previous call to this method and this time. Then the agent will merge the results from all such backends, and append them to the map that the user has provided. Similar to get transfer status, Internal to the backend, they can call their internal progress method, if that’s necessary to get the latest notifications received from the transfers initiated by the other agents towards them.
 
 ### Generate notification:
 
 If a backend is provided by the user, the agent will call **genNotif** from the SB API of that backend engine. Otherwise, it will look for a backend that is available locally and remotely and also supports notifications. If more than one candidate is found, it will choose the first one, or use a preference list.
 
 ### Destructor:
 
-When an agent is getting destroyed at the end of the application, it will deregister all the remaining memories that were not deregistered by the application (bad practice, but agent takes care of it). Then for each of the backends it will call their **destructor** from the SB API, and finally do the rest of internal clean up.
+When an agent is getting destroyed at the end of the application, it will deregister all the remaining memories that were not deregistered by the application (bad practice, but agent takes care of it). Then for each of the backends it will call their **destructor** from the SB API, and finally do the rest of internal clean up.

src/api/cpp/backend/backend_engine.h

@@ -112,9 +112,6 @@ class nixlBackendEngine {
         // pure virtual, and return errors, as parent shouldn't call if supportsNotif is false.
         virtual bool supportsNotif() const = 0;
 
-        // Determines if a backend supports progress thread.
-        virtual bool supportsProgTh() const = 0;
-
         virtual nixl_mem_list_t getSupportedMems() const = 0;  // TODO: Return by const-reference and mark noexcept?
 
 
@@ -209,14 +206,6 @@ class nixlBackendEngine {
         }
 
 
-        // *** Needs to be implemented if supportsProgTh() is true *** //
-
-        // Force backend engine worker to progress.
-        virtual int
-        progress() {
-            return 0;
-        }
-
         // *** Optional virtual methods that are good to be implemented in any backend *** //
 
         // Query information about a list of memory/storage

src/core/agent_data.h

@@ -130,7 +130,6 @@ class nixlBackendH {
         bool supportsRemote () const { return engine->supportsRemote(); }
         bool supportsLocal  () const { return engine->supportsLocal (); }
         bool supportsNotif  () const { return engine->supportsNotif (); }
-        bool supportsProgTh () const { return engine->supportsProgTh(); }
 
     friend class nixlAgentData;
     friend class nixlAgent;

src/plugins/cuda_gds/gds_backend.h

@@ -121,9 +121,6 @@ class nixlGdsEngine : public nixlBackendEngine {
         bool supportsLocal() const {
             return true;
         }
-        bool supportsProgTh() const {
-            return false;
-        }
 
         nixl_mem_list_t getSupportedMems() const {
             nixl_mem_list_t mems;

src/plugins/gds_mt/gds_mt_backend.h

@@ -52,10 +52,6 @@ class nixlGdsMtEngine : public nixlBackendEngine {
     supportsLocal() const override {
         return true;
     }
-    bool
-    supportsProgTh() const override {
-        return false;
-    }
 
     nixl_mem_list_t
     getSupportedMems() const override {

src/plugins/gpunetio/gpunetio_backend.h

@@ -46,9 +46,6 @@ class nixlDocaEngine : public nixlBackendEngine {
     bool supportsNotif() const {
         return true;
     }
-    bool supportsProgTh() const {
-        return false;
-    }
 
     nixl_mem_list_t
     getSupportedMems() const;

src/plugins/hf3fs/hf3fs_backend.h

@@ -138,9 +138,6 @@ class nixlHf3fsEngine : public nixlBackendEngine {
         bool supportsLocal   () const {
             return true;
         }
-        bool supportsProgTh  () const {
-            return false;
-        }
 
         nixl_mem_list_t getSupportedMems () const {
             nixl_mem_list_t mems;

src/plugins/mooncake/mooncake_backend.h

@@ -53,11 +53,6 @@ class nixlMooncakeEngine : public nixlBackendEngine {
         return true;
     }
 
-    bool
-    supportsProgTh() const {
-        return false;
-    }
-
     nixl_mem_list_t
     getSupportedMems() const;
 

src/plugins/obj/obj_backend.h

@@ -46,11 +46,6 @@ class nixlObjEngine : public nixlBackendEngine {
         return false;
     }
 
-    bool
-    supportsProgTh() const override {
-        return false;
-    }
-
     nixl_mem_list_t
     getSupportedMems() const override {
         return {OBJ_SEG, DRAM_SEG};

src/plugins/posix/posix_backend.h

@@ -81,10 +81,6 @@ class nixlPosixEngine : public nixlBackendEngine {
         return false;
     }
 
-    bool supportsProgTh() const override {
-        return false;
-    }
-
     nixl_mem_list_t getSupportedMems() const override {
         return {FILE_SEG, DRAM_SEG};
     }