Ethanlm commented on a change in pull request #3336:
URL: https://github.com/apache/storm/pull/3336#discussion_r491163677
##########
File path:
storm-server/src/main/java/org/apache/storm/localizer/AsyncLocalizer.java
##########
@@ -641,7 +642,12 @@ void cleanup() {
try {
forEachTopologyDistDir((p, topologyId) -> {
- if (!safeTopologyIds.contains(topologyId)) {
+ String topoJarKey =
ConfigUtils.masterStormJarKey(topologyId);
+ String topoCodeKey =
ConfigUtils.masterStormCodeKey(topologyId);
+ String topoConfKey =
ConfigUtils.masterStormConfKey(topologyId);
+ if (!topologyBlobs.containsKey(topoJarKey)
Review comment:
I guess your question is if line 650 (`&&
!topologyBlobs.containsKey(topoConfKey)) {`) executes and before line 651
(`fsOps.deleteIfExists(p.toFile());`) runs, getTopoJar(topologyId) is called?
Yes there is a chance. But current change is much better than what we have
before.
Just for future reference, one thing about Rui's comment that I want to note
is that we still don't know where the 10s delay we observed from the incident
we had in our production cluster between where `safeTopologyIds` is populated
and `fsOps.deleteIfExists` is invoked comes from. It could come from
`forEachTopologyDistDir` file operations. But we don't know yet. Also this
issue doesn't alway happen.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
[email protected]
