Thread safe StorageAccounting #10932
Conversation
As a step towards thread safety, LocalFileSystem is not immutable. The caching of any error messages caused by calling findCachePath is now retained within StorageFactory.
There were a number of functions which were marked virtual but which were not overriding methods of the base class. Given there are no classes which inherit from RFIOFile, these functions are now non-virtual and the class has been marked final. Added override to the appropriate places.
Switched to using smart pointers instead of bare pointers and converted appropriate functions to be const. Also provided a way to request only the const version of StorageFactory in order to better understand the threading issues.
The timeouts will now automatically happen if the StorageFactory was configured to use a timeout. The timeouts were never by file but always by protocol before so there should be no noticeable difference.
Changed the StorageMaker interface to allow them to be state-less which allows all methods to be const. Changed all classes inheriting from StorageMaker to make them thread-safe.
The output of this method was not being used.
The use of a boost::shared_ptr is unnecessary in this code since if there is an entry in the parent map the shared_ptr was always set.
Want to allow lock free read/write to the Counters. Reading the values while the program is running will lead to inconsistent values although for monitoring purposes this shouldn't be a problem.
Switched to the thread-safe tbb::concurrent_unordered_map container.
Instead of allowing each storage type to define its own set of strings for operations, we now use a preset list.
Previously the code just used strings to identify the different storage classes. This meant the need for lots of string comparisons. The code now uses StorageClassToken which is obtained using the original string name.
|
A new Pull Request was created by @Dr15Jones (Chris Jones) for CMSSW_7_6_X. Thread safe StorageAccounting It involves the following packages: FWCore/Services @cmsbuild, @smuzaffar, @Dr15Jones can you please review it and eventually sign? Thanks. |
|
please test |
|
+1 |
|
The tests are being triggered in jenkins. |
cmsbuild
added
core-approved
fully-signed
and removed
core-pending
pending-signatures
labels
|
This pull request is fully signed and it will be integrated in one of the next CMSSW_7_6_X IBs or unless it breaks tests. This pull request requires discussion in the ORP meeting before it's merged. @davidlange6, @Degano, @smuzaffar |
|
This pull request is fully signed and it will be integrated in one of the next CMSSW_7_6_X IBs or unless it breaks tests. This pull request requires discussion in the ORP meeting before it's merged. @davidlange6, @Degano, @smuzaffar |
|
please test |
|
The tests are being triggered in jenkins. |
cmsbuild
added
core-pending
tests-pending
comparison-pending
and removed
comparison-available
core-approved
tests-rejected
labels
|
+1 |
|
@smuzaffar @davidlange6 why does the github label say this needs approval by alca and generators? |
|
@Dr15Jones , due to changes in following packages |
|
Ping @bendavid @covarell @diguida @mmusich It has been 22 days since I submitted this. |
|
+1 |
Thread safe StorageAccounting
Changed StorageAccounting so that all methods are thread-safe.
Also changed the iterface to avoid multiple lookups via strings.