Github user satishd commented on a diff in the pull request:
https://github.com/apache/storm/pull/1280#discussion_r58077961
--- Diff: storm-core/src/clj/org/apache/storm/daemon/supervisor.clj ---
@@ -296,11 +293,15 @@
(worker-launcher-and-wait conf user ["signal" pid "9"] :log-prefix
(str "kill -9 " pid))
(force-kill-process pid))
(if as-user
- (rmr-as-user conf id (worker-pid-path conf id pid))
(try
+ (rmr-as-user conf id (worker-pid-path conf id pid))
(rmpath (worker-pid-path conf id pid))
(rmpath (worker-tmp-root conf id pid))
- (catch Exception e)))) ;; on windows, the supervisor may still
holds the lock on the worker directory
+ (catch IOException e
+ (log-warn-error e "Failed to cleanup pid dir: " pid " for
worker " id". Will retry later"))
+ (catch RuntimeException e
+ (log-warn-error e "Failed to cleanup pid dir: " pid " for
worker " id". Will retry later")))))
+ ;; on windows, the supervisor may still holds the lock on the
worker directory
--- End diff --
@zhuoliu IMHO, `Exception` should be caught and logged and we should not
really constrain with catching only `IOException` and `RuntimeException`. When
any of those methods is changed to throw some other `Exception` then this code
should also be changed to handle that, otherwise this code would throw that
Exception and get into unexpected scenarios. That may be the reason why
`Exception` is caught in earlier code.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at [email protected] or file a JIRA ticket
with INFRA.
---
