Github user HeartSaVioR commented on a diff in the pull request:
https://github.com/apache/storm/pull/2812#discussion_r212838182
--- Diff:
storm-server/src/main/java/org/apache/storm/localizer/LocallyCachedTopologyBlob.java
---
@@ -53,21 +54,24 @@
private final boolean
Github user danny0405 commented on the issue:
https://github.com/apache/storm/pull/2800
@srdo
Cause only modify it through multi-threads but not iterator over it, and
the cache key is executor-id, which will only have conflict between master and
supervisor.
---
Github user danny0405 commented on a diff in the pull request:
https://github.com/apache/storm/pull/2812#discussion_r212851256
--- Diff:
storm-server/src/main/java/org/apache/storm/localizer/LocallyCachedTopologyBlob.java
---
@@ -125,6 +129,12 @@ public long
Github user srdo commented on the issue:
https://github.com/apache/storm/pull/2800
@danny0405 That doesn't sound safe to me. I think you're right that it
works fine most of the time, but if there are key collisions or an insert leads
the map to get resized, I would think that two
Github user HeartSaVioR commented on a diff in the pull request:
https://github.com/apache/storm/pull/2809#discussion_r212838254
--- Diff: pom.xml ---
@@ -1011,12 +1005,12 @@
log4j
log4j
-
Github user srdo commented on the issue:
https://github.com/apache/storm/pull/2800
@danny0405 Could you elaborate on why fixing the `executorBeats == null`
branch is enough? My concern is that the other branch modifies a HashMap (the
`cache` parameter) from multiple threads with no
Github user HeartSaVioR commented on a diff in the pull request:
https://github.com/apache/storm/pull/2812#discussion_r212838119
--- Diff:
storm-server/src/main/java/org/apache/storm/localizer/LocallyCachedTopologyBlob.java
---
@@ -125,6 +129,12 @@ public long
Github user srdo commented on the issue:
https://github.com/apache/storm/pull/2800
Regarding performance, consider that Nimbus is already copying
`heartbeatCache` on writes everywhere else
Github user srdo commented on the issue:
https://github.com/apache/storm/pull/2800
I don't think fixing the `executorBeats == null` branch is enough. As far
as I can tell, two supervisors/workers can be in the
Github user danny0405 commented on the issue:
https://github.com/apache/storm/pull/2800
@srdo
Agree that copy is a better solution.
---
10 matches
Mail list logo
