Github user zhuoliu commented on a diff in the pull request:
https://github.com/apache/storm/pull/1280#discussion_r58066223
--- 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 --
Hi @satishd , your concern makes good sense, however, I do not find any
other type of exception that might be thrown by the try block (the
worker-launcher-and-wait in rmr-as-user throws InterruptedException and catches
it already). It this is case, catching specific exceptions should be preferable
to catching Exception.
---
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.
---
