Sonic dynamic model loading

[trevin-lee]

PR description:

This PR introduces dynamic model loading/unloading capabilities and server health monitoring to the SONIC Triton integration in CMSSW. The main features include:

1. Dynamic Model Loading and Unloading:

• Adds loadModel() and unloadModel() methods to TritonService for managing model lifecycle at runtime
• Implements thread-safe model operations using mutex protection
• Introduces DynamicModelLoadingProducer test module to validate dynamic model management
• Models can be loaded on-demand and unloaded when no longer needed, improving resource utilization

4. Code Improvements:

• Moves retry configuration options to customize.py for better configurability
• Updates TritonClient with new constructor for testing and enhanced server connection methods
• Improves logging for model operations and server health status
• Refactors code for better maintainability and documentation

Expected Output Changes:

• Users can now dynamically load and unload models during job execution
• Improved resilience through automatic server health monitoring and failover
• Better error handling and retry logic for transient server failures
• Enhanced logging messages for model operations and server health

Dependencies:

• Based on CMSSW_15_1_0_pre6
• No external PR dependencies

Files Modified:

• HeterogeneousCore/SonicCore/src/SonicClientBase.cc
• HeterogeneousCore/SonicCore/plugins/BuildFile.xml
• HeterogeneousCore/SonicTriton/interface/TritonService.h
• HeterogeneousCore/SonicTriton/interface/TritonClient.h
• HeterogeneousCore/SonicTriton/src/TritonService.cc
• HeterogeneousCore/SonicTriton/src/TritonClient.cc
• HeterogeneousCore/SonicTriton/src/RetryActionDiffServer.cc
• HeterogeneousCore/SonicTriton/test/BuildFile.xml
• HeterogeneousCore/SonicTriton/test/tritonTest_cfg.py

New Files:

• HeterogeneousCore/SonicTriton/test/DynamicModelLoadingProducer.cc
• HeterogeneousCore/SonicTriton/test/test_RetryActionDiffServer.cc

Removed Files:

• HeterogeneousCore/SonicTriton/test/RetryActionDiffServer.cc (replaced with unit test)

PR validation:

Integration Tests:

• Compiled successfully with scram b -j 8
• No compilation warnings or errors
• All modified code follows CMS coding standards

Known Issues:

• Dynamic model loading tests require polling to be disabled in configuration
• Some unit tests need mock server environment adjustments
• Will be addressed in follow-up commits or configuration updates

Documentation:

• Code includes inline documentation for new methods
• Test configurations demonstrate usage patterns
• README updates may be needed (can be done in follow-up)

Backport Information:

This PR is NOT a backport. It is intended for CMSSW_15_1_X release cycle.

If backporting becomes necessary, it would target future release cycles after initial integration and validation in CMSSW_15_1_X.

[kpedro88]

I have not looked at the test code yet because I think the logic in the TritonService needs to be addressed first.

Another general point: part of the idea for dynamic loading with the fallback server would be to get rid of the unservedModels_ list that is currently formed at the start of the job. The model repository folder for the fallback server should be created with all models known to the job included, but the fallback server should be launched in explicit model control mode (modifying the cmsTriton script). Then, whenever a module switches over to the fallback server, it should ask the fallback server to load its model. (Dynamic loading with remote servers may reuse some of the logic, but will be somewhat different and needs more thought.)

[kpedro88]

@trevin-lee thanks, this is in good shape now! Just a few more minor comments.

Further thoughts on mutex for load/unload: I have actually changed my mind somewhat. Because we are doing both find() and then insert() operations, locking at some level is necessary.

There is a more streamlined approach possible using tbb:concurrent_hash_map, which has an accessor feature that locks a specific slot in the map, instead of the whole map. That allows operations on different keys to proceed in parallel. However, since we actually insert() into multiple maps, synchronization might still be a challenge with this approach. Needs further thought/investigation.

[kpedro88]

this is an incorrect change. "explicit" should still be the (new) default. "none" is only needed on L101 (as the value if -L is specified)

[trevin-lee]

Fixed - reverted the default back to "explicit" and updated the -L flag to set MODELCONTROL="none" instead of empty string.

[kpedro88]

@trevin-lee the most recent commit has a lot of changes that I don't understand. At least one of them is definitely incorrect, as remarked above. There are also tests commented out for unclear reasons, etc.

More broadly, maybe my previous comment wasn't clear. The key remark was here:

However, since we actually insert() into multiple maps, synchronization might still be a challenge with this approach.

Making serial modifications where one concurrent map is locked with an accessor and then another normal map is locked with a mutex can still lead to data races.

I would just revert the last commit. For now, the single mutex approach is safest. We can reconsider the design later (i.e. not in this PR) if it becomes a bottleneck. This will require careful design and implementation, not widespread changes with unclear motivations.