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`~~ (those 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.
--
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]
