smengcl commented on code in PR #3653:
URL: https://github.com/apache/ozone/pull/3653#discussion_r955303867
##########
hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/OzoneManager.java:
##########
@@ -710,6 +703,8 @@ private void instantiateServices(boolean withNewSnapshot)
throws IOException {
prefixManager = new PrefixManagerImpl(metadataManager, isRatisEnabled);
keyManager = new KeyManagerImpl(this, scmClient, configuration,
omStorage.getOmId());
Review Comment:
Thanks @GeorgeJahad . Good read.
I see how stale read could potentially become a problem now. e.g. after
another `instantiateServices()` call while other threads are still actively
using `metadataManager`, `multiTenantManager`, `volumeManager`, etc. This might
cause legitimate issue in other worker threads ~~like `OpenKeyCleanupService`,
`KeyDeletingService`~~ (sry, bad example, these are re-inited in
`KeyManagerImpl` so should be alright). Thanks for raising awareness of this
potential issue.
IIRC, making those references `volatile` shouldn't impose much performance
impact on variable reads (at least on x86 because of
[TSO](https://docs.oracle.com/cd/E23824_01/html/819-3196/hwovr-15.html#:~:text=store%20ordering%20(PSO).-,Total%20Store%20Ordering,-(TSO))).
Let's open a separate jira on master branch to discuss this with others as
well (and to check if other existing OM threads could be affected by stale
read).
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
