Multiple NodeShared per process

[AmalDevHaridevan]

🎉 New feature

Closes #531

Summary

Currently the transport architecture restricts a single NodeShared object per process. This is achieved by mapping process id to an instance of NodeShared class. However, recently, I had some use cases where I need to isolate several nodes, each with their own DISCOVERY variables, such as GZ_DISCOVERY_MSG_PORT,GZ_DISCOVERY_SRV_PORT, GZ_DISCOVERY_MULTICAST_IP,GZ_PARTITION, in a single process. Due to the limitation mentioned above, this is not possible because
DISCOVERY is tied to a process, and when instantiating multiple Nodes per process, all of them obtain the same NodeShared object and hence the same DISCOVERY variables.

To overcome this, one approach is to also use a thread_id to create a unique identifier that maps to a NodeShared object. This pins down the uniqueness of NodeShared object to each thread, not the process. The proposed modification performs this by the following logic:

// Map for obtaining shared node
static std::unordered_map<std::string, NodeShared*> nodeSharedMap;


 auto pid = getProcessId();
  // Get current thread ID, so we can create multiple NodeShared per Process, per thread
  auto tid = std::this_thread::get_id();
  // Create a unique id using both pid and tid
  std::stringstream ss;
  ss << tid;
  std::stringstream unique_ss;
  unique_ss << std::hex << pid << "-" << ss.str();
  auto uid = unique_ss.str();

 // create or access the shared node
 nodeSharedMap.find(uid) ; //access
// OR
 nodeSharedMap.insert(uid, new NodeShared) ; //create

This allows specification of DISCOVERY variables per thread.
 

Test it

For testing the proposed improvement, I added two test files nodeDiscoveryPubs_TEST.py and nodeDiscoverySubs_TEST.py under python/test directory. Each one shall be run in a separate terminal before and after modifications to see the results. Before modification, it is not possible to receive the messages by the subscribers due to the issue I mentioned. However, after my modification, the subscribers are able to receive their messages.
Finally, the proposed modification can also be verified from CLI. When the publisher TEST is run, it prints the command that can be run in a shell to set the DISCOVERY vars to listen to their publications.

NOTE: The following tests failed:
NodeTest.ScopeProcess and NodeTest.ScopeProcessRemap, because the test runs by creating publisher and subscriber in different threads. Due to the proposed modification, these 2 tests will fail.

Checklist

• Signed all commits for DCO
• Added tests
• Added example and/or tutorial
• Updated documentation (as needed)
• Updated migration guide (as needed)
• Consider updating Python bindings (if the library has them)
• codecheck passed (See contributing)
• All tests passed (See test coverage)
• While waiting for a review on your PR, please help review another open pull request to support the maintainers

Note to maintainers: Remember to use Squash-Merge and edit the commit message to match the pull request summary while retaining Signed-off-by messages.

🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸

[AmalDevHaridevan]

Hi Azeey,

I found that there is an alternate approach to my use case. I feel like implementing my solution requires a lot of modifications, and after testing it, even though it works, it feels like I modified some fundamental architecture. Firstly, I do not know why the architecture was used in the first place, so modifying it may come with consequences.
So I decided to settle with an IPC solution to my problem (even though its much slower).