Github user danny0405 commented on a diff in the pull request:
https://github.com/apache/storm/pull/2812#discussion_r212787574
--- Diff:
storm-server/src/main/java/org/apache/storm/localizer/LocallyCachedTopologyBlob.java
---
@@ -230,6 +240,19 @@ public void commitNewVersion(long newVersion) throws
IOException {
//Don't try to move the JAR file in local mode, it does not
exist because it was not uploaded
Files.move(tempLoc, dest);
}
+ synchronized (LocallyCachedTopologyBlob.class) {
+ //This is a bit ugly, but it works. In order to maintain the
same directory structure that existed before
+ // we need to have storm conf, storm jar, and storm code in a
shared directory, and we need to set the
+ // permissions for that entire directory, but the tracking is
on a per item basis, so we are going to end
+ // up running the permission modification code once for each
blob that is downloaded (3 times in this case).
+ // Because the permission modification code runs in a separate
process we are doing a global lock to avoid
+ // any races between multiple versions running at the same
time. Ideally this would be on a per topology
+ // basis, but that is a lot harder and the changes run fairly
quickly so it should not be a big deal.
+ fsOps.setupStormCodeDir(owner,
topologyBasicBlobsRootDir.toFile());
--- End diff --
It's not a big deal if we modify permission for 3 times of every topology
if they are write conflict safe. Agree with you that a Supervisor scope lock
would be easier to do, but if we have multi supervisors, the write conflict
should be considered.
---
