ChengbingLiu commented on PR #5246: URL: https://github.com/apache/hadoop/pull/5246#issuecomment-1362517793
> So one issue I realized with this approach is that `LogThrottlingHelper` isn't thread-safe. We probably need to make `LogThrottlingHelper` properly threadsafe (atomics, concurrent hash map, etc.). IMO, to make `LogThrottlingHelper` thread-safe, it isn't enough just to make its member variables thread-safe. For example, if we just change `Map<String, LoggingAction> currentLogs;` to a ConcurrentHashMap, it won't help because there are multiple gets and puts inside the `record` method. It seems that the only way to make it thread-safe is to add `synchronized` to `record(...)` as well as other methods. However, doing so may affect the performance. > Is it possible to have multiple instances of `FSEditLogLoader` or `RedundantEditLogInputStream` running simultaneously? I think yes? Tracing down usages of `LogThrottlingHelper` in `FSEditLogLoader` or `RedundantEditLogInputStream`, the main usage seems to happen inside a `FSNameSystem` writeLock/writeUnlock block, yet there might be other cases that can have a concurrency issue. -- 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. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected] --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
